The following changes since commit 0ab4537f08e09b13788db67efd760592fb7db769:

Merge remote-tracking branch 'remotes/stefanberger/tags/pull-tpm-2018-03-07-1' into staging (2018-03-08 12:56:39 +0000)

git://github.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to 4486e89c219c0d1b9bd8dfa0b1dd5b0d51ff2268:

vl: introduce vm_shutdown() (2018-03-08 17:38:51 +0000)

----------------------------------------------------------------

Deepa Srinivasan (1):

block: Fix qemu crash when using scsi-block

Fam Zheng (1):

README: Fix typo 'git-publish'

Sergio Lopez (1):

virtio-blk: dataplane: Don't batch notifications if EVENT_IDX is

present

Stefan Hajnoczi (4):

block: add aio_wait_bh_oneshot()

virtio-blk: fix race between .ioeventfd_stop() and vq handler

virtio-scsi: fix race between .ioeventfd_stop() and vq handler

vl: introduce vm_shutdown()

include/block/aio-wait.h | 13 +++++++++++

include/sysemu/iothread.h | 1 -

include/sysemu/sysemu.h | 1 +

block/block-backend.c | 51 ++++++++++++++++++++---------------------

cpus.c | 16 ++++++++++---

hw/block/dataplane/virtio-blk.c | 39 +++++++++++++++++++++++--------

hw/scsi/virtio-scsi-dataplane.c | 9 ++++----

iothread.c | 31 -------------------------

util/aio-wait.c | 31 +++++++++++++++++++++++++

vl.c | 13 +++--------

README | 2 +-

11 files changed, 122 insertions(+), 85 deletions(-)

2.14.3

Commit 00d09fdbbae5f7864ce754913efc84c12fdf9f1a ("vl: pause vcpus before

stopping iothreads") and commit dce8921b2baaf95974af8176406881872067adfa

("iothread: Stop threads before main() quits") tried to work around the

fact that emulation was still active during termination by stopping

iothreads. They suffer from race conditions:

1. virtio_scsi_handle_cmd_vq() racing with iothread_stop_all() hits the

virtio_scsi_ctx_check() assertion failure because the BDS AioContext

has been modified by iothread_stop_all().

2. Guest vq kick racing with main loop termination leaves a readable

ioeventfd that is handled by the next aio_poll() when external

clients are enabled again, resulting in unwanted emulation activity.

This patch obsoletes those commits by fully disabling emulation activity

when vcpus are stopped.

Use the new vm_shutdown() function instead of pause_all_vcpus() so that

vm change state handlers are invoked too. Virtio devices will now stop

their ioeventfds, preventing further emulation activity after vm_stop().

Note that vm_stop(RUN_STATE_SHUTDOWN) cannot be used because it emits a

QMP STOP event that may affect existing clients.

It is no longer necessary to call replay_disable_events() directly since

vm_shutdown() does so already.

Drop iothread_stop_all() since it is no longer used.

Cc: Fam Zheng <famz@redhat.com>

Cc: Kevin Wolf <kwolf@redhat.com>

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Fam Zheng <famz@redhat.com>

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

Message-id: 20180307144205.20619-5-stefanha@redhat.com

include/sysemu/iothread.h | 1 -

include/sysemu/sysemu.h | 1 +

cpus.c | 16 +++++++++++++---

iothread.c | 31 -------------------------------

vl.c | 13 +++----------

5 files changed, 17 insertions(+), 45 deletions(-)

@@ -XXX,XX +XXX,XX @@ void iothread_stop(IOThread *iothread)

qemu_thread_join(&iothread->thread);

}

-static int iothread_stop_iter(Object *object, void *opaque)

-{

- IOThread *iothread;

-

- iothread = (IOThread *)object_dynamic_cast(object, TYPE_IOTHREAD);

- if (!iothread) {

- return 0;

- }

- iothread_stop(iothread);

- return 0;

-}

-

static void iothread_instance_init(Object *obj)

@@ -XXX,XX +XXX,XX @@ IOThreadInfoList *qmp_query_iothreads(Error **errp)

return head;

}

-void iothread_stop_all(void)

-{

- Object *container = object_get_objects_root();

- BlockDriverState *bs;

- BdrvNextIterator it;

-

- for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {

- AioContext *ctx = bdrv_get_aio_context(bs);

- if (ctx == qemu_get_aio_context()) {

- continue;

- }

- aio_context_acquire(ctx);

- bdrv_set_aio_context(bs, qemu_get_aio_context());

- aio_context_release(ctx);

- }

-

- object_child_foreach(container, iothread_stop_iter, NULL);

-}

-

static gpointer iothread_g_main_context_init(gpointer opaque)

AioContext *ctx;

diff --git a/vl.c b/vl.c

index XXXXXXX..XXXXXXX 100644

--- a/vl.c

+++ b/vl.c

@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv, char **envp)

os_setup_post();

main_loop();

- replay_disable_events();

- /* The ordering of the following is delicate. Stop vcpus to prevent new

- * I/O requests being queued by the guest. Then stop IOThreads (this

- * includes a drain operation and completes all request processing). At

- * this point emulated devices are still associated with their IOThreads

- * (if any) but no longer have any work to do. Only then can we close

- * block devices safely because we know there is no more I/O coming.

- */

- pause_all_vcpus();

- iothread_stop_all();

+ /* No more vcpu or device emulation activity beyond this point */

+ vm_shutdown();

+

bdrv_close_all();

res_free();

2.14.3

From: Deepa Srinivasan <deepa.srinivasan@oracle.com>

Starting qemu with the following arguments causes qemu to segfault:

... -device lsi,id=lsi0 -drive file=iscsi:<...>,format=raw,if=none,node-name=

iscsi1 -device scsi-block,bus=lsi0.0,id=<...>,drive=iscsi1

This patch fixes blk_aio_ioctl() so it does not pass stack addresses to

blk_aio_ioctl_entry() which may be invoked after blk_aio_ioctl() returns. More

details about the bug follow.

blk_aio_ioctl() invokes blk_aio_prwv() with blk_aio_ioctl_entry as the

coroutine parameter. blk_aio_prwv() ultimately calls aio_co_enter().

When blk_aio_ioctl() is executed from within a coroutine context (e.g.

iscsi_bh_cb()), aio_co_enter() adds the coroutine (blk_aio_ioctl_entry) to

the current coroutine's wakeup queue. blk_aio_ioctl() then returns.

When blk_aio_ioctl_entry() executes later, it accesses an invalid pointer:

....

BlkRwCo *rwco = &acb->rwco;

rwco->ret = blk_co_ioctl(rwco->blk, rwco->offset,

rwco->qiov->iov[0].iov_base); <--- qiov is

invalid here

...

In the case when blk_aio_ioctl() is called from a non-coroutine context,

blk_aio_ioctl_entry() executes immediately. But if bdrv_co_ioctl() calls

qemu_coroutine_yield(), blk_aio_ioctl() will return. When the coroutine

execution is complete, control returns to blk_aio_ioctl_entry() after the call

to blk_co_ioctl(). There is no invalid reference after this point, but the

function is still holding on to invalid pointers.

The fix is to change blk_aio_prwv() to accept a void pointer for the IO buffer

rather than a QEMUIOVector. blk_aio_prwv() passes this through in BlkRwCo and the

coroutine function casts it to QEMUIOVector or uses the void pointer directly.

Signed-off-by: Deepa Srinivasan <deepa.srinivasan@oracle.com>

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Reviewed-by: Mark Kanda <mark.kanda@oracle.com>

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

block/block-backend.c | 51 +++++++++++++++++++++++++--------------------------

1 file changed, 25 insertions(+), 26 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c

--- a/block/block-backend.c

+++ b/block/block-backend.c

@@ -XXX,XX +XXX,XX @@ int coroutine_fn blk_co_pwritev(BlockBackend *blk, int64_t offset,

typedef struct BlkRwCo {

BlockBackend *blk;

int64_t offset;

- QEMUIOVector *qiov;

+ void *iobuf;

int ret;

BdrvRequestFlags flags;

} BlkRwCo;

@@ -XXX,XX +XXX,XX @@ typedef struct BlkRwCo {

static void blk_read_entry(void *opaque)

BlkRwCo *rwco = opaque;

+ QEMUIOVector *qiov = rwco->iobuf;

- rwco->ret = blk_co_preadv(rwco->blk, rwco->offset, rwco->qiov->size,

- rwco->qiov, rwco->flags);

+ rwco->ret = blk_co_preadv(rwco->blk, rwco->offset, qiov->size,

+ qiov, rwco->flags);

static void blk_write_entry(void *opaque)

BlkRwCo *rwco = opaque;

+ QEMUIOVector *qiov = rwco->iobuf;

- rwco->ret = blk_co_pwritev(rwco->blk, rwco->offset, rwco->qiov->size,

- rwco->qiov, rwco->flags);

+ rwco->ret = blk_co_pwritev(rwco->blk, rwco->offset, qiov->size,

+ qiov, rwco->flags);

static int blk_prw(BlockBackend *blk, int64_t offset, uint8_t *buf,

@@ -XXX,XX +XXX,XX @@ static int blk_prw(BlockBackend *blk, int64_t offset, uint8_t *buf,

rwco = (BlkRwCo) {

.blk = blk,

.offset = offset,

- .qiov = &qiov,

+ .iobuf = &qiov,

.flags = flags,

.ret = NOT_DONE,

};

@@ -XXX,XX +XXX,XX @@ static void blk_aio_complete_bh(void *opaque)

static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, int64_t offset, int bytes,

- QEMUIOVector *qiov, CoroutineEntry co_entry,

+ void *iobuf, CoroutineEntry co_entry,

BdrvRequestFlags flags,

BlockCompletionFunc *cb, void *opaque)

@@ -XXX,XX +XXX,XX @@ static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, int64_t offset, int bytes,

acb->rwco = (BlkRwCo) {

.blk = blk,

.offset = offset,

- .qiov = qiov,

+ .iobuf = iobuf,

.flags = flags,

.ret = NOT_DONE,

};

@@ -XXX,XX +XXX,XX @@ static void blk_aio_read_entry(void *opaque)

{

BlkAioEmAIOCB *acb = opaque;

BlkRwCo *rwco = &acb->rwco;

+ QEMUIOVector *qiov = rwco->iobuf;

- assert(rwco->qiov->size == acb->bytes);

+ assert(qiov->size == acb->bytes);

rwco->ret = blk_co_preadv(rwco->blk, rwco->offset, acb->bytes,

- rwco->qiov, rwco->flags);

+ qiov, rwco->flags);

blk_aio_complete(acb);

}

@@ -XXX,XX +XXX,XX @@ static void blk_aio_write_entry(void *opaque)

{

BlkAioEmAIOCB *acb = opaque;

BlkRwCo *rwco = &acb->rwco;

+ QEMUIOVector *qiov = rwco->iobuf;

- assert(!rwco->qiov || rwco->qiov->size == acb->bytes);

+ assert(!qiov || qiov->size == acb->bytes);

rwco->ret = blk_co_pwritev(rwco->blk, rwco->offset, acb->bytes,

- rwco->qiov, rwco->flags);

+ qiov, rwco->flags);

blk_aio_complete(acb);

}

@@ -XXX,XX +XXX,XX @@ int blk_co_ioctl(BlockBackend *blk, unsigned long int req, void *buf)

static void blk_ioctl_entry(void *opaque)

{

BlkRwCo *rwco = opaque;

+ QEMUIOVector *qiov = rwco->iobuf;

+

rwco->ret = blk_co_ioctl(rwco->blk, rwco->offset,

- rwco->qiov->iov[0].iov_base);

+ qiov->iov[0].iov_base);

}

int blk_ioctl(BlockBackend *blk, unsigned long int req, void *buf)

@@ -XXX,XX +XXX,XX @@ static void blk_aio_ioctl_entry(void *opaque)

BlkAioEmAIOCB *acb = opaque;

BlkRwCo *rwco = &acb->rwco;

- rwco->ret = blk_co_ioctl(rwco->blk, rwco->offset,

- rwco->qiov->iov[0].iov_base);

+ rwco->ret = blk_co_ioctl(rwco->blk, rwco->offset, rwco->iobuf);

+

blk_aio_complete(acb);

}

BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, void *buf,

BlockCompletionFunc *cb, void *opaque)

{

- QEMUIOVector qiov;

- struct iovec iov;

-

- iov = (struct iovec) {

- .iov_base = buf,

- .iov_len = 0,

- };

- qemu_iovec_init_external(&qiov, &iov, 1);

-

- return blk_aio_prwv(blk, req, 0, &qiov, blk_aio_ioctl_entry, 0, cb, opaque);

+ return blk_aio_prwv(blk, req, 0, buf, blk_aio_ioctl_entry, 0, cb, opaque);

}

int blk_co_pdiscard(BlockBackend *blk, int64_t offset, int bytes)

@@ -XXX,XX +XXX,XX @@ int blk_truncate(BlockBackend *blk, int64_t offset, PreallocMode prealloc,

static void blk_pdiscard_entry(void *opaque)

{

BlkRwCo *rwco = opaque;

- rwco->ret = blk_co_pdiscard(rwco->blk, rwco->offset, rwco->qiov->size);

+ QEMUIOVector *qiov = rwco->iobuf;

+

+ rwco->ret = blk_co_pdiscard(rwco->blk, rwco->offset, qiov->size);

}

int blk_pdiscard(BlockBackend *blk, int64_t offset, int bytes)

2.14.3