Series comparison

-[Qemu-devel] [PULL 0/2] Block patches
+[PULL 0/2] Block patches
-The following changes since commit 36f87b4513373b3cd79c87c9197d17face95d4ac:
+The following changes since commit ca61fa4b803e5d0abaf6f1ceb690f23bb78a4def:
-  Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-2.10-20170630' into staging (2017-06-30 11:58:49 +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:
+are available in the Git repository at:
-  git://github.com/famz/qemu.git tags/block-pull-request
+  https://gitlab.com/stefanha/qemu.git tags/block-pull-request
-for you to fetch changes up to c61e684e44272f2acb2bef34cf2aa234582a73a9:
+for you to fetch changes up to 1cc7eada97914f090125e588497986f6f7900514:
-  block: Exploit BDRV_BLOCK_EOF for larger zero blocks (2017-06-30 21:48:06 +0800)
+  iothread: use IOThreadParamInfo in iothread_[set|get]_param() (2021-10-07 15:29:50 +0100)
 ----------------------------------------------------------------
 Pull request
 ----------------------------------------------------------------
-Hi Peter,
+Stefano Garzarella (2):
   iothread: rename PollParamInfo to IOThreadParamInfo
   iothread: use IOThreadParamInfo in iothread_[set|get]_param()
-Here are Eric Blake's enhancement to block layer API. Thanks!
+ iothread.c | 28 +++++++++++++++-------------
+file changed, 15 insertions(+), 13 deletions(-)
 ----------------------------------------------------------------
 Eric Blake (2):
   block: Add BDRV_BLOCK_EOF to bdrv_get_block_status()
   block: Exploit BDRV_BLOCK_EOF for larger zero blocks
  block/io.c                 | 42 +++++++++++++++++++++++++++++++++---------
  include/block/block.h      |  2 ++
  tests/qemu-iotests/154     |  4 ----
  tests/qemu-iotests/154.out | 12 ++++++------
 files changed, 41 insertions(+), 19 deletions(-)
 --
-.9.4
+.31.1

-[Qemu-devel] [PULL 2/2] block: Exploit BDRV_BLOCK_EOF for larger zero blocks
+[PULL 1/2] iothread: rename PollParamInfo to IOThreadParamInfo
-From: Eric Blake <eblake@redhat.com>
+From: Stefano Garzarella <sgarzare@redhat.com>
-When we have a BDS with unallocated clusters, but asking the status
+Commit 1793ad0247 ("iothread: add aio-max-batch parameter") added
-of its underlying bs->file or backing layer encounters an end-of-file
+a new parameter (aio-max-batch) to IOThread and used PollParamInfo
-condition, we know that the rest of the unallocated area will read as
+structure to handle it.
 zeroes.  However, pre-patch, this required two separate calls to
 bdrv_get_block_status(), as the first call stops at the point where
 the underlying file ends.  Thanks to BDRV_BLOCK_EOF, we can now widen
 the results of the primary status if the secondary status already
 includes BDRV_BLOCK_ZERO.
-In turn, this fixes a TODO mentioned in iotest 154, where we can now
+Since it is not a parameter of the polling mechanism, we rename the
-see that all sectors in a partial cluster at the end of a file read
+structure to a more generic IOThreadParamInfo.
 as zero when coupling the shorter backing file's status along with our
 knowledge that the remaining sectors came from an unallocated cluster.
-Also, note that the loop in bdrv_co_get_block_status_above() had an
+Suggested-by: Kevin Wolf <kwolf@redhat.com>
-inefficent exit: in cases where the active layer sets BDRV_BLOCK_ZERO
+Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
-but does NOT set BDRV_BLOCK_ALLOCATED (namely, where we know we read
+Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
-zeroes merely because our unallocated clusters lie beyond the backing
+Message-id: 20210727145936.147032-2-sgarzare@redhat.com
-file's shorter length), we still ended up probing the backing layer
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-even though we already had a good answer.
+---
  iothread.c | 14 +++++++-------
 file changed, 7 insertions(+), 7 deletions(-)
-Signed-off-by: Eric Blake <eblake@redhat.com>
+diff --git a/iothread.c b/iothread.c
 Message-Id: <20170505021500.19315-3-eblake@redhat.com>
 Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
 Signed-off-by: Fam Zheng <famz@redhat.com>
 ---
  block/io.c                 | 27 ++++++++++++++++++++++-----
  tests/qemu-iotests/154     |  4 ----
  tests/qemu-iotests/154.out | 12 ++++++------
 files changed, 28 insertions(+), 15 deletions(-)
 diff --git a/block/io.c b/block/io.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/io.c
+--- a/iothread.c
-+++ b/block/io.c
++++ b/iothread.c
-@@ -XXX,XX +XXX,XX @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
+@@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp)
-             /* Ignore errors.  This is just providing extra information, it
+ typedef struct {
-              * is useful but not necessary.
+     const char *name;
-              */
+     ptrdiff_t offset; /* field's byte offset in IOThread struct */
--            if (!file_pnum) {
+-} PollParamInfo;
--                /* !file_pnum indicates an offset at or beyond the EOF; it is
++} IOThreadParamInfo;
--                 * perfectly valid for the format block driver to point to such
--                 * offsets, so catch it and mark everything as zero */
+-static PollParamInfo poll_max_ns_info = {
-+            if (ret2 & BDRV_BLOCK_EOF &&
++static IOThreadParamInfo poll_max_ns_info = {
-+                (!file_pnum || ret2 & BDRV_BLOCK_ZERO)) {
+     "poll-max-ns", offsetof(IOThread, poll_max_ns),
-+                /*
+ };
-+                 * It is valid for the format block driver to read
+-static PollParamInfo poll_grow_info = {
-+                 * beyond the end of the underlying file's current
++static IOThreadParamInfo poll_grow_info = {
-+                 * size; such areas read as zero.
+     "poll-grow", offsetof(IOThread, poll_grow),
-+                 */
+ };
-                 ret |= BDRV_BLOCK_ZERO;
+-static PollParamInfo poll_shrink_info = {
-             } else {
++static IOThreadParamInfo poll_shrink_info = {
-                 /* Limit request to the range reported by the protocol driver */
+     "poll-shrink", offsetof(IOThread, poll_shrink),
-@@ -XXX,XX +XXX,XX @@ static int64_t coroutine_fn bdrv_co_get_block_status_above(BlockDriverState *bs,
+ };
 -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)
  {
-     BlockDriverState *p;
+     IOThread *iothread = IOTHREAD(obj);
-     int64_t ret = 0;
+-    PollParamInfo *info = opaque;
-+    bool first = true;
++    IOThreadParamInfo *info = opaque;
+     int64_t *field = (void *)iothread + info->offset;
-     assert(bs != base);
-     for (p = bs; p != base; p = backing_bs(p)) {
+     visit_type_int64(v, name, field, errp);
-         ret = bdrv_co_get_block_status(p, sector_num, nb_sectors, pnum, file);
+@@ -XXX,XX +XXX,XX @@ static bool iothread_set_param(Object *obj, Visitor *v,
--        if (ret < 0 || ret & BDRV_BLOCK_ALLOCATED) {
+         const char *name, void *opaque, Error **errp)
-+        if (ret < 0) {
+ {
-+            break;
+     IOThread *iothread = IOTHREAD(obj);
-+        }
+-    PollParamInfo *info = opaque;
-+        if (ret & BDRV_BLOCK_ZERO && ret & BDRV_BLOCK_EOF && !first) {
++    IOThreadParamInfo *info = opaque;
-+            /*
+     int64_t *field = (void *)iothread + info->offset;
-+             * Reading beyond the end of the file continues to read
+     int64_t value;
-+             * zeroes, but we can only widen the result to the
 +             * unallocated length we learned from an earlier
 +             * iteration.
 +             */
 +            *pnum = nb_sectors;
 +        }
 +        if (ret & (BDRV_BLOCK_ZERO | BDRV_BLOCK_DATA)) {
              break;
          }
          /* [sector_num, pnum] unallocated on this layer, which could be only
           * the first part of [sector_num, nb_sectors].  */
          nb_sectors = MIN(nb_sectors, *pnum);
 +        first = false;
      }
      return ret;
  }
 diff --git a/tests/qemu-iotests/154 b/tests/qemu-iotests/154
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/154
 +++ b/tests/qemu-iotests/154
@@ -XXX,XX +XXX,XX @@ $QEMU_IO -c "alloc $size 2048" "$TEST_IMG" | _filter_qemu_io
  $QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map
  # Repeat with backing file holding unallocated cluster.
 -# TODO: Note that this forces an allocation, because we aren't yet able to
 -# quickly detect that reads beyond EOF of the backing file are always zero
  CLUSTER_SIZE=2048 TEST_IMG="$TEST_IMG.base" _make_test_img $((size + 1024))
  # Write at the front: sector-wise, the request is:
@@ -XXX,XX +XXX,XX @@ $QEMU_IO -c "alloc $size 2048" "$TEST_IMG" | _filter_qemu_io
  $QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map
  # Repeat with backing file holding zero'd cluster
 -# TODO: Note that this forces an allocation, because we aren't yet able to
 -# quickly detect that reads beyond EOF of the backing file are always zero
  $QEMU_IO -c "write -z $size 512" "$TEST_IMG.base" | _filter_qemu_io
  # Write at the front: sector-wise, the request is:
 diff --git a/tests/qemu-iotests/154.out b/tests/qemu-iotests/154.out
 index XXXXXXX..XXXXXXX 100644
 --- a/tests/qemu-iotests/154.out
 +++ b/tests/qemu-iotests/154.out
@@ -XXX,XX +XXX,XX @@ wrote 512/512 bytes at offset 134217728
 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 /2048 bytes allocated at offset 128 MiB
  [{ "start": 0, "length": 134217728, "depth": 1, "zero": true, "data": false},
 -{ "start": 134217728, "length": 2048, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]
 +{ "start": 134217728, "length": 2048, "depth": 0, "zero": true, "data": false}]
  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134219776 backing_file=TEST_DIR/t.IMGFMT.base
  wrote 512/512 bytes at offset 134219264
 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 /2048 bytes allocated at offset 128 MiB
  [{ "start": 0, "length": 134217728, "depth": 1, "zero": true, "data": false},
 -{ "start": 134217728, "length": 2048, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]
 +{ "start": 134217728, "length": 2048, "depth": 0, "zero": true, "data": false}]
  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134219776 backing_file=TEST_DIR/t.IMGFMT.base
  wrote 1024/1024 bytes at offset 134218240
 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 /2048 bytes allocated at offset 128 MiB
  [{ "start": 0, "length": 134217728, "depth": 1, "zero": true, "data": false},
 -{ "start": 134217728, "length": 2048, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]
 +{ "start": 134217728, "length": 2048, "depth": 0, "zero": true, "data": false}]
  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134219776 backing_file=TEST_DIR/t.IMGFMT.base
  wrote 2048/2048 bytes at offset 134217728
 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
@@ -XXX,XX +XXX,XX @@ wrote 512/512 bytes at offset 134217728
 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 /2048 bytes allocated at offset 128 MiB
  [{ "start": 0, "length": 134217728, "depth": 1, "zero": true, "data": false},
 -{ "start": 134217728, "length": 2048, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]
 +{ "start": 134217728, "length": 2048, "depth": 0, "zero": true, "data": false}]
  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134219776 backing_file=TEST_DIR/t.IMGFMT.base
  wrote 512/512 bytes at offset 134219264
 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 /2048 bytes allocated at offset 128 MiB
  [{ "start": 0, "length": 134217728, "depth": 1, "zero": true, "data": false},
 -{ "start": 134217728, "length": 2048, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]
 +{ "start": 134217728, "length": 2048, "depth": 0, "zero": true, "data": false}]
  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134219776 backing_file=TEST_DIR/t.IMGFMT.base
  wrote 1024/1024 bytes at offset 134218240
 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 /2048 bytes allocated at offset 128 MiB
  [{ "start": 0, "length": 134217728, "depth": 1, "zero": true, "data": false},
 -{ "start": 134217728, "length": 2048, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]
 +{ "start": 134217728, "length": 2048, "depth": 0, "zero": true, "data": false}]
  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134219776 backing_file=TEST_DIR/t.IMGFMT.base
  wrote 2048/2048 bytes at offset 134217728
 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 --
-.9.4
+.31.1

-[Qemu-devel] [PULL 1/2] block: Add BDRV_BLOCK_EOF to bdrv_get_block_status()
+[PULL 2/2] iothread: use IOThreadParamInfo in iothread_[set|get]_param()
-From: Eric Blake <eblake@redhat.com>
+From: Stefano Garzarella <sgarzare@redhat.com>
-Just as the block layer already sets BDRV_BLOCK_ALLOCATED as a
+Commit 0445409d74 ("iothread: generalize
-shortcut for subsequent operations, there are also some optimizations
+iothread_set_param/iothread_get_param") moved common code to set and
-that are made easier if we can quickly tell that *pnum will advance
+get IOThread parameters in two new functions.
 us to the end of a file, via a new BDRV_BLOCK_EOF which gets set
 by the block layer.
-This just plumbs up the new bit; subsequent patches will make use
+These functions are called inside callbacks, so we don't need to use an
-of it.
+opaque pointer. Let's replace `void *opaque` parameter with
 `IOThreadParamInfo *info`.
-Signed-off-by: Eric Blake <eblake@redhat.com>
+Suggested-by: Kevin Wolf <kwolf@redhat.com>
-Message-Id: <20170505021500.19315-2-eblake@redhat.com>
+Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
-Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
+Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
-Signed-off-by: Fam Zheng <famz@redhat.com>
+Message-id: 20210727145936.147032-3-sgarzare@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/io.c            | 15 +++++++++++----
+ iothread.c | 18 ++++++++++--------
- include/block/block.h |  2 ++
+file changed, 10 insertions(+), 8 deletions(-)
 files changed, 13 insertions(+), 4 deletions(-)
-diff --git a/block/io.c b/block/io.c
+diff --git a/iothread.c b/iothread.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/io.c
+--- a/iothread.c
-+++ b/block/io.c
++++ b/iothread.c
-@@ -XXX,XX +XXX,XX @@ typedef struct BdrvCoGetBlockStatusData {
+@@ -XXX,XX +XXX,XX @@ static IOThreadParamInfo aio_max_batch_info = {
-  * Drivers not implementing the functionality are assumed to not support
+ };
-  * backing files, hence all their sectors are reported as allocated.
-  *
+ static void iothread_get_param(Object *obj, Visitor *v,
-- * If 'sector_num' is beyond the end of the disk image the return value is 0
+-        const char *name, void *opaque, Error **errp)
-- * and 'pnum' is set to 0.
++        const char *name, IOThreadParamInfo *info, Error **errp)
-+ * If 'sector_num' is beyond the end of the disk image the return value is
+ {
-+ * BDRV_BLOCK_EOF and 'pnum' is set to 0.
+     IOThread *iothread = IOTHREAD(obj);
-  *
+-    IOThreadParamInfo *info = opaque;
-  * 'pnum' is set to the number of sectors (including and immediately following
+     int64_t *field = (void *)iothread + info->offset;
-  * the specified sector) that are known to be in the same
-  * allocated/unallocated state.
+     visit_type_int64(v, name, field, errp);
-  *
+ }
-  * 'nb_sectors' is the max value 'pnum' should be set to.  If nb_sectors goes
-- * beyond the end of the disk image it will be clamped.
+ static bool iothread_set_param(Object *obj, Visitor *v,
-+ * beyond the end of the disk image it will be clamped; if 'pnum' is set to
+-        const char *name, void *opaque, Error **errp)
-+ * the end of the image, then the returned value will include BDRV_BLOCK_EOF.
++        const char *name, IOThreadParamInfo *info, Error **errp)
-  *
+ {
-  * If returned value is positive and BDRV_BLOCK_OFFSET_VALID bit is set, 'file'
+     IOThread *iothread = IOTHREAD(obj);
-  * points to the BDS which the sector range is allocated in.
+-    IOThreadParamInfo *info = opaque;
-@@ -XXX,XX +XXX,XX @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
+     int64_t *field = (void *)iothread + info->offset;
+     int64_t value;
-     if (sector_num >= total_sectors) {
-         *pnum = 0;
+@@ -XXX,XX +XXX,XX @@ static bool iothread_set_param(Object *obj, Visitor *v,
--        return 0;
+ static void iothread_get_poll_param(Object *obj, Visitor *v,
-+        return BDRV_BLOCK_EOF;
+         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;
      }
-     n = total_sectors - sector_num;
+@@ -XXX,XX +XXX,XX @@ static void iothread_set_poll_param(Object *obj, Visitor *v,
-@@ -XXX,XX +XXX,XX @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
+ static void iothread_get_aio_param(Object *obj, Visitor *v,
-     if (!bs->drv->bdrv_co_get_block_status) {
+         const char *name, void *opaque, Error **errp)
-         *pnum = nb_sectors;
+ {
-         ret = BDRV_BLOCK_DATA | BDRV_BLOCK_ALLOCATED;
++    IOThreadParamInfo *info = opaque;
-+        if (sector_num + nb_sectors == total_sectors) {
-+            ret |= BDRV_BLOCK_EOF;
+-    iothread_get_param(obj, v, name, opaque, errp);
-+        }
++    iothread_get_param(obj, v, name, info, errp);
          if (bs->drv->protocol_name) {
              ret |= BDRV_BLOCK_OFFSET_VALID | (sector_num * BDRV_SECTOR_SIZE);
          }
@@ -XXX,XX +XXX,XX @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
  out:
      bdrv_dec_in_flight(bs);
 +    if (ret >= 0 && sector_num + *pnum == total_sectors) {
 +        ret |= BDRV_BLOCK_EOF;
 +    }
      return ret;
  }
-diff --git a/include/block/block.h b/include/block/block.h
+ static void iothread_set_aio_param(Object *obj, Visitor *v,
-index XXXXXXX..XXXXXXX 100644
+         const char *name, void *opaque, Error **errp)
---- a/include/block/block.h
+ {
-+++ b/include/block/block.h
+     IOThread *iothread = IOTHREAD(obj);
-@@ -XXX,XX +XXX,XX @@ typedef struct HDGeometry {
++    IOThreadParamInfo *info = opaque;
-  * BDRV_BLOCK_OFFSET_VALID: an associated offset exists for accessing raw data
-  * BDRV_BLOCK_ALLOCATED: the content of the block is determined by this
+-    if (!iothread_set_param(obj, v, name, opaque, errp)) {
-  *                       layer (short for DATA || ZERO), set by block layer
++    if (!iothread_set_param(obj, v, name, info, errp)) {
-+ * BDRV_BLOCK_EOF: the returned pnum covers through end of file for this layer
+         return;
-  *
+     }
-  * Internal flag:
   * BDRV_BLOCK_RAW: used internally to indicate that the request was
@@ -XXX,XX +XXX,XX @@ typedef struct HDGeometry {
  #define BDRV_BLOCK_OFFSET_VALID 0x04
  #define BDRV_BLOCK_RAW          0x08
  #define BDRV_BLOCK_ALLOCATED    0x10
 +#define BDRV_BLOCK_EOF          0x20
  #define BDRV_BLOCK_OFFSET_MASK  BDRV_SECTOR_MASK
  typedef QSIMPLEQ_HEAD(BlockReopenQueue, BlockReopenQueueEntry) BlockReopenQueue;
 --
-.9.4
+.31.1

The following changes since commit 36f87b4513373b3cd79c87c9197d17face95d4ac:

Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-2.10-20170630' into staging (2017-06-30 11:58:49 +0100)

are available in the git repository at:

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

for you to fetch changes up to c61e684e44272f2acb2bef34cf2aa234582a73a9:

block: Exploit BDRV_BLOCK_EOF for larger zero blocks (2017-06-30 21:48:06 +0800)

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

Hi Peter,

Here are Eric Blake's enhancement to block layer API. Thanks!

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

Eric Blake (2):
  block: Add BDRV_BLOCK_EOF to bdrv_get_block_status()
  block: Exploit BDRV_BLOCK_EOF for larger zero blocks

block/io.c                 | 42 +++++++++++++++++++++++++++++++++---------
 include/block/block.h      |  2 ++
 tests/qemu-iotests/154     |  4 ----
 tests/qemu-iotests/154.out | 12 ++++++------
 4 files changed, 41 insertions(+), 19 deletions(-)

-- 
2.9.4

From: Eric Blake <eblake@redhat.com>

Just as the block layer already sets BDRV_BLOCK_ALLOCATED as a
shortcut for subsequent operations, there are also some optimizations
that are made easier if we can quickly tell that *pnum will advance
us to the end of a file, via a new BDRV_BLOCK_EOF which gets set
by the block layer.

This just plumbs up the new bit; subsequent patches will make use
of it.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20170505021500.19315-2-eblake@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/io.c            | 15 +++++++++++----
 include/block/block.h |  2 ++
 2 files changed, 13 insertions(+), 4 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 @@ typedef struct BdrvCoGetBlockStatusData {
  * Drivers not implementing the functionality are assumed to not support
  * backing files, hence all their sectors are reported as allocated.
  *
- * If 'sector_num' is beyond the end of the disk image the return value is 0
- * and 'pnum' is set to 0.
+ * If 'sector_num' is beyond the end of the disk image the return value is
+ * BDRV_BLOCK_EOF and 'pnum' is set to 0.
  *
  * 'pnum' is set to the number of sectors (including and immediately following
  * the specified sector) that are known to be in the same
  * allocated/unallocated state.
  *
  * 'nb_sectors' is the max value 'pnum' should be set to.  If nb_sectors goes
- * beyond the end of the disk image it will be clamped.
+ * beyond the end of the disk image it will be clamped; if 'pnum' is set to
+ * the end of the image, then the returned value will include BDRV_BLOCK_EOF.
  *
  * If returned value is positive and BDRV_BLOCK_OFFSET_VALID bit is set, 'file'
  * points to the BDS which the sector range is allocated in.
@@ -XXX,XX +XXX,XX @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
 
     if (sector_num >= total_sectors) {
         *pnum = 0;
-        return 0;
+        return BDRV_BLOCK_EOF;
     }
 
     n = total_sectors - sector_num;
@@ -XXX,XX +XXX,XX @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
     if (!bs->drv->bdrv_co_get_block_status) {
         *pnum = nb_sectors;
         ret = BDRV_BLOCK_DATA | BDRV_BLOCK_ALLOCATED;
+        if (sector_num + nb_sectors == total_sectors) {
+            ret |= BDRV_BLOCK_EOF;
+        }
         if (bs->drv->protocol_name) {
             ret |= BDRV_BLOCK_OFFSET_VALID | (sector_num * BDRV_SECTOR_SIZE);
         }
@@ -XXX,XX +XXX,XX @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
 
 out:
     bdrv_dec_in_flight(bs);
+    if (ret >= 0 && sector_num + *pnum == total_sectors) {
+        ret |= BDRV_BLOCK_EOF;
+    }
     return ret;
 }
 
diff --git a/include/block/block.h b/include/block/block.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -XXX,XX +XXX,XX @@ typedef struct HDGeometry {
  * BDRV_BLOCK_OFFSET_VALID: an associated offset exists for accessing raw data
  * BDRV_BLOCK_ALLOCATED: the content of the block is determined by this
  *                       layer (short for DATA || ZERO), set by block layer
+ * BDRV_BLOCK_EOF: the returned pnum covers through end of file for this layer
  *
  * Internal flag:
  * BDRV_BLOCK_RAW: used internally to indicate that the request was
@@ -XXX,XX +XXX,XX @@ typedef struct HDGeometry {
 #define BDRV_BLOCK_OFFSET_VALID 0x04
 #define BDRV_BLOCK_RAW          0x08
 #define BDRV_BLOCK_ALLOCATED    0x10
+#define BDRV_BLOCK_EOF          0x20
 #define BDRV_BLOCK_OFFSET_MASK  BDRV_SECTOR_MASK
 
 typedef QSIMPLEQ_HEAD(BlockReopenQueue, BlockReopenQueueEntry) BlockReopenQueue;
-- 
2.9.4

From: Eric Blake <eblake@redhat.com>

When we have a BDS with unallocated clusters, but asking the status
of its underlying bs->file or backing layer encounters an end-of-file
condition, we know that the rest of the unallocated area will read as
zeroes.  However, pre-patch, this required two separate calls to
bdrv_get_block_status(), as the first call stops at the point where
the underlying file ends.  Thanks to BDRV_BLOCK_EOF, we can now widen
the results of the primary status if the secondary status already
includes BDRV_BLOCK_ZERO.

In turn, this fixes a TODO mentioned in iotest 154, where we can now
see that all sectors in a partial cluster at the end of a file read
as zero when coupling the shorter backing file's status along with our
knowledge that the remaining sectors came from an unallocated cluster.

Also, note that the loop in bdrv_co_get_block_status_above() had an
inefficent exit: in cases where the active layer sets BDRV_BLOCK_ZERO
but does NOT set BDRV_BLOCK_ALLOCATED (namely, where we know we read
zeroes merely because our unallocated clusters lie beyond the backing
file's shorter length), we still ended up probing the backing layer
even though we already had a good answer.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20170505021500.19315-3-eblake@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/io.c                 | 27 ++++++++++++++++++++++-----
 tests/qemu-iotests/154     |  4 ----
 tests/qemu-iotests/154.out | 12 ++++++------
 3 files changed, 28 insertions(+), 15 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 int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
             /* Ignore errors.  This is just providing extra information, it
              * is useful but not necessary.
              */
-            if (!file_pnum) {
-                /* !file_pnum indicates an offset at or beyond the EOF; it is
-                 * perfectly valid for the format block driver to point to such
-                 * offsets, so catch it and mark everything as zero */
+            if (ret2 & BDRV_BLOCK_EOF &&
+                (!file_pnum || ret2 & BDRV_BLOCK_ZERO)) {
+                /*
+                 * It is valid for the format block driver to read
+                 * beyond the end of the underlying file's current
+                 * size; such areas read as zero.
+                 */
                 ret |= BDRV_BLOCK_ZERO;
             } else {
                 /* Limit request to the range reported by the protocol driver */
@@ -XXX,XX +XXX,XX @@ static int64_t coroutine_fn bdrv_co_get_block_status_above(BlockDriverState *bs,
 {
     BlockDriverState *p;
     int64_t ret = 0;
+    bool first = true;
 
     assert(bs != base);
     for (p = bs; p != base; p = backing_bs(p)) {
         ret = bdrv_co_get_block_status(p, sector_num, nb_sectors, pnum, file);
-        if (ret < 0 || ret & BDRV_BLOCK_ALLOCATED) {
+        if (ret < 0) {
+            break;
+        }
+        if (ret & BDRV_BLOCK_ZERO && ret & BDRV_BLOCK_EOF && !first) {
+            /*
+             * Reading beyond the end of the file continues to read
+             * zeroes, but we can only widen the result to the
+             * unallocated length we learned from an earlier
+             * iteration.
+             */
+            *pnum = nb_sectors;
+        }
+        if (ret & (BDRV_BLOCK_ZERO | BDRV_BLOCK_DATA)) {
             break;
         }
         /* [sector_num, pnum] unallocated on this layer, which could be only
          * the first part of [sector_num, nb_sectors].  */
         nb_sectors = MIN(nb_sectors, *pnum);
+        first = false;
     }
     return ret;
 }
diff --git a/tests/qemu-iotests/154 b/tests/qemu-iotests/154
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/154
+++ b/tests/qemu-iotests/154
@@ -XXX,XX +XXX,XX @@ $QEMU_IO -c "alloc $size 2048" "$TEST_IMG" | _filter_qemu_io
 $QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map
 
 # Repeat with backing file holding unallocated cluster.
-# TODO: Note that this forces an allocation, because we aren't yet able to
-# quickly detect that reads beyond EOF of the backing file are always zero
 CLUSTER_SIZE=2048 TEST_IMG="$TEST_IMG.base" _make_test_img $((size + 1024))
 
 # Write at the front: sector-wise, the request is:
@@ -XXX,XX +XXX,XX @@ $QEMU_IO -c "alloc $size 2048" "$TEST_IMG" | _filter_qemu_io
 $QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map
 
 # Repeat with backing file holding zero'd cluster
-# TODO: Note that this forces an allocation, because we aren't yet able to
-# quickly detect that reads beyond EOF of the backing file are always zero
 $QEMU_IO -c "write -z $size 512" "$TEST_IMG.base" | _filter_qemu_io
 
 # Write at the front: sector-wise, the request is:
diff --git a/tests/qemu-iotests/154.out b/tests/qemu-iotests/154.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/154.out
+++ b/tests/qemu-iotests/154.out
@@ -XXX,XX +XXX,XX @@ wrote 512/512 bytes at offset 134217728
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 2048/2048 bytes allocated at offset 128 MiB
 [{ "start": 0, "length": 134217728, "depth": 1, "zero": true, "data": false},
-{ "start": 134217728, "length": 2048, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]
+{ "start": 134217728, "length": 2048, "depth": 0, "zero": true, "data": false}]
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134219776 backing_file=TEST_DIR/t.IMGFMT.base
 wrote 512/512 bytes at offset 134219264
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 2048/2048 bytes allocated at offset 128 MiB
 [{ "start": 0, "length": 134217728, "depth": 1, "zero": true, "data": false},
-{ "start": 134217728, "length": 2048, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]
+{ "start": 134217728, "length": 2048, "depth": 0, "zero": true, "data": false}]
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134219776 backing_file=TEST_DIR/t.IMGFMT.base
 wrote 1024/1024 bytes at offset 134218240
 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 2048/2048 bytes allocated at offset 128 MiB
 [{ "start": 0, "length": 134217728, "depth": 1, "zero": true, "data": false},
-{ "start": 134217728, "length": 2048, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]
+{ "start": 134217728, "length": 2048, "depth": 0, "zero": true, "data": false}]
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134219776 backing_file=TEST_DIR/t.IMGFMT.base
 wrote 2048/2048 bytes at offset 134217728
 2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
@@ -XXX,XX +XXX,XX @@ wrote 512/512 bytes at offset 134217728
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 2048/2048 bytes allocated at offset 128 MiB
 [{ "start": 0, "length": 134217728, "depth": 1, "zero": true, "data": false},
-{ "start": 134217728, "length": 2048, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]
+{ "start": 134217728, "length": 2048, "depth": 0, "zero": true, "data": false}]
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134219776 backing_file=TEST_DIR/t.IMGFMT.base
 wrote 512/512 bytes at offset 134219264
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 2048/2048 bytes allocated at offset 128 MiB
 [{ "start": 0, "length": 134217728, "depth": 1, "zero": true, "data": false},
-{ "start": 134217728, "length": 2048, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]
+{ "start": 134217728, "length": 2048, "depth": 0, "zero": true, "data": false}]
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134219776 backing_file=TEST_DIR/t.IMGFMT.base
 wrote 1024/1024 bytes at offset 134218240
 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 2048/2048 bytes allocated at offset 128 MiB
 [{ "start": 0, "length": 134217728, "depth": 1, "zero": true, "data": false},
-{ "start": 134217728, "length": 2048, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]
+{ "start": 134217728, "length": 2048, "depth": 0, "zero": true, "data": false}]
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134219776 backing_file=TEST_DIR/t.IMGFMT.base
 wrote 2048/2048 bytes at offset 134217728
 2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-- 
2.9.4

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