Series comparison

-[Qemu-devel] [PULL 0/3] Block patches
+[PULL 0/2] Block patches
-The following changes since commit f90ea7ba7c5ae7010ee0ce062207ae42530f57d6:
+The following changes since commit 346ed3151f1c43e72c40cb55b392a1d4cface62c:
-  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20171012' into staging (2017-10-12 17:06:50 +0100)
+  Merge remote-tracking branch 'remotes/awilliam/tags/vfio-update-20200206.0' into staging (2020-02-07 11:52:15 +0000)
-are available in the git repository at:
+are available in the Git repository at:
-  git://github.com/stefanha/qemu.git tags/block-pull-request
+  https://github.com/stefanha/qemu.git tags/block-pull-request
-for you to fetch changes up to b867eaa17b3940760f51134e409cb0580dd3dde3:
+for you to fetch changes up to 11a18c84db4a71497d3d40769688a01b6f64b2ad:
-  block/throttle.c: add bdrv_co_drain_begin/end callbacks (2017-10-13 12:38:41 +0100)
+  hw/core: Allow setting 'virtio-blk-device.scsi' property on OSX host (2020-02-07 16:49:39 +0000)
 ----------------------------------------------------------------
 Pull request
 ----------------------------------------------------------------
-----------------------------------------------------------------
+Philippe Mathieu-Daudé (1):
   hw/core: Allow setting 'virtio-blk-device.scsi' property on OSX host
-Manos Pitsidianakis (3):
+Vladimir Sementsov-Ogievskiy (1):
-  block: add bdrv_co_drain_end callback
+  block: fix crash on zero-length unaligned write and read
   block: rename bdrv_co_drain to bdrv_co_drain_begin
   block/throttle.c: add bdrv_co_drain_begin/end callbacks
- include/block/block_int.h | 13 ++++++++++---
+ block/io.c        | 28 +++++++++++++++++++++++++++-
- block/io.c                | 48 +++++++++++++++++++++++++++++++++--------------
+ hw/core/machine.c |  3 ++-
- block/qed.c               |  6 +++---
+files changed, 29 insertions(+), 2 deletions(-)
  block/throttle.c          | 18 ++++++++++++++++++
 files changed, 65 insertions(+), 20 deletions(-)
 --
-.13.6
+.24.1

-[Qemu-devel] [PULL 1/3] block: add bdrv_co_drain_end callback
+[PULL 1/2] block: fix crash on zero-length unaligned write and read
-From: Manos Pitsidianakis <el13635@mail.ntua.gr>
+From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-BlockDriverState has a bdrv_co_drain() callback but no equivalent for
+Commit 7a3f542fbd "block/io: refactor padding" occasionally dropped
-the end of the drain. The throttle driver (block/throttle.c) needs a way
+aligning for zero-length request: bdrv_init_padding() blindly return
-to mark the end of the drain in order to toggle io_limits_disabled
+false if bytes == 0, like there is nothing to align.
 correctly, thus bdrv_co_drain_end is needed.
-Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr>
+This leads the following command to crash:
-Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
-Reviewed-by: Fam Zheng <famz@redhat.com>
+./qemu-io --image-opts -c 'write 1 0' \
   driver=blkdebug,align=512,image.driver=null-co,image.size=512
 >> qemu-io: block/io.c:1955: bdrv_aligned_pwritev: Assertion
     `(offset & (align - 1)) == 0' failed.
 >> Aborted (core dumped)
 Prior to 7a3f542fbd we does aligning of such zero requests. Instead of
 recovering this behavior let's just do nothing on such requests as it
 is useless.
 Note that driver may have special meaning of zero-length reqeusts, like
 qcow2_co_pwritev_compressed_part, so we can't skip any zero-length
 operation. But for unaligned ones, we can't pass it to driver anyway.
 This commit also fixes crash in iotest 80 running with -nocache:
 ./check -nocache -qcow2 80
 which crashes on same assertion due to trying to read empty extra data
 in qcow2_do_read_snapshots().
 Cc: qemu-stable@nongnu.org # v4.2
 Fixes: 7a3f542fbd
 Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
 Reviewed-by: Max Reitz <mreitz@redhat.com>
 Message-id: 20200206164245.17781-1-vsementsov@virtuozzo.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- include/block/block_int.h | 11 +++++++++--
+ block/io.c | 28 +++++++++++++++++++++++++++-
- block/io.c                | 48 +++++++++++++++++++++++++++++++++--------------
+file changed, 27 insertions(+), 1 deletion(-)
 files changed, 43 insertions(+), 16 deletions(-)
-diff --git a/include/block/block_int.h b/include/block/block_int.h
-index XXXXXXX..XXXXXXX 100644
---- a/include/block/block_int.h
-+++ b/include/block/block_int.h
-@@ -XXX,XX +XXX,XX @@ struct BlockDriver {
-     int (*bdrv_probe_geometry)(BlockDriverState *bs, HDGeometry *geo);
-     /**
--     * Drain and stop any internal sources of requests in the driver, and
--     * remain so until next I/O callback (e.g. bdrv_co_writev) is called.
-+     * bdrv_co_drain is called if implemented in the beginning of a
-+     * drain operation to drain and stop any internal sources of requests in
-+     * the driver.
-+     * bdrv_co_drain_end is called if implemented at the end of the drain.
-+     *
-+     * They should be used by the driver to e.g. manage scheduled I/O
-+     * requests, or toggle an internal state. After the end of the drain new
-+     * requests will continue normally.
-      */
-     void coroutine_fn (*bdrv_co_drain)(BlockDriverState *bs);
-+    void coroutine_fn (*bdrv_co_drain_end)(BlockDriverState *bs);
-     void (*bdrv_add_child)(BlockDriverState *parent, BlockDriverState *child,
-                            Error **errp);
 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 @@ typedef struct {
+@@ -XXX,XX +XXX,XX @@ static bool bdrv_init_padding(BlockDriverState *bs,
-     Coroutine *co;
+         pad->tail = align - pad->tail;
      BlockDriverState *bs;
      bool done;
 +    bool begin;
  } BdrvCoDrainData;
  static void coroutine_fn bdrv_drain_invoke_entry(void *opaque)
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn bdrv_drain_invoke_entry(void *opaque)
      BdrvCoDrainData *data = opaque;
      BlockDriverState *bs = data->bs;
 -    bs->drv->bdrv_co_drain(bs);
 +    if (data->begin) {
 +        bs->drv->bdrv_co_drain(bs);
 +    } else {
 +        bs->drv->bdrv_co_drain_end(bs);
 +    }
      /* Set data->done before reading bs->wakeup.  */
      atomic_mb_set(&data->done, true);
      bdrv_wakeup(bs);
  }
 -static void bdrv_drain_invoke(BlockDriverState *bs)
 +static void bdrv_drain_invoke(BlockDriverState *bs, bool begin)
  {
 -    BdrvCoDrainData data = { .bs = bs, .done = false };
 +    BdrvCoDrainData data = { .bs = bs, .done = false, .begin = begin};
 -    if (!bs->drv || !bs->drv->bdrv_co_drain) {
 +    if (!bs->drv || (begin && !bs->drv->bdrv_co_drain) ||
 +            (!begin && !bs->drv->bdrv_co_drain_end)) {
          return;
      }
-@@ -XXX,XX +XXX,XX @@ static void bdrv_drain_invoke(BlockDriverState *bs)
+-    if ((!pad->head && !pad->tail) || !bytes) {
-     BDRV_POLL_WHILE(bs, !data.done);
++    if (!pad->head && !pad->tail) {
- }
+         return false;
+     }
--static bool bdrv_drain_recurse(BlockDriverState *bs)
-+static bool bdrv_drain_recurse(BlockDriverState *bs, bool begin)
++    assert(bytes); /* Nothing good in aligning zero-length requests */
  {
      BdrvChild *child, *tmp;
      bool waited;
 -    waited = BDRV_POLL_WHILE(bs, atomic_read(&bs->in_flight) > 0);
 -
      /* Ensure any pending metadata writes are submitted to bs->file.  */
 -    bdrv_drain_invoke(bs);
 +    bdrv_drain_invoke(bs, begin);
 +
-+    /* Wait for drained requests to finish */
+     sum = pad->head + bytes + pad->tail;
-+    waited = BDRV_POLL_WHILE(bs, atomic_read(&bs->in_flight) > 0);
+     pad->buf_len = (sum > align && pad->head && pad->tail) ? 2 * align : align;
+     pad->buf = qemu_blockalign(bs, pad->buf_len);
-     QLIST_FOREACH_SAFE(child, &bs->children, next, tmp) {
+@@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_preadv_part(BdrvChild *child,
-         BlockDriverState *bs = child->bs;
+         return ret;
-@@ -XXX,XX +XXX,XX @@ static bool bdrv_drain_recurse(BlockDriverState *bs)
+     }
-              */
-             bdrv_ref(bs);
++    if (bytes == 0 && !QEMU_IS_ALIGNED(offset, bs->bl.request_alignment)) {
-         }
++        /*
--        waited |= bdrv_drain_recurse(bs);
++         * Aligning zero request is nonsense. Even if driver has special meaning
-+        waited |= bdrv_drain_recurse(bs, begin);
++         * of zero-length (like qcow2_co_pwritev_compressed_part), we can't pass
-         if (in_main_loop) {
++         * it to driver due to request_alignment.
-             bdrv_unref(bs);
++         *
-         }
++         * Still, no reason to return an error if someone do unaligned
-@@ -XXX,XX +XXX,XX @@ static void bdrv_co_drain_bh_cb(void *opaque)
++         * zero-length read occasionally.
-     BlockDriverState *bs = data->bs;
++         */
++        return 0;
      bdrv_dec_in_flight(bs);
 -    bdrv_drained_begin(bs);
 +    if (data->begin) {
 +        bdrv_drained_begin(bs);
 +    } else {
 +        bdrv_drained_end(bs);
 +    }
 +
-     data->done = true;
-     aio_co_wake(co);
- }
--static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs)
-+static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs,
-+                                                bool begin)
- {
-     BdrvCoDrainData data;
-@@ -XXX,XX +XXX,XX @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs)
-         .co = qemu_coroutine_self(),
-         .bs = bs,
-         .done = false,
-+        .begin = begin,
-     };
      bdrv_inc_in_flight(bs);
-     aio_bh_schedule_oneshot(bdrv_get_aio_context(bs),
-@@ -XXX,XX +XXX,XX @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs)
+     /* Don't do copy-on-read if we read data before write operation */
- void bdrv_drained_begin(BlockDriverState *bs)
+@@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_pwritev_part(BdrvChild *child,
- {
+         return -ENOTSUP;
      if (qemu_in_coroutine()) {
 -        bdrv_co_yield_to_drain(bs);
 +        bdrv_co_yield_to_drain(bs, true);
          return;
      }
-@@ -XXX,XX +XXX,XX @@ void bdrv_drained_begin(BlockDriverState *bs)
++    if (bytes == 0 && !QEMU_IS_ALIGNED(offset, bs->bl.request_alignment)) {
-         bdrv_parent_drained_begin(bs);
++        /*
-     }
++         * Aligning zero request is nonsense. Even if driver has special meaning
++         * of zero-length (like qcow2_co_pwritev_compressed_part), we can't pass
--    bdrv_drain_recurse(bs);
++         * it to driver due to request_alignment.
-+    bdrv_drain_recurse(bs, true);
++         *
- }
++         * Still, no reason to return an error if someone do unaligned
++         * zero-length write occasionally.
- void bdrv_drained_end(BlockDriverState *bs)
++         */
- {
++        return 0;
 +    if (qemu_in_coroutine()) {
 +        bdrv_co_yield_to_drain(bs, false);
 +        return;
 +    }
-     assert(bs->quiesce_counter > 0);
++
-     if (atomic_fetch_dec(&bs->quiesce_counter) > 1) {
+     bdrv_inc_in_flight(bs);
-         return;
+     /*
-     }
+      * Align write if necessary by performing a read-modify-write cycle.
      bdrv_parent_drained_end(bs);
 +    bdrv_drain_recurse(bs, false);
      aio_enable_external(bdrv_get_aio_context(bs));
  }
@@ -XXX,XX +XXX,XX @@ void bdrv_drain_all_begin(void)
              aio_context_acquire(aio_context);
              for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
                  if (aio_context == bdrv_get_aio_context(bs)) {
 -                    waited |= bdrv_drain_recurse(bs);
 +                    waited |= bdrv_drain_recurse(bs, true);
                  }
              }
              aio_context_release(aio_context);
@@ -XXX,XX +XXX,XX @@ void bdrv_drain_all_end(void)
          aio_context_acquire(aio_context);
          aio_enable_external(aio_context);
          bdrv_parent_drained_end(bs);
 +        bdrv_drain_recurse(bs, false);
          aio_context_release(aio_context);
      }
 --
-.13.6
+.24.1

-[Qemu-devel] [PULL 2/3] block: rename bdrv_co_drain to bdrv_co_drain_begin
+Deleted patch
-From: Manos Pitsidianakis <el13635@mail.ntua.gr>
-Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
-Reviewed-by: Fam Zheng <famz@redhat.com>
-Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr>
-Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
----
- include/block/block_int.h | 4 ++--
- block/io.c                | 4 ++--
- block/qed.c               | 6 +++---
-files changed, 7 insertions(+), 7 deletions(-)
-diff --git a/include/block/block_int.h b/include/block/block_int.h
-index XXXXXXX..XXXXXXX 100644
---- a/include/block/block_int.h
-+++ b/include/block/block_int.h
-@@ -XXX,XX +XXX,XX @@ struct BlockDriver {
-     int (*bdrv_probe_geometry)(BlockDriverState *bs, HDGeometry *geo);
-     /**
--     * bdrv_co_drain is called if implemented in the beginning of a
-+     * bdrv_co_drain_begin is called if implemented in the beginning of a
-      * drain operation to drain and stop any internal sources of requests in
-      * the driver.
-      * bdrv_co_drain_end is called if implemented at the end of the drain.
-@@ -XXX,XX +XXX,XX @@ struct BlockDriver {
-      * requests, or toggle an internal state. After the end of the drain new
-      * requests will continue normally.
-      */
--    void coroutine_fn (*bdrv_co_drain)(BlockDriverState *bs);
-+    void coroutine_fn (*bdrv_co_drain_begin)(BlockDriverState *bs);
-     void coroutine_fn (*bdrv_co_drain_end)(BlockDriverState *bs);
-     void (*bdrv_add_child)(BlockDriverState *parent, BlockDriverState *child,
-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 void coroutine_fn bdrv_drain_invoke_entry(void *opaque)
-     BlockDriverState *bs = data->bs;
-     if (data->begin) {
--        bs->drv->bdrv_co_drain(bs);
-+        bs->drv->bdrv_co_drain_begin(bs);
-     } else {
-         bs->drv->bdrv_co_drain_end(bs);
-     }
-@@ -XXX,XX +XXX,XX @@ static void bdrv_drain_invoke(BlockDriverState *bs, bool begin)
- {
-     BdrvCoDrainData data = { .bs = bs, .done = false, .begin = begin};
--    if (!bs->drv || (begin && !bs->drv->bdrv_co_drain) ||
-+    if (!bs->drv || (begin && !bs->drv->bdrv_co_drain_begin) ||
-             (!begin && !bs->drv->bdrv_co_drain_end)) {
-         return;
-     }
-diff --git a/block/qed.c b/block/qed.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/qed.c
-+++ b/block/qed.c
-@@ -XXX,XX +XXX,XX @@ static bool qed_plug_allocating_write_reqs(BDRVQEDState *s)
-     assert(!s->allocating_write_reqs_plugged);
-     if (s->allocating_acb != NULL) {
-         /* Another allocating write came concurrently.  This cannot happen
--         * from bdrv_qed_co_drain, but it can happen when the timer runs.
-+         * from bdrv_qed_co_drain_begin, but it can happen when the timer runs.
-          */
-         qemu_co_mutex_unlock(&s->table_lock);
-         return false;
-@@ -XXX,XX +XXX,XX @@ static void bdrv_qed_attach_aio_context(BlockDriverState *bs,
-     }
- }
--static void coroutine_fn bdrv_qed_co_drain(BlockDriverState *bs)
-+static void coroutine_fn bdrv_qed_co_drain_begin(BlockDriverState *bs)
- {
-     BDRVQEDState *s = bs->opaque;
-@@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_qed = {
-     .bdrv_check               = bdrv_qed_check,
-     .bdrv_detach_aio_context  = bdrv_qed_detach_aio_context,
-     .bdrv_attach_aio_context  = bdrv_qed_attach_aio_context,
--    .bdrv_co_drain            = bdrv_qed_co_drain,
-+    .bdrv_co_drain_begin      = bdrv_qed_co_drain_begin,
- };
- static void bdrv_qed_init(void)
---
-.13.6

-[Qemu-devel] [PULL 3/3] block/throttle.c: add bdrv_co_drain_begin/end callbacks
+[PULL 2/2] hw/core: Allow setting 'virtio-blk-device.scsi' property on OSX host
-From: Manos Pitsidianakis <el13635@mail.ntua.gr>
+From: Philippe Mathieu-Daudé <philmd@redhat.com>
-Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
+Commit ed65fd1a2750 ("virtio-blk: switch off scsi-passthrough by
-Reviewed-by: Fam Zheng <famz@redhat.com>
+default") changed the default value of the 'scsi' property of
-Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr>
+virtio-blk, which is only available on Linux hosts. It also added
 an unconditional compat entry for 2.4 or earlier machines.
 Trying to set this property on a pre-2.5 machine on OSX, we get:
    Unexpected error in object_property_find() at qom/object.c:1201:
    qemu-system-x86_64: -device virtio-blk-pci,id=scsi0,drive=drive0: can't apply global virtio-blk-device.scsi=true: Property '.scsi' not found
 Fix this error by marking the property optional.
 Fixes: ed65fd1a27 ("virtio-blk: switch off scsi-passthrough by default")
 Suggested-by: Cornelia Huck <cohuck@redhat.com>
 Reviewed-by: Cornelia Huck <cohuck@redhat.com>
 Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
 Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
 Message-id: 20200207001404.1739-1-philmd@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/throttle.c | 18 ++++++++++++++++++
+ hw/core/machine.c | 3 ++-
-file changed, 18 insertions(+)
+file changed, 2 insertions(+), 1 deletion(-)
-diff --git a/block/throttle.c b/block/throttle.c
+diff --git a/hw/core/machine.c b/hw/core/machine.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/throttle.c
+--- a/hw/core/machine.c
-+++ b/block/throttle.c
++++ b/hw/core/machine.c
-@@ -XXX,XX +XXX,XX @@ static bool throttle_recurse_is_first_non_filter(BlockDriverState *bs,
+@@ -XXX,XX +XXX,XX @@ GlobalProperty hw_compat_2_5[] = {
-     return bdrv_recurse_is_first_non_filter(bs->file->bs, candidate);
+ const size_t hw_compat_2_5_len = G_N_ELEMENTS(hw_compat_2_5);
- }
+ GlobalProperty hw_compat_2_4[] = {
-+static void coroutine_fn throttle_co_drain_begin(BlockDriverState *bs)
+-    { "virtio-blk-device", "scsi", "true" },
-+{
++    /* Optional because the 'scsi' property is Linux-only */
-+    ThrottleGroupMember *tgm = bs->opaque;
++    { "virtio-blk-device", "scsi", "true", .optional = true },
-+    if (atomic_fetch_inc(&tgm->io_limits_disabled) == 0) {
+     { "e1000", "extra_mac_registers", "off" },
-+        throttle_group_restart_tgm(tgm);
+     { "virtio-pci", "x-disable-pcie", "on" },
-+    }
+     { "virtio-pci", "migrate-extra", "off" },
 +}
 +
 +static void coroutine_fn throttle_co_drain_end(BlockDriverState *bs)
 +{
 +    ThrottleGroupMember *tgm = bs->opaque;
 +    assert(tgm->io_limits_disabled);
 +    atomic_dec(&tgm->io_limits_disabled);
 +}
 +
  static BlockDriver bdrv_throttle = {
      .format_name                        =   "throttle",
      .protocol_name                      =   "throttle",
@@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_throttle = {
      .bdrv_reopen_abort                  =   throttle_reopen_abort,
      .bdrv_co_get_block_status           =   bdrv_co_get_block_status_from_file,
 +    .bdrv_co_drain_begin                =   throttle_co_drain_begin,
 +    .bdrv_co_drain_end                  =   throttle_co_drain_end,
 +
      .is_filter                          =   true,
  };
 --
-.13.6
+.24.1

The following changes since commit f90ea7ba7c5ae7010ee0ce062207ae42530f57d6:

Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20171012' into staging (2017-10-12 17:06:50 +0100)

are available in the git repository at:

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

for you to fetch changes up to b867eaa17b3940760f51134e409cb0580dd3dde3:

block/throttle.c: add bdrv_co_drain_begin/end callbacks (2017-10-13 12:38:41 +0100)

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

Manos Pitsidianakis (3):
  block: add bdrv_co_drain_end callback
  block: rename bdrv_co_drain to bdrv_co_drain_begin
  block/throttle.c: add bdrv_co_drain_begin/end callbacks

include/block/block_int.h | 13 ++++++++++---
 block/io.c                | 48 +++++++++++++++++++++++++++++++++--------------
 block/qed.c               |  6 +++---
 block/throttle.c          | 18 ++++++++++++++++++
 4 files changed, 65 insertions(+), 20 deletions(-)

-- 
2.13.6

From: Manos Pitsidianakis <el13635@mail.ntua.gr>

BlockDriverState has a bdrv_co_drain() callback but no equivalent for
the end of the drain. The throttle driver (block/throttle.c) needs a way
to mark the end of the drain in order to toggle io_limits_disabled
correctly, thus bdrv_co_drain_end is needed.

Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/block/block_int.h | 11 +++++++++--
 block/io.c                | 48 +++++++++++++++++++++++++++++++++--------------
 2 files changed, 43 insertions(+), 16 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -XXX,XX +XXX,XX @@ struct BlockDriver {
     int (*bdrv_probe_geometry)(BlockDriverState *bs, HDGeometry *geo);
 
     /**
-     * Drain and stop any internal sources of requests in the driver, and
-     * remain so until next I/O callback (e.g. bdrv_co_writev) is called.
+     * bdrv_co_drain is called if implemented in the beginning of a
+     * drain operation to drain and stop any internal sources of requests in
+     * the driver.
+     * bdrv_co_drain_end is called if implemented at the end of the drain.
+     *
+     * They should be used by the driver to e.g. manage scheduled I/O
+     * requests, or toggle an internal state. After the end of the drain new
+     * requests will continue normally.
      */
     void coroutine_fn (*bdrv_co_drain)(BlockDriverState *bs);
+    void coroutine_fn (*bdrv_co_drain_end)(BlockDriverState *bs);
 
     void (*bdrv_add_child)(BlockDriverState *parent, BlockDriverState *child,
                            Error **errp);
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 @@ typedef struct {
     Coroutine *co;
     BlockDriverState *bs;
     bool done;
+    bool begin;
 } BdrvCoDrainData;
 
 static void coroutine_fn bdrv_drain_invoke_entry(void *opaque)
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn bdrv_drain_invoke_entry(void *opaque)
     BdrvCoDrainData *data = opaque;
     BlockDriverState *bs = data->bs;
 
-    bs->drv->bdrv_co_drain(bs);
+    if (data->begin) {
+        bs->drv->bdrv_co_drain(bs);
+    } else {
+        bs->drv->bdrv_co_drain_end(bs);
+    }
 
     /* Set data->done before reading bs->wakeup.  */
     atomic_mb_set(&data->done, true);
     bdrv_wakeup(bs);
 }
 
-static void bdrv_drain_invoke(BlockDriverState *bs)
+static void bdrv_drain_invoke(BlockDriverState *bs, bool begin)
 {
-    BdrvCoDrainData data = { .bs = bs, .done = false };
+    BdrvCoDrainData data = { .bs = bs, .done = false, .begin = begin};
 
-    if (!bs->drv || !bs->drv->bdrv_co_drain) {
+    if (!bs->drv || (begin && !bs->drv->bdrv_co_drain) ||
+            (!begin && !bs->drv->bdrv_co_drain_end)) {
         return;
     }
 
@@ -XXX,XX +XXX,XX @@ static void bdrv_drain_invoke(BlockDriverState *bs)
     BDRV_POLL_WHILE(bs, !data.done);
 }
 
-static bool bdrv_drain_recurse(BlockDriverState *bs)
+static bool bdrv_drain_recurse(BlockDriverState *bs, bool begin)
 {
     BdrvChild *child, *tmp;
     bool waited;
 
-    waited = BDRV_POLL_WHILE(bs, atomic_read(&bs->in_flight) > 0);
-
     /* Ensure any pending metadata writes are submitted to bs->file.  */
-    bdrv_drain_invoke(bs);
+    bdrv_drain_invoke(bs, begin);
+
+    /* Wait for drained requests to finish */
+    waited = BDRV_POLL_WHILE(bs, atomic_read(&bs->in_flight) > 0);
 
     QLIST_FOREACH_SAFE(child, &bs->children, next, tmp) {
         BlockDriverState *bs = child->bs;
@@ -XXX,XX +XXX,XX @@ static bool bdrv_drain_recurse(BlockDriverState *bs)
              */
             bdrv_ref(bs);
         }
-        waited |= bdrv_drain_recurse(bs);
+        waited |= bdrv_drain_recurse(bs, begin);
         if (in_main_loop) {
             bdrv_unref(bs);
         }
@@ -XXX,XX +XXX,XX @@ static void bdrv_co_drain_bh_cb(void *opaque)
     BlockDriverState *bs = data->bs;
 
     bdrv_dec_in_flight(bs);
-    bdrv_drained_begin(bs);
+    if (data->begin) {
+        bdrv_drained_begin(bs);
+    } else {
+        bdrv_drained_end(bs);
+    }
+
     data->done = true;
     aio_co_wake(co);
 }
 
-static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs)
+static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs,
+                                                bool begin)
 {
     BdrvCoDrainData data;
 
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs)
         .co = qemu_coroutine_self(),
         .bs = bs,
         .done = false,
+        .begin = begin,
     };
     bdrv_inc_in_flight(bs);
     aio_bh_schedule_oneshot(bdrv_get_aio_context(bs),
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs)
 void bdrv_drained_begin(BlockDriverState *bs)
 {
     if (qemu_in_coroutine()) {
-        bdrv_co_yield_to_drain(bs);
+        bdrv_co_yield_to_drain(bs, true);
         return;
     }
 
@@ -XXX,XX +XXX,XX @@ void bdrv_drained_begin(BlockDriverState *bs)
         bdrv_parent_drained_begin(bs);
     }
 
-    bdrv_drain_recurse(bs);
+    bdrv_drain_recurse(bs, true);
 }
 
 void bdrv_drained_end(BlockDriverState *bs)
 {
+    if (qemu_in_coroutine()) {
+        bdrv_co_yield_to_drain(bs, false);
+        return;
+    }
     assert(bs->quiesce_counter > 0);
     if (atomic_fetch_dec(&bs->quiesce_counter) > 1) {
         return;
     }
 
     bdrv_parent_drained_end(bs);
+    bdrv_drain_recurse(bs, false);
     aio_enable_external(bdrv_get_aio_context(bs));
 }
 
@@ -XXX,XX +XXX,XX @@ void bdrv_drain_all_begin(void)
             aio_context_acquire(aio_context);
             for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
                 if (aio_context == bdrv_get_aio_context(bs)) {
-                    waited |= bdrv_drain_recurse(bs);
+                    waited |= bdrv_drain_recurse(bs, true);
                 }
             }
             aio_context_release(aio_context);
@@ -XXX,XX +XXX,XX @@ void bdrv_drain_all_end(void)
         aio_context_acquire(aio_context);
         aio_enable_external(aio_context);
         bdrv_parent_drained_end(bs);
+        bdrv_drain_recurse(bs, false);
         aio_context_release(aio_context);
     }
 
-- 
2.13.6

From: Manos Pitsidianakis <el13635@mail.ntua.gr>

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/block/block_int.h | 4 ++--
 block/io.c                | 4 ++--
 block/qed.c               | 6 +++---
 3 files changed, 7 insertions(+), 7 deletions(-)

From: Manos Pitsidianakis <el13635@mail.ntua.gr>

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/throttle.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/block/throttle.c b/block/throttle.c
index XXXXXXX..XXXXXXX 100644
--- a/block/throttle.c
+++ b/block/throttle.c
@@ -XXX,XX +XXX,XX @@ static bool throttle_recurse_is_first_non_filter(BlockDriverState *bs,
     return bdrv_recurse_is_first_non_filter(bs->file->bs, candidate);
 }
 
+static void coroutine_fn throttle_co_drain_begin(BlockDriverState *bs)
+{
+    ThrottleGroupMember *tgm = bs->opaque;
+    if (atomic_fetch_inc(&tgm->io_limits_disabled) == 0) {
+        throttle_group_restart_tgm(tgm);
+    }
+}
+
+static void coroutine_fn throttle_co_drain_end(BlockDriverState *bs)
+{
+    ThrottleGroupMember *tgm = bs->opaque;
+    assert(tgm->io_limits_disabled);
+    atomic_dec(&tgm->io_limits_disabled);
+}
+
 static BlockDriver bdrv_throttle = {
     .format_name                        =   "throttle",
     .protocol_name                      =   "throttle",
@@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_throttle = {
     .bdrv_reopen_abort                  =   throttle_reopen_abort,
     .bdrv_co_get_block_status           =   bdrv_co_get_block_status_from_file,
 
+    .bdrv_co_drain_begin                =   throttle_co_drain_begin,
+    .bdrv_co_drain_end                  =   throttle_co_drain_end,
+
     .is_filter                          =   true,
 };
 
-- 
2.13.6

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

Commit 7a3f542fbd "block/io: refactor padding" occasionally dropped
aligning for zero-length request: bdrv_init_padding() blindly return
false if bytes == 0, like there is nothing to align.

This leads the following command to crash:

./qemu-io --image-opts -c 'write 1 0' \
  driver=blkdebug,align=512,image.driver=null-co,image.size=512

>> qemu-io: block/io.c:1955: bdrv_aligned_pwritev: Assertion
    `(offset & (align - 1)) == 0' failed.
>> Aborted (core dumped)

Prior to 7a3f542fbd we does aligning of such zero requests. Instead of
recovering this behavior let's just do nothing on such requests as it
is useless.

Note that driver may have special meaning of zero-length reqeusts, like
qcow2_co_pwritev_compressed_part, so we can't skip any zero-length
operation. But for unaligned ones, we can't pass it to driver anyway.

This commit also fixes crash in iotest 80 running with -nocache:

./check -nocache -qcow2 80

which crashes on same assertion due to trying to read empty extra data
in qcow2_do_read_snapshots().

Cc: qemu-stable@nongnu.org # v4.2
Fixes: 7a3f542fbd
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 20200206164245.17781-1-vsementsov@virtuozzo.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/io.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

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 bool bdrv_init_padding(BlockDriverState *bs,
         pad->tail = align - pad->tail;
     }
 
-    if ((!pad->head && !pad->tail) || !bytes) {
+    if (!pad->head && !pad->tail) {
         return false;
     }
 
+    assert(bytes); /* Nothing good in aligning zero-length requests */
+
     sum = pad->head + bytes + pad->tail;
     pad->buf_len = (sum > align && pad->head && pad->tail) ? 2 * align : align;
     pad->buf = qemu_blockalign(bs, pad->buf_len);
@@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_preadv_part(BdrvChild *child,
         return ret;
     }
 
+    if (bytes == 0 && !QEMU_IS_ALIGNED(offset, bs->bl.request_alignment)) {
+        /*
+         * Aligning zero request is nonsense. Even if driver has special meaning
+         * of zero-length (like qcow2_co_pwritev_compressed_part), we can't pass
+         * it to driver due to request_alignment.
+         *
+         * Still, no reason to return an error if someone do unaligned
+         * zero-length read occasionally.
+         */
+        return 0;
+    }
+
     bdrv_inc_in_flight(bs);
 
     /* Don't do copy-on-read if we read data before write operation */
@@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_pwritev_part(BdrvChild *child,
         return -ENOTSUP;
     }
 
+    if (bytes == 0 && !QEMU_IS_ALIGNED(offset, bs->bl.request_alignment)) {
+        /*
+         * Aligning zero request is nonsense. Even if driver has special meaning
+         * of zero-length (like qcow2_co_pwritev_compressed_part), we can't pass
+         * it to driver due to request_alignment.
+         *
+         * Still, no reason to return an error if someone do unaligned
+         * zero-length write occasionally.
+         */
+        return 0;
+    }
+
     bdrv_inc_in_flight(bs);
     /*
      * Align write if necessary by performing a read-modify-write cycle.
-- 
2.24.1

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Commit ed65fd1a2750 ("virtio-blk: switch off scsi-passthrough by
default") changed the default value of the 'scsi' property of
virtio-blk, which is only available on Linux hosts. It also added
an unconditional compat entry for 2.4 or earlier machines.

Trying to set this property on a pre-2.5 machine on OSX, we get:

Unexpected error in object_property_find() at qom/object.c:1201:
   qemu-system-x86_64: -device virtio-blk-pci,id=scsi0,drive=drive0: can't apply global virtio-blk-device.scsi=true: Property '.scsi' not found

Fix this error by marking the property optional.

Fixes: ed65fd1a27 ("virtio-blk: switch off scsi-passthrough by default")
Suggested-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-id: 20200207001404.1739-1-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/core/machine.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -XXX,XX +XXX,XX @@ GlobalProperty hw_compat_2_5[] = {
 const size_t hw_compat_2_5_len = G_N_ELEMENTS(hw_compat_2_5);
 
 GlobalProperty hw_compat_2_4[] = {
-    { "virtio-blk-device", "scsi", "true" },
+    /* Optional because the 'scsi' property is Linux-only */
+    { "virtio-blk-device", "scsi", "true", .optional = true },
     { "e1000", "extra_mac_registers", "off" },
     { "virtio-pci", "x-disable-pcie", "on" },
     { "virtio-pci", "migrate-extra", "off" },
-- 
2.24.1