Series comparison

-[PULL 0/4] Block patches
+[PULL 0/2] Block patches
-The following changes since commit 560009f2c8b57b7cdd31a5693ea86ab369382f49:
+The following changes since commit ca61fa4b803e5d0abaf6f1ceb690f23bb78a4def:
-  Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging (2019-10-07 15:40:53 +0100)
+  Merge remote-tracking branch 'remotes/quic/tags/pull-hex-20211006' into staging (2021-10-06 12:11:14 -0700)
 are available in the Git repository at:
-  https://github.com/stefanha/qemu.git tags/block-pull-request
+  https://gitlab.com/stefanha/qemu.git tags/block-pull-request
-for you to fetch changes up to 4d804b5305ffb4d5fa414c38d4f1bdfb987c8d0b:
+for you to fetch changes up to 1cc7eada97914f090125e588497986f6f7900514:
-  iotests/262: Switch source/dest VM launch order (2019-10-08 14:28:25 +0100)
+  iothread: use IOThreadParamInfo in iothread_[set|get]_param() (2021-10-07 15:29:50 +0100)
 ----------------------------------------------------------------
 Pull request
-This pull request also contains the two commits from the previous pull request
-that was dropped due to a mingw compilation error.  The compilation should now
-be fixed.
 ----------------------------------------------------------------
-Max Reitz (2):
+Stefano Garzarella (2):
-  block: Skip COR for inactive nodes
+  iothread: rename PollParamInfo to IOThreadParamInfo
-  iotests/262: Switch source/dest VM launch order
+  iothread: use IOThreadParamInfo in iothread_[set|get]_param()
-Sergio Lopez (1):
+ iothread.c | 28 +++++++++++++++-------------
-  virtio-blk: schedule virtio_notify_config to run on main context
+file changed, 15 insertions(+), 13 deletions(-)
 Vladimir Sementsov-Ogievskiy (1):
   util/ioc.c: try to reassure Coverity about qemu_iovec_init_extended
  block/io.c                 | 41 +++++++++++++++++++++++++-------------
  hw/block/virtio-blk.c      | 16 ++++++++++++++-
  util/iov.c                 |  5 +++--
  tests/qemu-iotests/262     | 12 +++++------
  tests/qemu-iotests/262.out |  6 +++---
 files changed, 54 insertions(+), 26 deletions(-)
 --
-.21.0
+.31.1

-[PULL 1/4] util/ioc.c: try to reassure Coverity about qemu_iovec_init_extended
+Deleted patch
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Make it more obvious, that filling qiov corresponds to qiov allocation,
-which in turn corresponds to total_niov calculation, based on mid_niov
-(not mid_len). Still add an assertion to show that there should be no
-difference.
-[Added mingw "error: 'mid_iov' may be used uninitialized in this
-function" compiler error fix suggested by Vladimir.
---Stefan]
-Reported-by: Coverity (CID 1405302)
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Message-id: 20190910090310.14032-1-vsementsov@virtuozzo.com
-Suggested-by: Peter Maydell <peter.maydell@linaro.org>
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Message-Id: <20190910090310.14032-1-vsementsov@virtuozzo.com>
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-fixup! util/ioc.c: try to reassure Coverity about qemu_iovec_init_extended
----
- util/iov.c | 5 +++--
-file changed, 3 insertions(+), 2 deletions(-)
-diff --git a/util/iov.c b/util/iov.c
-index XXXXXXX..XXXXXXX 100644
---- a/util/iov.c
-+++ b/util/iov.c
-@@ -XXX,XX +XXX,XX @@ void qemu_iovec_init_extended(
- {
-     size_t mid_head, mid_tail;
-     int total_niov, mid_niov = 0;
--    struct iovec *p, *mid_iov;
-+    struct iovec *p, *mid_iov = NULL;
-     if (mid_len) {
-         mid_iov = qiov_slice(mid_qiov, mid_offset, mid_len,
-@@ -XXX,XX +XXX,XX @@ void qemu_iovec_init_extended(
-         p++;
-     }
--    if (mid_len) {
-+    assert(!mid_niov == !mid_len);
-+    if (mid_niov) {
-         memcpy(p, mid_iov, mid_niov * sizeof(*p));
-         p[0].iov_base = (uint8_t *)p[0].iov_base + mid_head;
-         p[0].iov_len -= mid_head;
---
-.21.0

-[PULL 4/4] iotests/262: Switch source/dest VM launch order
+[PULL 1/2] iothread: rename PollParamInfo to IOThreadParamInfo
-From: Max Reitz <mreitz@redhat.com>
+From: Stefano Garzarella <sgarzare@redhat.com>
-Launching the destination VM before the source VM gives us a regression
+Commit 1793ad0247 ("iothread: add aio-max-batch parameter") added
-test for HEAD^:
+a new parameter (aio-max-batch) to IOThread and used PollParamInfo
 structure to handle it.
-The guest device causes a read from the disk image through
+Since it is not a parameter of the polling mechanism, we rename the
-guess_disk_lchs().  This will not work if the first sector (containing
+structure to a more generic IOThreadParamInfo.
 the partition table) is yet unallocated, we use COR, and the node is
 inactive.
-By launching the source VM before the destination, however, the COR
+Suggested-by: Kevin Wolf <kwolf@redhat.com>
-filter on the source will allocate that area in the image shared between
+Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
-both VMs, thus the problem will not become apparent.
+Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
+Message-id: 20210727145936.147032-2-sgarzare@redhat.com
 Switching the launch order causes the sector to still be unallocated
 when guess_disk_lchs() runs on the inactive node in the destination VM,
 and thus we get our test case.
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 Reviewed-by: Eric Blake <eblake@redhat.com>
 Message-id: 20191001174827.11081-3-mreitz@redhat.com
 Message-Id: <20191001174827.11081-3-mreitz@redhat.com>
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- tests/qemu-iotests/262     | 12 ++++++------
+ iothread.c | 14 +++++++-------
- tests/qemu-iotests/262.out |  6 +++---
+file changed, 7 insertions(+), 7 deletions(-)
 files changed, 9 insertions(+), 9 deletions(-)
-diff --git a/tests/qemu-iotests/262 b/tests/qemu-iotests/262
+diff --git a/iothread.c b/iothread.c
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/262
 +++ b/tests/qemu-iotests/262
@@ -XXX,XX +XXX,XX @@ with iotests.FilePath('img') as img_path, \
      os.mkfifo(fifo)
 -    iotests.log('Launching source VM...')
 -    add_opts(vm_a)
 -    vm_a.launch()
 -
 -    vm_a.enable_migration_events('A')
 -
      iotests.log('Launching destination VM...')
      add_opts(vm_b)
      vm_b.add_incoming("exec: cat '%s'" % (fifo))
@@ -XXX,XX +XXX,XX @@ with iotests.FilePath('img') as img_path, \
      vm_b.enable_migration_events('B')
 +    iotests.log('Launching source VM...')
 +    add_opts(vm_a)
 +    vm_a.launch()
 +
 +    vm_a.enable_migration_events('A')
 +
      iotests.log('Starting migration to B...')
      iotests.log(vm_a.qmp('migrate', uri='exec:cat >%s' % (fifo)))
      with iotests.Timeout(3, 'Migration does not complete'):
 diff --git a/tests/qemu-iotests/262.out b/tests/qemu-iotests/262.out
 index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/262.out
+--- a/iothread.c
-+++ b/tests/qemu-iotests/262.out
++++ b/iothread.c
-@@ -XXX,XX +XXX,XX @@
+@@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp)
--Launching source VM...
+ typedef struct {
--Enabling migration QMP events on A...
+     const char *name;
--{"return": {}}
+     ptrdiff_t offset; /* field's byte offset in IOThread struct */
- Launching destination VM...
+-} PollParamInfo;
- Enabling migration QMP events on B...
++} IOThreadParamInfo;
- {"return": {}}
-+Launching source VM...
+-static PollParamInfo poll_max_ns_info = {
-+Enabling migration QMP events on A...
++static IOThreadParamInfo poll_max_ns_info = {
-+{"return": {}}
+     "poll-max-ns", offsetof(IOThread, poll_max_ns),
- Starting migration to B...
+ };
- {"return": {}}
+-static PollParamInfo poll_grow_info = {
- {"data": {"status": "setup"}, "event": "MIGRATION", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
++static IOThreadParamInfo poll_grow_info = {
      "poll-grow", offsetof(IOThread, poll_grow),
  };
 -static PollParamInfo poll_shrink_info = {
 +static IOThreadParamInfo poll_shrink_info = {
      "poll-shrink", offsetof(IOThread, poll_shrink),
  };
 -static PollParamInfo aio_max_batch_info = {
 +static IOThreadParamInfo aio_max_batch_info = {
      "aio-max-batch", offsetof(IOThread, aio_max_batch),
  };
@@ -XXX,XX +XXX,XX @@ static void iothread_get_param(Object *obj, Visitor *v,
          const char *name, void *opaque, Error **errp)
  {
      IOThread *iothread = IOTHREAD(obj);
 -    PollParamInfo *info = opaque;
 +    IOThreadParamInfo *info = opaque;
      int64_t *field = (void *)iothread + info->offset;
      visit_type_int64(v, name, field, errp);
@@ -XXX,XX +XXX,XX @@ static bool iothread_set_param(Object *obj, Visitor *v,
          const char *name, void *opaque, Error **errp)
  {
      IOThread *iothread = IOTHREAD(obj);
 -    PollParamInfo *info = opaque;
 +    IOThreadParamInfo *info = opaque;
      int64_t *field = (void *)iothread + info->offset;
      int64_t value;
 --
-.21.0
+.31.1

-[PULL 2/4] virtio-blk: schedule virtio_notify_config to run on main context
+[PULL 2/2] iothread: use IOThreadParamInfo in iothread_[set|get]_param()
-From: Sergio Lopez <slp@redhat.com>
+From: Stefano Garzarella <sgarzare@redhat.com>
-virtio_notify_config() needs to acquire the global mutex, which isn't
+Commit 0445409d74 ("iothread: generalize
-allowed from an iothread, and may lead to a deadlock like this:
+iothread_set_param/iothread_get_param") moved common code to set and
 get IOThread parameters in two new functions.
- - main thead
+These functions are called inside callbacks, so we don't need to use an
-  * Has acquired: qemu_global_mutex.
+opaque pointer. Let's replace `void *opaque` parameter with
-  * Is trying the acquire: iothread AioContext lock via
+`IOThreadParamInfo *info`.
     AIO_WAIT_WHILE (after aio_poll).
- - iothread
+Suggested-by: Kevin Wolf <kwolf@redhat.com>
-  * Has acquired: AioContext lock.
+Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
-  * Is trying to acquire: qemu_global_mutex (via
+Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
-    virtio_notify_config->prepare_mmio_access).
+Message-id: 20210727145936.147032-3-sgarzare@redhat.com
 If virtio_blk_resize() is called from an iothread, schedule
 virtio_notify_config() to be run in the main context BH.
 [Removed unnecessary newline as suggested by Kevin Wolf
 <kwolf@redhat.com>.
 --Stefan]
 Signed-off-by: Sergio Lopez <slp@redhat.com>
 Reviewed-by: Kevin Wolf <kwolf@redhat.com>
 Message-id: 20190916112411.21636-1-slp@redhat.com
 Message-Id: <20190916112411.21636-1-slp@redhat.com>
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- hw/block/virtio-blk.c | 16 +++++++++++++++-
+ iothread.c | 18 ++++++++++--------
-file changed, 15 insertions(+), 1 deletion(-)
+file changed, 10 insertions(+), 8 deletions(-)
-diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
+diff --git a/iothread.c b/iothread.c
 index XXXXXXX..XXXXXXX 100644
---- a/hw/block/virtio-blk.c
+--- a/iothread.c
-+++ b/hw/block/virtio-blk.c
++++ b/iothread.c
-@@ -XXX,XX +XXX,XX @@
+@@ -XXX,XX +XXX,XX @@ static IOThreadParamInfo aio_max_batch_info = {
- #include "qemu/iov.h"
+ };
- #include "qemu/module.h"
- #include "qemu/error-report.h"
+ static void iothread_get_param(Object *obj, Visitor *v,
-+#include "qemu/main-loop.h"
+-        const char *name, void *opaque, Error **errp)
- #include "trace.h"
++        const char *name, IOThreadParamInfo *info, Error **errp)
- #include "hw/block/block.h"
+ {
- #include "hw/qdev-properties.h"
+     IOThread *iothread = IOTHREAD(obj);
-@@ -XXX,XX +XXX,XX @@ static int virtio_blk_load_device(VirtIODevice *vdev, QEMUFile *f,
+-    IOThreadParamInfo *info = opaque;
-     return 0;
+     int64_t *field = (void *)iothread + info->offset;
      visit_type_int64(v, name, field, errp);
  }
-+static void virtio_resize_cb(void *opaque)
+ static bool iothread_set_param(Object *obj, Visitor *v,
-+{
+-        const char *name, void *opaque, Error **errp)
-+    VirtIODevice *vdev = opaque;
++        const char *name, IOThreadParamInfo *info, Error **errp)
 +
 +    assert(qemu_get_current_aio_context() == qemu_get_aio_context());
 +    virtio_notify_config(vdev);
 +}
 +
  static void virtio_blk_resize(void *opaque)
  {
-     VirtIODevice *vdev = VIRTIO_DEVICE(opaque);
+     IOThread *iothread = IOTHREAD(obj);
+-    IOThreadParamInfo *info = opaque;
--    virtio_notify_config(vdev);
+     int64_t *field = (void *)iothread + info->offset;
-+    /*
+     int64_t value;
-+     * virtio_notify_config() needs to acquire the global mutex,
-+     * so it can't be called from an iothread. Instead, schedule
+@@ -XXX,XX +XXX,XX @@ static bool iothread_set_param(Object *obj, Visitor *v,
-+     * it to be run in the main context BH.
+ static void iothread_get_poll_param(Object *obj, Visitor *v,
-+     */
+         const char *name, void *opaque, Error **errp)
-+    aio_bh_schedule_oneshot(qemu_get_aio_context(), virtio_resize_cb, vdev);
+ {
 +    IOThreadParamInfo *info = opaque;
 -    iothread_get_param(obj, v, name, opaque, errp);
 +    iothread_get_param(obj, v, name, info, errp);
  }
- static const BlockDevOps virtio_block_ops = {
+ static void iothread_set_poll_param(Object *obj, Visitor *v,
          const char *name, void *opaque, Error **errp)
  {
      IOThread *iothread = IOTHREAD(obj);
 +    IOThreadParamInfo *info = opaque;
 -    if (!iothread_set_param(obj, v, name, opaque, errp)) {
 +    if (!iothread_set_param(obj, v, name, info, errp)) {
          return;
      }
@@ -XXX,XX +XXX,XX @@ static void iothread_set_poll_param(Object *obj, Visitor *v,
  static void iothread_get_aio_param(Object *obj, Visitor *v,
          const char *name, void *opaque, Error **errp)
  {
 +    IOThreadParamInfo *info = opaque;
 -    iothread_get_param(obj, v, name, opaque, errp);
 +    iothread_get_param(obj, v, name, info, errp);
  }
  static void iothread_set_aio_param(Object *obj, Visitor *v,
          const char *name, void *opaque, Error **errp)
  {
      IOThread *iothread = IOTHREAD(obj);
 +    IOThreadParamInfo *info = opaque;
 -    if (!iothread_set_param(obj, v, name, opaque, errp)) {
 +    if (!iothread_set_param(obj, v, name, info, errp)) {
          return;
      }
 --
-.21.0
+.31.1

-[PULL 3/4] block: Skip COR for inactive nodes
+Deleted patch
-From: Max Reitz <mreitz@redhat.com>
-We must not write data to inactive nodes, and a COR is certainly
-something we can simply not do without upsetting anyone.  So skip COR
-operations on inactive nodes.
-Signed-off-by: Max Reitz <mreitz@redhat.com>
-Reviewed-by: Eric Blake <eblake@redhat.com>
-Message-id: 20191001174827.11081-2-mreitz@redhat.com
-Message-Id: <20191001174827.11081-2-mreitz@redhat.com>
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
----
- block/io.c | 41 +++++++++++++++++++++++++++--------------
-file changed, 27 insertions(+), 14 deletions(-)
-diff --git a/block/io.c b/block/io.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/io.c
-+++ b/block/io.c
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
-     int max_transfer = MIN_NON_ZERO(bs->bl.max_transfer,
-                                     BDRV_REQUEST_MAX_BYTES);
-     unsigned int progress = 0;
-+    bool skip_write;
-     if (!drv) {
-         return -ENOMEDIUM;
-     }
-+    /*
-+     * Do not write anything when the BDS is inactive.  That is not
-+     * allowed, and it would not help.
-+     */
-+    skip_write = (bs->open_flags & BDRV_O_INACTIVE);
-+
-     /* FIXME We cannot require callers to have write permissions when all they
-      * are doing is a read request. If we did things right, write permissions
-      * would be obtained anyway, but internally by the copy-on-read code. As
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
-     while (cluster_bytes) {
-         int64_t pnum;
--        ret = bdrv_is_allocated(bs, cluster_offset,
--                                MIN(cluster_bytes, max_transfer), &pnum);
--        if (ret < 0) {
--            /* Safe to treat errors in querying allocation as if
--             * unallocated; we'll probably fail again soon on the
--             * read, but at least that will set a decent errno.
--             */
-+        if (skip_write) {
-+            ret = 1; /* "already allocated", so nothing will be copied */
-             pnum = MIN(cluster_bytes, max_transfer);
--        }
-+        } else {
-+            ret = bdrv_is_allocated(bs, cluster_offset,
-+                                    MIN(cluster_bytes, max_transfer), &pnum);
-+            if (ret < 0) {
-+                /*
-+                 * Safe to treat errors in querying allocation as if
-+                 * unallocated; we'll probably fail again soon on the
-+                 * read, but at least that will set a decent errno.
-+                 */
-+                pnum = MIN(cluster_bytes, max_transfer);
-+            }
--        /* Stop at EOF if the image ends in the middle of the cluster */
--        if (ret == 0 && pnum == 0) {
--            assert(progress >= bytes);
--            break;
--        }
-+            /* Stop at EOF if the image ends in the middle of the cluster */
-+            if (ret == 0 && pnum == 0) {
-+                assert(progress >= bytes);
-+                break;
-+            }
--        assert(skip_bytes < pnum);
-+            assert(skip_bytes < pnum);
-+        }
-         if (ret <= 0) {
-             QEMUIOVector local_qiov;
---
-.21.0

The following changes since commit 560009f2c8b57b7cdd31a5693ea86ab369382f49:

Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging (2019-10-07 15:40:53 +0100)

are available in the Git repository at:

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

for you to fetch changes up to 4d804b5305ffb4d5fa414c38d4f1bdfb987c8d0b:

iotests/262: Switch source/dest VM launch order (2019-10-08 14:28:25 +0100)

----------------------------------------------------------------
Pull request

This pull request also contains the two commits from the previous pull request
that was dropped due to a mingw compilation error.  The compilation should now
be fixed.

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

Max Reitz (2):
  block: Skip COR for inactive nodes
  iotests/262: Switch source/dest VM launch order

Sergio Lopez (1):
  virtio-blk: schedule virtio_notify_config to run on main context

Vladimir Sementsov-Ogievskiy (1):
  util/ioc.c: try to reassure Coverity about qemu_iovec_init_extended

-- 
2.21.0

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Make it more obvious, that filling qiov corresponds to qiov allocation,
which in turn corresponds to total_niov calculation, based on mid_niov
(not mid_len). Still add an assertion to show that there should be no
difference.

[Added mingw "error: 'mid_iov' may be used uninitialized in this
function" compiler error fix suggested by Vladimir.
--Stefan]

Reported-by: Coverity (CID 1405302)
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190910090310.14032-1-vsementsov@virtuozzo.com
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20190910090310.14032-1-vsementsov@virtuozzo.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

fixup! util/ioc.c: try to reassure Coverity about qemu_iovec_init_extended
---
 util/iov.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/util/iov.c b/util/iov.c
index XXXXXXX..XXXXXXX 100644
--- a/util/iov.c
+++ b/util/iov.c
@@ -XXX,XX +XXX,XX @@ void qemu_iovec_init_extended(
 {
     size_t mid_head, mid_tail;
     int total_niov, mid_niov = 0;
-    struct iovec *p, *mid_iov;
+    struct iovec *p, *mid_iov = NULL;
 
     if (mid_len) {
         mid_iov = qiov_slice(mid_qiov, mid_offset, mid_len,
@@ -XXX,XX +XXX,XX @@ void qemu_iovec_init_extended(
         p++;
     }
 
-    if (mid_len) {
+    assert(!mid_niov == !mid_len);
+    if (mid_niov) {
         memcpy(p, mid_iov, mid_niov * sizeof(*p));
         p[0].iov_base = (uint8_t *)p[0].iov_base + mid_head;
         p[0].iov_len -= mid_head;
-- 
2.21.0

From: Sergio Lopez <slp@redhat.com>

virtio_notify_config() needs to acquire the global mutex, which isn't
allowed from an iothread, and may lead to a deadlock like this:

- main thead
  * Has acquired: qemu_global_mutex.
  * Is trying the acquire: iothread AioContext lock via
    AIO_WAIT_WHILE (after aio_poll).

- iothread
  * Has acquired: AioContext lock.
  * Is trying to acquire: qemu_global_mutex (via
    virtio_notify_config->prepare_mmio_access).

If virtio_blk_resize() is called from an iothread, schedule
virtio_notify_config() to be run in the main context BH.

[Removed unnecessary newline as suggested by Kevin Wolf
<kwolf@redhat.com>.
--Stefan]

Signed-off-by: Sergio Lopez <slp@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-id: 20190916112411.21636-1-slp@redhat.com
Message-Id: <20190916112411.21636-1-slp@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/block/virtio-blk.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -XXX,XX +XXX,XX @@
 #include "qemu/iov.h"
 #include "qemu/module.h"
 #include "qemu/error-report.h"
+#include "qemu/main-loop.h"
 #include "trace.h"
 #include "hw/block/block.h"
 #include "hw/qdev-properties.h"
@@ -XXX,XX +XXX,XX @@ static int virtio_blk_load_device(VirtIODevice *vdev, QEMUFile *f,
     return 0;
 }
 
+static void virtio_resize_cb(void *opaque)
+{
+    VirtIODevice *vdev = opaque;
+
+    assert(qemu_get_current_aio_context() == qemu_get_aio_context());
+    virtio_notify_config(vdev);
+}
+
 static void virtio_blk_resize(void *opaque)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(opaque);
 
-    virtio_notify_config(vdev);
+    /*
+     * virtio_notify_config() needs to acquire the global mutex,
+     * so it can't be called from an iothread. Instead, schedule
+     * it to be run in the main context BH.
+     */
+    aio_bh_schedule_oneshot(qemu_get_aio_context(), virtio_resize_cb, vdev);
 }
 
 static const BlockDevOps virtio_block_ops = {
-- 
2.21.0

From: Max Reitz <mreitz@redhat.com>

We must not write data to inactive nodes, and a COR is certainly
something we can simply not do without upsetting anyone.  So skip COR
operations on inactive nodes.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20191001174827.11081-2-mreitz@redhat.com
Message-Id: <20191001174827.11081-2-mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/io.c | 41 +++++++++++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 14 deletions(-)

diff --git a/block/io.c b/block/io.c
index XXXXXXX..XXXXXXX 100644
--- a/block/io.c
+++ b/block/io.c
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
     int max_transfer = MIN_NON_ZERO(bs->bl.max_transfer,
                                     BDRV_REQUEST_MAX_BYTES);
     unsigned int progress = 0;
+    bool skip_write;
 
     if (!drv) {
         return -ENOMEDIUM;
     }
 
+    /*
+     * Do not write anything when the BDS is inactive.  That is not
+     * allowed, and it would not help.
+     */
+    skip_write = (bs->open_flags & BDRV_O_INACTIVE);
+
     /* FIXME We cannot require callers to have write permissions when all they
      * are doing is a read request. If we did things right, write permissions
      * would be obtained anyway, but internally by the copy-on-read code. As
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
     while (cluster_bytes) {
         int64_t pnum;
 
-        ret = bdrv_is_allocated(bs, cluster_offset,
-                                MIN(cluster_bytes, max_transfer), &pnum);
-        if (ret < 0) {
-            /* Safe to treat errors in querying allocation as if
-             * unallocated; we'll probably fail again soon on the
-             * read, but at least that will set a decent errno.
-             */
+        if (skip_write) {
+            ret = 1; /* "already allocated", so nothing will be copied */
             pnum = MIN(cluster_bytes, max_transfer);
-        }
+        } else {
+            ret = bdrv_is_allocated(bs, cluster_offset,
+                                    MIN(cluster_bytes, max_transfer), &pnum);
+            if (ret < 0) {
+                /*
+                 * Safe to treat errors in querying allocation as if
+                 * unallocated; we'll probably fail again soon on the
+                 * read, but at least that will set a decent errno.
+                 */
+                pnum = MIN(cluster_bytes, max_transfer);
+            }
 
-        /* Stop at EOF if the image ends in the middle of the cluster */
-        if (ret == 0 && pnum == 0) {
-            assert(progress >= bytes);
-            break;
-        }
+            /* Stop at EOF if the image ends in the middle of the cluster */
+            if (ret == 0 && pnum == 0) {
+                assert(progress >= bytes);
+                break;
+            }
 
-        assert(skip_bytes < pnum);
+            assert(skip_bytes < pnum);
+        }
 
         if (ret <= 0) {
             QEMUIOVector local_qiov;
-- 
2.21.0

From: Max Reitz <mreitz@redhat.com>

Launching the destination VM before the source VM gives us a regression
test for HEAD^:

The guest device causes a read from the disk image through
guess_disk_lchs().  This will not work if the first sector (containing
the partition table) is yet unallocated, we use COR, and the node is
inactive.

By launching the source VM before the destination, however, the COR
filter on the source will allocate that area in the image shared between
both VMs, thus the problem will not become apparent.

Switching the launch order causes the sector to still be unallocated
when guess_disk_lchs() runs on the inactive node in the destination VM,
and thus we get our test case.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20191001174827.11081-3-mreitz@redhat.com
Message-Id: <20191001174827.11081-3-mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qemu-iotests/262     | 12 ++++++------
 tests/qemu-iotests/262.out |  6 +++---
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/tests/qemu-iotests/262 b/tests/qemu-iotests/262
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/262
+++ b/tests/qemu-iotests/262
@@ -XXX,XX +XXX,XX @@ with iotests.FilePath('img') as img_path, \
 
     os.mkfifo(fifo)
 
-    iotests.log('Launching source VM...')
-    add_opts(vm_a)
-    vm_a.launch()
-
-    vm_a.enable_migration_events('A')
-
     iotests.log('Launching destination VM...')
     add_opts(vm_b)
     vm_b.add_incoming("exec: cat '%s'" % (fifo))
@@ -XXX,XX +XXX,XX @@ with iotests.FilePath('img') as img_path, \
 
     vm_b.enable_migration_events('B')
 
+    iotests.log('Launching source VM...')
+    add_opts(vm_a)
+    vm_a.launch()
+
+    vm_a.enable_migration_events('A')
+
     iotests.log('Starting migration to B...')
     iotests.log(vm_a.qmp('migrate', uri='exec:cat >%s' % (fifo)))
     with iotests.Timeout(3, 'Migration does not complete'):
diff --git a/tests/qemu-iotests/262.out b/tests/qemu-iotests/262.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/262.out
+++ b/tests/qemu-iotests/262.out
@@ -XXX,XX +XXX,XX @@
-Launching source VM...
-Enabling migration QMP events on A...
-{"return": {}}
 Launching destination VM...
 Enabling migration QMP events on B...
 {"return": {}}
+Launching source VM...
+Enabling migration QMP events on A...
+{"return": {}}
 Starting migration to B...
 {"return": {}}
 {"data": {"status": "setup"}, "event": "MIGRATION", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
-- 
2.21.0

From: Stefano Garzarella <sgarzare@redhat.com>

Commit 1793ad0247 ("iothread: add aio-max-batch parameter") added
a new parameter (aio-max-batch) to IOThread and used PollParamInfo
structure to handle it.

Since it is not a parameter of the polling mechanism, we rename the
structure to a more generic IOThreadParamInfo.

Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20210727145936.147032-2-sgarzare@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 iothread.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/iothread.c b/iothread.c
index XXXXXXX..XXXXXXX 100644
--- a/iothread.c
+++ b/iothread.c
@@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp)
 typedef struct {
     const char *name;
     ptrdiff_t offset; /* field's byte offset in IOThread struct */
-} PollParamInfo;
+} IOThreadParamInfo;
 
-static PollParamInfo poll_max_ns_info = {
+static IOThreadParamInfo poll_max_ns_info = {
     "poll-max-ns", offsetof(IOThread, poll_max_ns),
 };
-static PollParamInfo poll_grow_info = {
+static IOThreadParamInfo poll_grow_info = {
     "poll-grow", offsetof(IOThread, poll_grow),
 };
-static PollParamInfo poll_shrink_info = {
+static IOThreadParamInfo poll_shrink_info = {
     "poll-shrink", offsetof(IOThread, poll_shrink),
 };
-static PollParamInfo aio_max_batch_info = {
+static IOThreadParamInfo aio_max_batch_info = {
     "aio-max-batch", offsetof(IOThread, aio_max_batch),
 };
 
@@ -XXX,XX +XXX,XX @@ static void iothread_get_param(Object *obj, Visitor *v,
         const char *name, void *opaque, Error **errp)
 {
     IOThread *iothread = IOTHREAD(obj);
-    PollParamInfo *info = opaque;
+    IOThreadParamInfo *info = opaque;
     int64_t *field = (void *)iothread + info->offset;
 
     visit_type_int64(v, name, field, errp);
@@ -XXX,XX +XXX,XX @@ static bool iothread_set_param(Object *obj, Visitor *v,
         const char *name, void *opaque, Error **errp)
 {
     IOThread *iothread = IOTHREAD(obj);
-    PollParamInfo *info = opaque;
+    IOThreadParamInfo *info = opaque;
     int64_t *field = (void *)iothread + info->offset;
     int64_t value;
 
-- 
2.31.1

From: Stefano Garzarella <sgarzare@redhat.com>

Commit 0445409d74 ("iothread: generalize
iothread_set_param/iothread_get_param") moved common code to set and
get IOThread parameters in two new functions.

These functions are called inside callbacks, so we don't need to use an
opaque pointer. Let's replace `void *opaque` parameter with
`IOThreadParamInfo *info`.

Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20210727145936.147032-3-sgarzare@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 iothread.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/iothread.c b/iothread.c
index XXXXXXX..XXXXXXX 100644
--- a/iothread.c
+++ b/iothread.c
@@ -XXX,XX +XXX,XX @@ static IOThreadParamInfo aio_max_batch_info = {
 };
 
 static void iothread_get_param(Object *obj, Visitor *v,
-        const char *name, void *opaque, Error **errp)
+        const char *name, IOThreadParamInfo *info, Error **errp)
 {
     IOThread *iothread = IOTHREAD(obj);
-    IOThreadParamInfo *info = opaque;
     int64_t *field = (void *)iothread + info->offset;
 
     visit_type_int64(v, name, field, errp);
 }
 
 static bool iothread_set_param(Object *obj, Visitor *v,
-        const char *name, void *opaque, Error **errp)
+        const char *name, IOThreadParamInfo *info, Error **errp)
 {
     IOThread *iothread = IOTHREAD(obj);
-    IOThreadParamInfo *info = opaque;
     int64_t *field = (void *)iothread + info->offset;
     int64_t value;
 
@@ -XXX,XX +XXX,XX @@ static bool iothread_set_param(Object *obj, Visitor *v,
 static void iothread_get_poll_param(Object *obj, Visitor *v,
         const char *name, void *opaque, Error **errp)
 {
+    IOThreadParamInfo *info = opaque;
 
-    iothread_get_param(obj, v, name, opaque, errp);
+    iothread_get_param(obj, v, name, info, errp);
 }
 
 static void iothread_set_poll_param(Object *obj, Visitor *v,
         const char *name, void *opaque, Error **errp)
 {
     IOThread *iothread = IOTHREAD(obj);
+    IOThreadParamInfo *info = opaque;
 
-    if (!iothread_set_param(obj, v, name, opaque, errp)) {
+    if (!iothread_set_param(obj, v, name, info, errp)) {
         return;
     }
 
@@ -XXX,XX +XXX,XX @@ static void iothread_set_poll_param(Object *obj, Visitor *v,
 static void iothread_get_aio_param(Object *obj, Visitor *v,
         const char *name, void *opaque, Error **errp)
 {
+    IOThreadParamInfo *info = opaque;
 
-    iothread_get_param(obj, v, name, opaque, errp);
+    iothread_get_param(obj, v, name, info, errp);
 }
 
 static void iothread_set_aio_param(Object *obj, Visitor *v,
         const char *name, void *opaque, Error **errp)
 {
     IOThread *iothread = IOTHREAD(obj);
+    IOThreadParamInfo *info = opaque;
 
-    if (!iothread_set_param(obj, v, name, opaque, errp)) {
+    if (!iothread_set_param(obj, v, name, info, errp)) {
         return;
     }
 
-- 
2.31.1