The following changes since commit 411ad78115ebeb3411cf4b7622784b93dfabe259:

Merge remote-tracking branch 'remotes/stefanberger/tags/pull-tpm-2017-12-15-1' into staging (2017-12-17 15:27:41 +0000)

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

for you to fetch changes up to 585426c518958aa768564596091474be786aae51:

qemu-iotests: add 203 savevm with IOThreads test (2017-12-18 13:12:53 +0000)

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

Mao Zhongyi (4):

hw/block/nvme: Convert to realize

hw/block: Fix the return type

hw/block: Use errp directly rather than local_err

dev-storage: Fix the unusual function name

Mark Kanda (2):

virtio-blk: make queue size configurable

virtio-blk: reject configs with logical block size > physical block

size

Paolo Bonzini (1):

block: avoid recursive AioContext acquire in bdrv_inactivate_all()

Stefan Hajnoczi (16):

coroutine: simplify co_aio_sleep_ns() prototype

qdev: drop unused #include "sysemu/iothread.h"

blockdev: hold AioContext for bdrv_unref() in

external_snapshot_clean()

block: don't keep AioContext acquired after

external_snapshot_prepare()

block: don't keep AioContext acquired after drive_backup_prepare()

block: don't keep AioContext acquired after blockdev_backup_prepare()

block: don't keep AioContext acquired after

internal_snapshot_prepare()

block: drop unused BlockDirtyBitmapState->aio_context field

iothread: add iothread_by_id() API

blockdev: add x-blockdev-set-iothread testing command

qemu-iotests: add 202 external snapshots IOThread test

docs: mark nested AioContext locking as a legacy API

blockdev: add x-blockdev-set-iothread force boolean

iotests: add VM.add_object()

iothread: fix iothread_stop() race condition

qemu-iotests: add 203 savevm with IOThreads test

docs/devel/multiple-iothreads.txt | 7 +-

qapi/block-core.json | 40 ++++++

hw/block/dataplane/virtio-blk.h | 2 +-

include/hw/block/block.h | 4 +-

include/hw/virtio/virtio-blk.h | 1 +

include/qemu/coroutine.h | 6 +-

include/sysemu/iothread.h | 4 +-

block.c | 14 ++-

block/null.c | 3 +-

block/sheepdog.c | 3 +-

blockdev.c | 259 +++++++++++++++++++++++++++-----------

hw/block/block.c | 15 ++-

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

hw/block/fdc.c | 17 +--

hw/block/nvme.c | 23 ++--

hw/block/virtio-blk.c | 35 ++++--

hw/core/qdev-properties-system.c | 1 -

hw/ide/qdev.c | 12 +-

hw/scsi/scsi-disk.c | 13 +-

hw/usb/dev-storage.c | 29 ++---

iothread.c | 27 +++-

util/qemu-coroutine-sleep.c | 4 +-

tests/qemu-iotests/202 | 95 ++++++++++++++

tests/qemu-iotests/202.out | 11 ++

tests/qemu-iotests/203 | 59 +++++++++

tests/qemu-iotests/203.out | 6 +

tests/qemu-iotests/group | 2 +

tests/qemu-iotests/iotests.py | 5 +

28 files changed, 532 insertions(+), 177 deletions(-)

create mode 100755 tests/qemu-iotests/202

create mode 100644 tests/qemu-iotests/202.out

create mode 100755 tests/qemu-iotests/203

create mode 100644 tests/qemu-iotests/203.out

2.14.3

There is a small chance that iothread_stop() hangs as follows:

Thread 3 (Thread 0x7f63eba5f700 (LWP 16105)):

#0 0x00007f64012c09b6 in ppoll () at /lib64/libc.so.6

#1 0x000055959992eac9 in ppoll (__ss=0x0, __timeout=0x0, __nfds=<optimized out>, __fds=<optimized out>) at /usr/include/bits/poll2.h:77

#2 0x000055959992eac9 in qemu_poll_ns (fds=<optimized out>, nfds=<optimized out>, timeout=<optimized out>) at util/qemu-timer.c:322

#3 0x0000559599930711 in aio_poll (ctx=0x55959bdb83c0, blocking=blocking@entry=true) at util/aio-posix.c:629

#4 0x00005595996806fe in iothread_run (opaque=0x55959bd78400) at iothread.c:59

#5 0x00007f640159f609 in start_thread () at /lib64/libpthread.so.0

#6 0x00007f64012cce6f in clone () at /lib64/libc.so.6

Thread 1 (Thread 0x7f640b45b280 (LWP 16103)):

#0 0x00007f64015a0b6d in pthread_join () at /lib64/libpthread.so.0

#1 0x00005595999332ef in qemu_thread_join (thread=<optimized out>) at util/qemu-thread-posix.c:547

#2 0x00005595996808ae in iothread_stop (iothread=<optimized out>) at iothread.c:91

#3 0x000055959968094d in iothread_stop_iter (object=<optimized out>, opaque=<optimized out>) at iothread.c:102

#4 0x0000559599857d97 in do_object_child_foreach (obj=obj@entry=0x55959bdb8100, fn=fn@entry=0x559599680930 <iothread_stop_iter>, opaque=opaque@entry=0x0, recurse=recurse@entry=false) at qom/object.c:852

#5 0x0000559599859477 in object_child_foreach (obj=obj@entry=0x55959bdb8100, fn=fn@entry=0x559599680930 <iothread_stop_iter>, opaque=opaque@entry=0x0) at qom/object.c:867

#6 0x0000559599680a6e in iothread_stop_all () at iothread.c:341

#7 0x000055959955b1d5 in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at vl.c:4913

The relevant code from iothread_run() is:

while (!atomic_read(&iothread->stopping)) {

aio_poll(iothread->ctx, true);

and iothread_stop():

iothread->stopping = true;

aio_notify(iothread->ctx);

...

qemu_thread_join(&iothread->thread);

The following scenario can occur:

1. IOThread:

while (!atomic_read(&iothread->stopping)) -> stopping=false

2. Main loop:

iothread->stopping = true;

aio_notify(iothread->ctx);

3. IOThread:

aio_poll(iothread->ctx, true); -> hang

The bug is explained by the AioContext->notify_me doc comments:

"If this field is 0, everything (file descriptors, bottom halves,

timers) will be re-evaluated before the next blocking poll(), thus the

event_notifier_set call can be skipped."

The problem is that "everything" does not include checking

iothread->stopping. This means iothread_run() will block in aio_poll()

if aio_notify() was called just before aio_poll().

This patch fixes the hang by replacing aio_notify() with

aio_bh_schedule_oneshot(). This makes aio_poll() or g_main_loop_run()

to return.

Implementing this properly required a new bool running flag. The new

flag prevents races that are tricky if we try to use iothread->stopping.

Now iothread->stopping is purely for iothread_stop() and

iothread->running is purely for the iothread_run() thread.

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

Reviewed-by: Eric Blake <eblake@redhat.com>

Message-id: 20171207201320.19284-6-stefanha@redhat.com

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

---

include/sysemu/iothread.h | 3 ++-

iothread.c | 20 +++++++++++++++-----

2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h

--- a/include/sysemu/iothread.h

+++ b/include/sysemu/iothread.h

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

GOnce once;

QemuMutex init_done_lock;

QemuCond init_done_cond; /* is thread initialization done? */

- bool stopping;

+ bool stopping; /* has iothread_stop() been called? */

+ bool running; /* should iothread_run() continue? */

int thread_id;

/* AioContext poll parameters */

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

qemu_cond_signal(&iothread->init_done_cond);

qemu_mutex_unlock(&iothread->init_done_lock);

- while (!atomic_read(&iothread->stopping)) {

+ while (iothread->running) {

aio_poll(iothread->ctx, true);

if (atomic_read(&iothread->worker_context)) {

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

return NULL;

+/* Runs in iothread_run() thread */

+static void iothread_stop_bh(void *opaque)

+{

+ IOThread *iothread = opaque;

+

+ iothread->running = false; /* stop iothread_run() */

+

+ if (iothread->main_loop) {

+ g_main_loop_quit(iothread->main_loop);

+ }

+}

+

void iothread_stop(IOThread *iothread)

if (!iothread->ctx || iothread->stopping) {

iothread->stopping = true;

- aio_notify(iothread->ctx);

- if (atomic_read(&iothread->main_loop)) {

- g_main_loop_quit(iothread->main_loop);

+ aio_bh_schedule_oneshot(iothread->ctx, iothread_stop_bh, iothread);

qemu_thread_join(&iothread->thread);

}

@@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp)

char *name, *thread_name;

iothread->stopping = false;

+ iothread->running = true;

iothread->thread_id = -1;

iothread->ctx = aio_context_new(&local_error);

if (!iothread->ctx) {

2.14.3

From: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>

Cc: John Snow <jsnow@redhat.com>

Cc: Kevin Wolf <kwolf@redhat.com>

Cc: Max Reitz <mreitz@redhat.com>

Cc: Keith Busch <keith.busch@intel.com>

Cc: Stefan Hajnoczi <stefanha@redhat.com>

Cc: "Michael S. Tsirkin" <mst@redhat.com>

Cc: Paolo Bonzini <pbonzini@redhat.com>

Cc: Gerd Hoffmann <kraxel@redhat.com>

Cc: Markus Armbruster <armbru@redhat.com>

Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>

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

Message-id: e77848d3735ba590f23ffbf8094379c646c33d79.1511317952.git.maozy.fnst@cn.fujitsu.com

hw/block/fdc.c | 17 ++++++-----------

hw/block/nvme.c | 7 ++-----

hw/block/virtio-blk.c | 18 ++++++------------

hw/ide/qdev.c | 12 ++++--------

hw/scsi/scsi-disk.c | 13 ++++---------

hw/usb/dev-storage.c | 9 +++------

6 files changed, 25 insertions(+), 51 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/block/fdc.c

+++ b/hw/block/fdc.c

@@ -XXX,XX +XXX,XX @@ static void fd_revalidate(FDrive *drv)

static void fd_change_cb(void *opaque, bool load, Error **errp)

{

FDrive *drive = opaque;

- Error *local_err = NULL;

if (!load) {

blk_set_perm(drive->blk, 0, BLK_PERM_ALL, &error_abort);

- blkconf_apply_backend_options(drive->conf,

- blk_is_read_only(drive->blk), false,

- &local_err);

- if (local_err) {

- error_propagate(errp, local_err);

+ if (!blkconf_apply_backend_options(drive->conf,

+ blk_is_read_only(drive->blk), false,

+ errp)) {