All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] block: measure for commit
@ 2026-09-15 14:04 Jean-Louis Dupond
  2026-09-15 14:04 ` [PATCH v2 1/3] block: add bdrv_open_backing_chain_until Jean-Louis Dupond
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Jean-Louis Dupond @ 2026-09-15 14:04 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Pierrick Bouvier, John Snow,
	Vladimir Sementsov-Ogievskiy, Hanna Reitz, Fiona Ebner,
	qemu-block, Jean-Louis Dupond

In some cases we want to calculate the size an image will be after we commit A into B.
This for example on block storage, where you need to resize the target storage before you start the merge.

These commits adds the functionality to do this.
First of all a commit that adds bdrv_open_backing_chain_until. So we can open an image up to base image B.

And then a modified qcow2_measure that calculates the size of B after A has been merged into it.
This with taking into account the discard-no-unref setting of the base.

Jean-Louis Dupond (3):
  block: add bdrv_open_backing_chain_until
  block: support measure for commit
  iotests/290: add test case for qemu-img measure

 block.c                            | 77 +++++++++++++++++++++++-----
 block/crypto.c                     |  1 +
 block/mirror.c                     |  2 +-
 block/qcow2.c                      | 82 ++++++++++++++++++++++++++----
 block/raw-format.c                 |  2 +-
 docs/tools/qemu-img.rst            |  9 +++-
 include/block/block-global-state.h |  7 ++-
 include/block/block-io.h           |  3 +-
 include/block/block_int-common.h   |  2 +-
 qemu-img-cmds.hx                   |  4 +-
 qemu-img.c                         | 45 +++++++++++++---
 tests/qemu-iotests/178.out.qcow2   |  6 +--
 tests/qemu-iotests/178.out.raw     |  6 +--
 tests/qemu-iotests/290             | 45 ++++++++++++++++
 tests/qemu-iotests/290.out         | 61 ++++++++++++++++++++++
 15 files changed, 310 insertions(+), 42 deletions(-)

-- 
2.55.0



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/3] block: add bdrv_open_backing_chain_until
  2026-09-15 14:04 [PATCH v2 0/3] block: measure for commit Jean-Louis Dupond
@ 2026-09-15 14:04 ` Jean-Louis Dupond
  2026-10-30 17:25   ` Kevin Wolf
  2026-09-15 14:04 ` [PATCH v2 2/3] block: support measure for commit Jean-Louis Dupond
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Jean-Louis Dupond @ 2026-09-15 14:04 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Pierrick Bouvier, John Snow,
	Vladimir Sementsov-Ogievskiy, Hanna Reitz, Fiona Ebner,
	qemu-block, Jean-Louis Dupond

Add a new function called bdrv_open_backing_chain_until.
This is used to open a block up until a specified backing chain.

Modified bdrv_open_inherit to accept a new arg
'open_backing_recursively' to force to not open the next backing.
Adjusting all references to keep the existing behaviour.

Signed-off-by: Jean-Louis Dupond <jean-louis@dupond.be>
---
 block.c                            | 72 +++++++++++++++++++++++++-----
 block/mirror.c                     |  2 +-
 include/block/block-global-state.h |  7 ++-
 3 files changed, 69 insertions(+), 12 deletions(-)

diff --git a/block.c b/block.c
index f0a6042e61..9763137d8c 100644
--- a/block.c
+++ b/block.c
@@ -83,6 +83,7 @@ static QLIST_HEAD(, BlockDriver) bdrv_drivers =
 static BlockDriverState *bdrv_open_inherit(const char *filename,
                                            const char *reference,
                                            QDict *options, int flags,
+                                           bool open_backing_recursively,
                                            BlockDriverState *parent,
                                            const BdrvChildClass *child_class,
                                            BdrvChildRole child_role,
@@ -3617,6 +3618,7 @@ out:
  * TODO Can this be unified with bdrv_open_image()?
  */
 int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
+                           bool open_backing_recursively,
                            const char *bdref_key, Error **errp)
 {
     ERRP_GUARD();
@@ -3695,9 +3697,9 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
         qdict_put_str(options, "driver", bs->backing_format);
     }
 
-    backing_hd = bdrv_open_inherit(backing_filename, reference, options, 0, bs,
-                                   &child_of_bds, bdrv_backing_role(bs), true,
-                                   errp);
+    backing_hd = bdrv_open_inherit(backing_filename, reference, options, 0,
+                                   open_backing_recursively, bs, &child_of_bds,
+                                   bdrv_backing_role(bs), true, errp);
     if (!backing_hd) {
         bs->open_flags |= BDRV_O_NO_BACKING;
         error_prepend(errp, "Could not open backing file: ");
@@ -3733,6 +3735,54 @@ free_exit:
     return ret;
 }
 
+int
+bdrv_open_backing_chain_until(BlockDriverState *top_bs,
+                              const char *base_filename,
+                              Error **errp)
+{
+    BlockDriverState *curr = top_bs;
+    int ret;
+
+    GLOBAL_STATE_CODE();
+
+    if (!base_filename) {
+        return 0;
+    }
+
+    if (bdrv_find_backing_image(top_bs, base_filename)) {
+        return 0;
+    }
+
+    while (true) {
+        QDict *options = qdict_clone_shallow(curr->options);
+
+        ret = bdrv_open_backing_file(curr, options, false, "backing", errp);
+        qobject_unref(options);
+        if (ret < 0) {
+            return ret;
+        }
+
+        bdrv_graph_rdlock_main_loop();
+        if (!curr->backing) {
+            bdrv_graph_rdunlock_main_loop();
+            error_setg(errp,
+                       "Did not find '%s' in the backing chain of '%s'",
+                       base_filename, top_bs->filename);
+            return -ENOENT;
+        }
+        bdrv_graph_rdunlock_main_loop();
+
+        if (bdrv_find_backing_image(curr, base_filename)) {
+            return 0;
+        }
+
+        /* Switch to the next layer */
+        bdrv_graph_rdlock_main_loop();
+        curr = curr->backing->bs;
+        bdrv_graph_rdunlock_main_loop();
+    }
+}
+
 static BlockDriverState *
 bdrv_open_child_bs(const char *filename, QDict *options, const char *bdref_key,
                    BlockDriverState *parent, const BdrvChildClass *child_class,
@@ -3767,7 +3817,7 @@ bdrv_open_child_bs(const char *filename, QDict *options, const char *bdref_key,
         goto done;
     }
 
-    bs = bdrv_open_inherit(filename, reference, image_options, 0,
+    bs = bdrv_open_inherit(filename, reference, image_options, 0, true,
                            parent, child_class, child_role, parse_filename,
                            errp);
     if (!bs) {
@@ -3897,8 +3947,8 @@ BlockDriverState *bdrv_open_blockdev_ref(BlockdevRef *ref, Error **errp)
 
     }
 
-    bs = bdrv_open_inherit(NULL, reference, qdict, 0, NULL, NULL, 0, false,
-                           errp);
+    bs = bdrv_open_inherit(NULL, reference, qdict, 0, true, NULL, NULL, 0,
+                           false, errp);
     obj = NULL;
     qobject_unref(obj);
     visit_free(v);
@@ -3986,7 +4036,8 @@ out:
  */
 static BlockDriverState * no_coroutine_fn
 bdrv_open_inherit(const char *filename, const char *reference, QDict *options,
-                  int flags, BlockDriverState *parent,
+                  int flags, bool open_backing_recursively,
+                  BlockDriverState *parent,
                   const BdrvChildClass *child_class, BdrvChildRole child_role,
                   bool parse_filename, Error **errp)
 {
@@ -4199,8 +4250,9 @@ bdrv_open_inherit(const char *filename, const char *reference, QDict *options,
     }
 
     /* If there is a backing file, use it */
-    if ((flags & BDRV_O_NO_BACKING) == 0) {
-        ret = bdrv_open_backing_file(bs, options, "backing", &local_err);
+    if ((flags & BDRV_O_NO_BACKING) == 0 && open_backing_recursively) {
+        ret = bdrv_open_backing_file(bs, options, open_backing_recursively,
+                                     "backing", &local_err);
         if (ret < 0) {
             goto close_and_fail;
         }
@@ -4283,7 +4335,7 @@ BlockDriverState *bdrv_open(const char *filename, const char *reference,
 {
     GLOBAL_STATE_CODE();
 
-    return bdrv_open_inherit(filename, reference, options, flags, NULL,
+    return bdrv_open_inherit(filename, reference, options, flags, true, NULL,
                              NULL, 0, true, errp);
 }
 
diff --git a/block/mirror.c b/block/mirror.c
index 089856f4a8..a4dde5d36d 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -785,7 +785,7 @@ static int mirror_exit_common(Job *job)
         bdrv_graph_rdlock_main_loop();
         assert(!bdrv_backing_chain_next(target_bs));
         ret = bdrv_open_backing_file(bdrv_skip_filters(target_bs), NULL,
-                                     "backing", &local_err);
+                                     true, "backing", &local_err);
         bdrv_graph_rdunlock_main_loop();
         if (ret < 0) {
             error_report_err(local_err);
diff --git a/include/block/block-global-state.h b/include/block/block-global-state.h
index ed89999f0f..e842bed45a 100644
--- a/include/block/block-global-state.h
+++ b/include/block/block-global-state.h
@@ -110,7 +110,12 @@ bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
                     Error **errp);
 
 int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
-                           const char *bdref_key, Error **errp);
+                           bool open_backing_recursively, const char *bdref_key,
+                           Error **errp);
+
+int bdrv_open_backing_chain_until(BlockDriverState *top_bs,
+                                  const char *base_filename,
+                                  Error **errp);
 
 BlockDriverState * no_coroutine_fn
 bdrv_open(const char *filename, const char *reference, QDict *options,
-- 
2.55.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/3] block: support measure for commit
  2026-09-15 14:04 [PATCH v2 0/3] block: measure for commit Jean-Louis Dupond
  2026-09-15 14:04 ` [PATCH v2 1/3] block: add bdrv_open_backing_chain_until Jean-Louis Dupond
@ 2026-09-15 14:04 ` Jean-Louis Dupond
  2026-10-30 19:06   ` Kevin Wolf
  2026-09-15 14:04 ` [PATCH v2 3/3] iotests/290: add test case for qemu-img measure Jean-Louis Dupond
  2026-10-13 11:10 ` [PATCH v2 0/3] block: measure for commit Jean-Louis Dupond
  3 siblings, 1 reply; 8+ messages in thread
From: Jean-Louis Dupond @ 2026-09-15 14:04 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Pierrick Bouvier, John Snow,
	Vladimir Sementsov-Ogievskiy, Hanna Reitz, Fiona Ebner,
	qemu-block, Jean-Louis Dupond

Add a new option to qemu-img measure to specify the base node you want
to merge the image into.
This will open all the images between top and base, and calculate the
size required for merging this layer into the base image.

Also modify the calculation so it takes into account the
discard-no-unref setting of the base image.
If discard-no-unref is enabled on the base, discarded blocks in the
layers above the base will not free clusters in the base image, but will
only mark them ZERO.

[1]: https://gitlab.com/qemu-project/qemu/-/issues/2369

Signed-off-by: Jean-Louis Dupond <jean-louis@dupond.be>
---
 block.c                          |  5 +-
 block/crypto.c                   |  1 +
 block/qcow2.c                    | 82 ++++++++++++++++++++++++++++----
 block/raw-format.c               |  2 +-
 docs/tools/qemu-img.rst          |  9 +++-
 include/block/block-io.h         |  3 +-
 include/block/block_int-common.h |  2 +-
 qemu-img-cmds.hx                 |  4 +-
 qemu-img.c                       | 45 +++++++++++++++---
 tests/qemu-iotests/178.out.qcow2 |  6 +--
 tests/qemu-iotests/178.out.raw   |  6 +--
 11 files changed, 135 insertions(+), 30 deletions(-)

diff --git a/block.c b/block.c
index 9763137d8c..acc77777ed 100644
--- a/block.c
+++ b/block.c
@@ -6091,7 +6091,8 @@ int64_t coroutine_fn bdrv_co_get_allocated_file_size(BlockDriverState *bs)
  * If an error occurs the @errp pointer is set.
  */
 BlockMeasureInfo *bdrv_measure(BlockDriver *drv, QemuOpts *opts,
-                               BlockDriverState *in_bs, Error **errp)
+                               BlockDriverState *in_bs,
+                               BlockDriverState *base_bs, Error **errp)
 {
     IO_CODE();
     if (!drv->bdrv_measure) {
@@ -6100,7 +6101,7 @@ BlockMeasureInfo *bdrv_measure(BlockDriver *drv, QemuOpts *opts,
         return NULL;
     }
 
-    return drv->bdrv_measure(opts, in_bs, errp);
+    return drv->bdrv_measure(opts, in_bs, base_bs, errp);
 }
 
 /**
diff --git a/block/crypto.c b/block/crypto.c
index 36abb7af46..e0236b3382 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -634,6 +634,7 @@ block_crypto_co_getlength(BlockDriverState *bs)
 
 static BlockMeasureInfo *block_crypto_measure(QemuOpts *opts,
                                               BlockDriverState *in_bs,
+                                              BlockDriverState *base_bs,
                                               Error **errp)
 {
     g_autoptr(QCryptoBlockCreateOptions) create_opts = NULL;
diff --git a/block/qcow2.c b/block/qcow2.c
index 7292dd036c..ee9c815830 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -5225,7 +5225,7 @@ static coroutine_fn GRAPH_RDLOCK int qcow2_co_flush_to_os(BlockDriverState *bs)
 }
 
 static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
-                                       Error **errp)
+                                       BlockDriverState *base_bs, Error **errp)
 {
     Error *local_err = NULL;
     BlockMeasureInfo *info;
@@ -5241,6 +5241,7 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
     bool has_backing_file;
     bool has_luks;
     bool extended_l2;
+    bool base_discard_no_unref = false;
     size_t l2e_size;
 
     /* Parse image creation options */
@@ -5321,6 +5322,13 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
             goto err;
         }
 
+        if (base_bs && base_bs->drv &&
+            !strcmp(base_bs->drv->format_name, "qcow2")) {
+            BDRVQcow2State *base_s = base_bs->opaque;
+
+            base_discard_no_unref = base_s->discard_no_unref;
+        }
+
         virtual_size = ROUND_UP(ssize, cluster_size);
 
         if (has_backing_file) {
@@ -5336,8 +5344,10 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
 
             for (offset = 0; offset < ssize; offset += pnum) {
                 int ret;
+                int retp = 0;
+                bool count = false;
 
-                ret = bdrv_block_status_above(in_bs, NULL, offset,
+                ret = bdrv_block_status_above(in_bs, base_bs, offset,
                                               ssize - offset, &pnum, NULL,
                                               NULL);
                 if (ret < 0) {
@@ -5346,15 +5356,67 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
                     goto err;
                 }
 
-                if (ret & BDRV_BLOCK_ZERO) {
+                if (ret & BDRV_BLOCK_ZERO && !base_bs) {
                     /* Skip zero regions (safe with no backing file) */
-                } else if ((ret & (BDRV_BLOCK_DATA | BDRV_BLOCK_ALLOCATED)) ==
-                           (BDRV_BLOCK_DATA | BDRV_BLOCK_ALLOCATED)) {
-                    /* Extend pnum to end of cluster for next iteration */
-                    pnum = ROUND_UP(offset + pnum, cluster_size) - offset;
-
-                    /* Count clusters we've seen */
-                    required += offset % cluster_size + pnum;
+                } else {
+                    /*
+                     * If there is a base image in the chain, query its
+                     * allocation status for this region so we can decide
+                     * whether the cluster survives a commit into the base.
+                     */
+                    if (base_bs) {
+                        int64_t pnum_base = 0;
+                        retp = bdrv_block_status(base_bs, offset,
+                                            ssize - offset, &pnum_base, NULL,
+                                            NULL);
+                        if (retp < 0) {
+                            error_setg_errno(&local_err, -retp,
+                                    "Unable to get block status of the base");
+                            goto err;
+                        }
+                        /*
+                         * If the base contiguous block is smaller,
+                         * use that pnum, so the next iteration starts with
+                         * the smallest offset.
+                         */
+                        if (pnum_base > 0 && pnum_base < pnum) {
+                            pnum = pnum_base;
+                        }
+                    }
+
+                    if ((ret & (BDRV_BLOCK_DATA | BDRV_BLOCK_ALLOCATED)) ==
+                        (BDRV_BLOCK_DATA | BDRV_BLOCK_ALLOCATED)) {
+                        /* The overlay has its own data here; it is written. */
+                        count = true;
+                    } else if (base_discard_no_unref) {
+                        /*
+                         * With discard-no-unref enabled on the base, clusters
+                         * allocated in the base keep their reference when the
+                         * overlay is committed (discarded clusters are only
+                         * marked zero), so count any cluster that has a valid
+                         * offset in the base.
+                         */
+                        count = (retp & BDRV_BLOCK_OFFSET_VALID) != 0;
+                    } else {
+                        /*
+                         * Without discard-no-unref, committing the overlay
+                         * frees the base clusters that the overlay discards
+                         * (the overlay marks them zero). Every other cluster
+                         * that is allocated in the base is retained, so count
+                         * it unless the overlay zeroes it out.
+                         */
+                        count = (retp & BDRV_BLOCK_ALLOCATED) &&
+                                !((ret & BDRV_BLOCK_ALLOCATED) &&
+                                  (ret & BDRV_BLOCK_ZERO));
+                    }
+
+                    if (count) {
+                        /* Extend pnum to end of cluster for next iteration */
+                        pnum = ROUND_UP(offset + pnum, cluster_size) - offset;
+
+                        /* Count clusters we've seen */
+                        required += offset % cluster_size + pnum;
+                    }
                 }
             }
         }
diff --git a/block/raw-format.c b/block/raw-format.c
index a57c2922d5..8efa86c8b0 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -369,7 +369,7 @@ raw_co_getlength(BlockDriverState *bs)
 }
 
 static BlockMeasureInfo *raw_measure(QemuOpts *opts, BlockDriverState *in_bs,
-                                     Error **errp)
+                                     BlockDriverState *base_bs, Error **errp)
 {
     BlockMeasureInfo *info;
     int64_t required;
diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst
index b0c798b77a..3d9052c4bb 100644
--- a/docs/tools/qemu-img.rst
+++ b/docs/tools/qemu-img.rst
@@ -632,7 +632,7 @@ Command description:
   For more information, consult ``include/block/block.h`` in QEMU's
   source code.
 
-.. option:: measure [--output=OFMT] [-O OUTPUT_FMT] [-o OPTIONS] [--size N | [--object OBJECTDEF] [--image-opts] [-f FMT] [-l SNAPSHOT_PARAM] FILENAME]
+.. option:: measure [--output=OFMT] [-O OUTPUT_FMT] [-o OPTIONS] [-b BASE] [--size N | [--object OBJECTDEF] [--image-opts] [-f FMT] [-l SNAPSHOT_PARAM] FILENAME]
 
   Calculate the file size required for a new image.  This information
   can be used to size logical volumes or SAN LUNs appropriately for
@@ -649,6 +649,13 @@ Command description:
 
   A snapshot in an existing image can be specified using *SNAPSHOT_PARAM*.
 
+  If *BASE* is given, open the backing chain of *FILENAME* up to and
+  including the image *BASE*, and calculate the file size that *BASE*
+  will occupy after *FILENAME* is committed into it (i.e. the size of
+  the merged image).  This is different from the default behaviour,
+  which calculates the size of a standalone copy of *FILENAME*.
+  *BASE* has to be part of the backing chain of *FILENAME*.
+
   The following fields are reported:
 
   ::
diff --git a/include/block/block-io.h b/include/block/block-io.h
index d34d846bb2..c0abd646d0 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -92,7 +92,8 @@ int64_t co_wrapper_bdrv_rdlock
 bdrv_get_allocated_file_size(BlockDriverState *bs);
 
 BlockMeasureInfo *bdrv_measure(BlockDriver *drv, QemuOpts *opts,
-                               BlockDriverState *in_bs, Error **errp);
+                               BlockDriverState *in_bs,
+                               BlockDriverState *base_bs, Error **errp);
 
 int coroutine_fn GRAPH_RDLOCK
 bdrv_co_delete_file(BlockDriverState *bs, Error **errp);
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 147c08155f..b99b1a7d7d 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -731,7 +731,7 @@ struct BlockDriver {
         BlockDriverState *bs);
 
     BlockMeasureInfo *(*bdrv_measure)(QemuOpts *opts, BlockDriverState *in_bs,
-                                      Error **errp);
+                                      BlockDriverState *base_bs, Error **errp);
 
     int coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_pwritev_compressed)(
         BlockDriverState *bs, int64_t offset, int64_t bytes,
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 6bc8265cfb..f3f5c63773 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -78,9 +78,9 @@ SRST
 ERST
 
 DEF("measure", img_measure,
-"measure [--output=ofmt] [-O output_fmt] [-o options] [--size N | [--object objectdef] [--image-opts] [-f fmt] [-l snapshot_param] filename]")
+"measure [--output=ofmt] [-O output_fmt] [-o options] [--size N | [--object objectdef] [--image-opts] [-f fmt] [-l snapshot_param] [-b base] filename]")
 SRST
-.. option:: measure [--output=OFMT] [-O OUTPUT_FMT] [-o OPTIONS] [--size N | [--object OBJECTDEF] [--image-opts] [-f FMT] [-l SNAPSHOT_PARAM] FILENAME]
+.. option:: measure [--output=OFMT] [-O OUTPUT_FMT] [-o OPTIONS] [--size N | [--object OBJECTDEF] [--image-opts] [-f FMT] [-l SNAPSHOT_PARAM] [-b BASE] FILENAME]
 ERST
 
 DEF("snapshot", img_snapshot,
diff --git a/qemu-img.c b/qemu-img.c
index 2f63d31141..0567067ec2 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -5694,6 +5694,7 @@ static int img_measure(const img_cmd_t *ccmd, int argc, char **argv)
     BlockBackend *in_blk = NULL;
     BlockDriver *drv;
     const char *filename = NULL;
+    const char *base_filename = NULL;
     const char *fmt = NULL;
     const char *out_fmt = "raw";
     char *options = NULL;
@@ -5705,6 +5706,7 @@ static int img_measure(const img_cmd_t *ccmd, int argc, char **argv)
     QemuOptsList *create_opts = NULL;
     bool image_opts = false;
     int64_t img_size = -1;
+    BlockDriverState *base_bs = NULL;
     BlockMeasureInfo *info = NULL;
     Error *local_err = NULL;
     int ret = 1;
@@ -5717,6 +5719,7 @@ static int img_measure(const img_cmd_t *ccmd, int argc, char **argv)
         {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
         {"source-image-opts", no_argument, 0, OPTION_IMAGE_OPTS}, /* img_convert */
         {"snapshot", required_argument, 0, 'l'},
+        {"base", required_argument, 0, 'b'},
         {"target-format", required_argument, 0, 'O'},
         {"target-format-options", required_argument, 0, 'o'}, /* img_convert */
         {"options", required_argument, 0, 'o'},
@@ -5727,11 +5730,11 @@ static int img_measure(const img_cmd_t *ccmd, int argc, char **argv)
         {0, 0, 0, 0}
     };
 
-    while ((c = getopt_long(argc, argv, "hf:l:O:o:Us:",
+    while ((c = getopt_long(argc, argv, "hf:l:b:O:o:Us:",
                             long_options, NULL)) != -1) {
         switch (c) {
         case 'h':
-            cmd_help(ccmd, "[-f FMT|--image-opts] [-l SNAPSHOT]\n"
+            cmd_help(ccmd, "[-f FMT|--image-opts] [-l SNAPSHOT] [-b BASE]\n"
 "       [-O TARGET_FMT] [-o TARGET_FMT_OPTS] [--output human|json]\n"
 "       [--object OBJDEF] (--size SIZE | FILE)\n"
 ,
@@ -5742,6 +5745,8 @@ static int img_measure(const img_cmd_t *ccmd, int argc, char **argv)
 "     instead of a file name (incompatible with --format)\n"
 "  -l, --snapshot SNAPSHOT\n"
 "     use this snapshot in FILE as source\n"
+"  -b, --base BASE\n"
+"     open FILE backing chain up to BASE (inclusive)\n"
 "  -O, --target-format TARGET_FMT\n"
 "     desired target/output image format (default: raw)\n"
 "  -o TARGET_FMT_OPTS\n"
@@ -5779,6 +5784,9 @@ static int img_measure(const img_cmd_t *ccmd, int argc, char **argv)
                 snapshot_name = optarg;
             }
             break;
+        case 'b':
+            base_filename = optarg;
+            break;
         case 'O':
             out_fmt = optarg;
             break;
@@ -5814,8 +5822,10 @@ static int img_measure(const img_cmd_t *ccmd, int argc, char **argv)
         filename = argv[optind];
     }
 
-    if (!filename && (image_opts || fmt || snapshot_name || sn_opts)) {
-        error_report("--image-opts, -f, and -l require a filename argument.");
+    if (!filename && (image_opts || fmt || snapshot_name || sn_opts ||
+                      base_filename)) {
+        error_report("--image-opts, -f, -l, and -b require a filename "
+                     "argument.");
         goto out;
     }
     if (filename && img_size != -1) {
@@ -5828,12 +5838,34 @@ static int img_measure(const img_cmd_t *ccmd, int argc, char **argv)
     }
 
     if (filename) {
-        in_blk = img_open(image_opts, filename, fmt, 0,
+        int src_flags = 0;
+
+        /*
+         * When measuring with --base, avoid opening the full backing chain.
+         * We selectively open only up to the requested base afterwards.
+         * An explicit image specification (--image-opts or a json: filename)
+         * defines the backing chain itself, so it has to be opened normally.
+         */
+        if (base_filename && !image_opts &&
+            !g_str_has_prefix(filename, "json:")) {
+            src_flags |= BDRV_O_NO_BACKING;
+        }
+
+        in_blk = img_open(image_opts, filename, fmt, src_flags,
                           false, false, force_share);
         if (!in_blk) {
             goto out;
         }
 
+        if (base_filename) {
+            if (bdrv_open_backing_chain_until(blk_bs(in_blk), base_filename,
+                                              &local_err) < 0) {
+                error_report_err(local_err);
+                goto out;
+            }
+            base_bs = bdrv_find_backing_image(blk_bs(in_blk), base_filename);
+        }
+
         if (sn_opts) {
             bdrv_snapshot_load_tmp(blk_bs(in_blk),
                     qemu_opt_get(sn_opts, SNAPSHOT_OPT_ID),
@@ -5874,7 +5906,8 @@ static int img_measure(const img_cmd_t *ccmd, int argc, char **argv)
         qemu_opt_set_number(opts, BLOCK_OPT_SIZE, img_size, &error_abort);
     }
 
-    info = bdrv_measure(drv, opts, in_blk ? blk_bs(in_blk) : NULL, &local_err);
+    info = bdrv_measure(drv, opts, in_blk ? blk_bs(in_blk) : NULL, base_bs,
+                        &local_err);
     if (local_err) {
         error_report_err(local_err);
         goto out;
diff --git a/tests/qemu-iotests/178.out.qcow2 b/tests/qemu-iotests/178.out.qcow2
index 61506b519f..0984a74a50 100644
--- a/tests/qemu-iotests/178.out.qcow2
+++ b/tests/qemu-iotests/178.out.qcow2
@@ -6,9 +6,9 @@ qemu-img: Either --size N or one filename must be specified.
 qemu-img: --size N cannot be used together with a filename.
 qemu-img: At most one filename argument is allowed.
 qemu-img: Either --size N or one filename must be specified.
-qemu-img: --image-opts, -f, and -l require a filename argument.
-qemu-img: --image-opts, -f, and -l require a filename argument.
-qemu-img: --image-opts, -f, and -l require a filename argument.
+qemu-img: --image-opts, -f, -l, and -b require a filename argument.
+qemu-img: --image-opts, -f, -l, and -b require a filename argument.
+qemu-img: --image-opts, -f, -l, and -b require a filename argument.
 qemu-img: Invalid option list: ,
 qemu-img: Invalid parameter 'snapshot.foo'
 qemu-img: Failed in parsing snapshot param 'snapshot.foo=bar'
diff --git a/tests/qemu-iotests/178.out.raw b/tests/qemu-iotests/178.out.raw
index 6d994a433a..81249e718b 100644
--- a/tests/qemu-iotests/178.out.raw
+++ b/tests/qemu-iotests/178.out.raw
@@ -6,9 +6,9 @@ qemu-img: Either --size N or one filename must be specified.
 qemu-img: --size N cannot be used together with a filename.
 qemu-img: At most one filename argument is allowed.
 qemu-img: Either --size N or one filename must be specified.
-qemu-img: --image-opts, -f, and -l require a filename argument.
-qemu-img: --image-opts, -f, and -l require a filename argument.
-qemu-img: --image-opts, -f, and -l require a filename argument.
+qemu-img: --image-opts, -f, -l, and -b require a filename argument.
+qemu-img: --image-opts, -f, -l, and -b require a filename argument.
+qemu-img: --image-opts, -f, -l, and -b require a filename argument.
 qemu-img: Invalid option list: ,
 qemu-img: Invalid parameter 'snapshot.foo'
 qemu-img: Failed in parsing snapshot param 'snapshot.foo=bar'
-- 
2.55.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 3/3] iotests/290: add test case for qemu-img measure
  2026-09-15 14:04 [PATCH v2 0/3] block: measure for commit Jean-Louis Dupond
  2026-09-15 14:04 ` [PATCH v2 1/3] block: add bdrv_open_backing_chain_until Jean-Louis Dupond
  2026-09-15 14:04 ` [PATCH v2 2/3] block: support measure for commit Jean-Louis Dupond
@ 2026-09-15 14:04 ` Jean-Louis Dupond
  2026-10-30 19:11   ` Kevin Wolf
  2026-10-13 11:10 ` [PATCH v2 0/3] block: measure for commit Jean-Louis Dupond
  3 siblings, 1 reply; 8+ messages in thread
From: Jean-Louis Dupond @ 2026-09-15 14:04 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Pierrick Bouvier, John Snow,
	Vladimir Sementsov-Ogievskiy, Hanna Reitz, Fiona Ebner,
	qemu-block, Jean-Louis Dupond

We create an image, write and discard some data in it, and then create a
snapshot. In the snapshot we write and discard again some data.
Then we measure the images with the base specified to calculate the
merged image size. Finally we commit the image and check it's size.

This scenario is executed for discard-no-unref enabled and disabled.

Signed-off-by: Jean-Louis Dupond <jean-louis@dupond.be>
---
 tests/qemu-iotests/290     | 45 ++++++++++++++++++++++++++++
 tests/qemu-iotests/290.out | 61 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 106 insertions(+)

diff --git a/tests/qemu-iotests/290 b/tests/qemu-iotests/290
index 776b59de1b..2bf811384e 100755
--- a/tests/qemu-iotests/290
+++ b/tests/qemu-iotests/290
@@ -92,6 +92,51 @@ for qcow2_compat in 0.10 1.1; do
     $QEMU_IMG map "$TEST_IMG" | _filter_testdir
 done
 
+echo
+echo "### Test qemu-img measure for commit differences with 'discard-no-unref' option enabled"
+echo
+
+for DISCARD_NO_UNREF in true false; do
+    echo "# Create a base image and fill it with data"
+    TEST_IMG="$TEST_IMG.base" _make_test_img 128M
+
+    $QEMU_IO -c 'write 0 8M' "$TEST_IMG.base" | _filter_qemu_io
+    $QEMU_IO -c 'write 10M 8M' "$TEST_IMG.base" | _filter_qemu_io
+    $QEMU_IO -c 'write 24M 32M' "$TEST_IMG.base" | _filter_qemu_io
+    $QEMU_IO -c 'write 56M 20M' "$TEST_IMG.base" | _filter_qemu_io
+    $QEMU_IO -c "reopen -o discard=unmap,discard-no-unref=$DISCARD_NO_UNREF" \
+        -c 'discard 32M 10M' "$TEST_IMG.base" | _filter_qemu_io
+
+    echo "# Create a top image and do some writes and discards"
+    TEST_IMG="$TEST_IMG.top" _make_test_img -b "$TEST_IMG.base" -F $IMGFMT 128M
+
+    $QEMU_IO -c "reopen -o discard=unmap,discard-no-unref=$DISCARD_NO_UNREF" -c 'write 16M 8M' \
+        -c 'discard 60M 20M' -c 'write 84M 10M' "$TEST_IMG.top" | _filter_qemu_io
+
+    FILE_JSON="json:{
+        'file': {
+            'driver': 'file',
+            'filename': '$TEST_IMG.top'
+        },
+        'driver': 'qcow2',
+        'discard': 'unmap',
+        'discard-no-unref': '$DISCARD_NO_UNREF',
+        'backing': {
+            'driver': 'qcow2',
+            'discard-no-unref': '$DISCARD_NO_UNREF',
+            'file': {
+                'driver': 'file',
+                'filename': '$TEST_IMG.base'
+            },
+            'backing': null
+        }}"
+    echo "# Measure size with discard-no-unref=$DISCARD_NO_UNREF"
+    $QEMU_IMG measure --output=json -O qcow2 -b "$TEST_IMG.base" "${FILE_JSON}"
+    echo "# Merging the top image into the base image"
+    $QEMU_IMG commit -t none -f qcow2 "${FILE_JSON}"
+    stat -c"base disk image file size in bytes: %s" "$TEST_IMG.base"
+done
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/290.out b/tests/qemu-iotests/290.out
index 22b476594f..6dafbfc2b9 100644
--- a/tests/qemu-iotests/290.out
+++ b/tests/qemu-iotests/290.out
@@ -58,4 +58,65 @@ read 131072/131072 bytes at offset 0
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 # Output of qemu-img map
 Offset          Length          Mapped to       File
+
+### Test qemu-img measure for commit differences with 'discard-no-unref' option enabled
+
+# Create a base image and fill it with data
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728
+wrote 8388608/8388608 bytes at offset 0
+8 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 8388608/8388608 bytes at offset 10485760
+8 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 33554432/33554432 bytes at offset 25165824
+32 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 20971520/20971520 bytes at offset 58720256
+20 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+discard 10485760/10485760 bytes at offset 33554432
+10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+# Create a top image and do some writes and discards
+Formatting 'TEST_DIR/t.IMGFMT.top', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=IMGFMT
+wrote 8388608/8388608 bytes at offset 16777216
+8 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+discard 20971520/20971520 bytes at offset 62914560
+20 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 10485760/10485760 bytes at offset 88080384
+10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+# Measure size with discard-no-unref=true
+{
+    "bitmaps": 0,
+    "required": 88408064,
+    "fully-allocated": 134545408
+}
+# Merging the top image into the base image
+Image committed.
+base disk image file size in bytes: 88408064
+# Create a base image and fill it with data
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728
+wrote 8388608/8388608 bytes at offset 0
+8 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 8388608/8388608 bytes at offset 10485760
+8 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 33554432/33554432 bytes at offset 25165824
+32 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 20971520/20971520 bytes at offset 58720256
+20 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+discard 10485760/10485760 bytes at offset 33554432
+10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+# Create a top image and do some writes and discards
+Formatting 'TEST_DIR/t.IMGFMT.top', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=IMGFMT
+wrote 8388608/8388608 bytes at offset 16777216
+8 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+discard 20971520/20971520 bytes at offset 62914560
+20 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 10485760/10485760 bytes at offset 88080384
+10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+# Measure size with discard-no-unref=false
+{
+    "bitmaps": 0,
+    "required": 71630848,
+    "fully-allocated": 134545408
+}
+# Merging the top image into the base image
+Image committed.
+base disk image file size in bytes: 71630848
 *** done
-- 
2.55.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 0/3] block: measure for commit
  2026-09-15 14:04 [PATCH v2 0/3] block: measure for commit Jean-Louis Dupond
                   ` (2 preceding siblings ...)
  2026-09-15 14:04 ` [PATCH v2 3/3] iotests/290: add test case for qemu-img measure Jean-Louis Dupond
@ 2026-10-13 11:10 ` Jean-Louis Dupond
  3 siblings, 0 replies; 8+ messages in thread
From: Jean-Louis Dupond @ 2026-10-13 11:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Pierrick Bouvier, John Snow,
	Vladimir Sementsov-Ogievskiy, Hanna Reitz, Fiona Ebner,
	qemu-block

Ping :)

On 15/09/2026 16:04, Jean-Louis Dupond wrote:
> In some cases we want to calculate the size an image will be after we commit A into B.
> This for example on block storage, where you need to resize the target storage before you start the merge.
>
> These commits adds the functionality to do this.
> First of all a commit that adds bdrv_open_backing_chain_until. So we can open an image up to base image B.
>
> And then a modified qcow2_measure that calculates the size of B after A has been merged into it.
> This with taking into account the discard-no-unref setting of the base.
>
> Jean-Louis Dupond (3):
>    block: add bdrv_open_backing_chain_until
>    block: support measure for commit
>    iotests/290: add test case for qemu-img measure
>
>   block.c                            | 77 +++++++++++++++++++++++-----
>   block/crypto.c                     |  1 +
>   block/mirror.c                     |  2 +-
>   block/qcow2.c                      | 82 ++++++++++++++++++++++++++----
>   block/raw-format.c                 |  2 +-
>   docs/tools/qemu-img.rst            |  9 +++-
>   include/block/block-global-state.h |  7 ++-
>   include/block/block-io.h           |  3 +-
>   include/block/block_int-common.h   |  2 +-
>   qemu-img-cmds.hx                   |  4 +-
>   qemu-img.c                         | 45 +++++++++++++---
>   tests/qemu-iotests/178.out.qcow2   |  6 +--
>   tests/qemu-iotests/178.out.raw     |  6 +--
>   tests/qemu-iotests/290             | 45 ++++++++++++++++
>   tests/qemu-iotests/290.out         | 61 ++++++++++++++++++++++
>   15 files changed, 310 insertions(+), 42 deletions(-)
>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/3] block: add bdrv_open_backing_chain_until
  2026-09-15 14:04 ` [PATCH v2 1/3] block: add bdrv_open_backing_chain_until Jean-Louis Dupond
@ 2026-10-30 17:25   ` Kevin Wolf
  0 siblings, 0 replies; 8+ messages in thread
From: Kevin Wolf @ 2026-10-30 17:25 UTC (permalink / raw)
  To: Jean-Louis Dupond
  Cc: qemu-devel, Pierrick Bouvier, John Snow,
	Vladimir Sementsov-Ogievskiy, Hanna Reitz, Fiona Ebner,
	qemu-block

Am 15.09.2026 um 16:04 hat Jean-Louis Dupond geschrieben:
> Add a new function called bdrv_open_backing_chain_until.
> This is used to open a block up until a specified backing chain.
> 
> Modified bdrv_open_inherit to accept a new arg
> 'open_backing_recursively' to force to not open the next backing.
> Adjusting all references to keep the existing behaviour.
> 
> Signed-off-by: Jean-Louis Dupond <jean-louis@dupond.be>

Do you actually have a use case where limiting the backing chain is a
requirement? It looks a bit like premature optimisation, because for
running a VM, you'll open the whole chain anyway, so it can't be
unworkably long.

> diff --git a/block.c b/block.c
> index f0a6042e61..9763137d8c 100644
> --- a/block.c
> +++ b/block.c
> @@ -83,6 +83,7 @@ static QLIST_HEAD(, BlockDriver) bdrv_drivers =
>  static BlockDriverState *bdrv_open_inherit(const char *filename,
>                                             const char *reference,
>                                             QDict *options, int flags,
> +                                           bool open_backing_recursively,
>                                             BlockDriverState *parent,
>                                             const BdrvChildClass *child_class,
>                                             BdrvChildRole child_role,

bdrv_open_inherit() already has too many parameters. Can't you just pass
BDRV_O_NO_BACKING instead of adding a new one?

> @@ -3733,6 +3735,54 @@ free_exit:
>      return ret;
>  }
>  
> +int
> +bdrv_open_backing_chain_until(BlockDriverState *top_bs,
> +                              const char *base_filename,
> +                              Error **errp)
> +{
> +    BlockDriverState *curr = top_bs;
> +    int ret;
> +
> +    GLOBAL_STATE_CODE();
> +
> +    if (!base_filename) {
> +        return 0;
> +    }
> +
> +    if (bdrv_find_backing_image(top_bs, base_filename)) {
> +        return 0;
> +    }
> +
> +    while (true) {
> +        QDict *options = qdict_clone_shallow(curr->options);
> +
> +        ret = bdrv_open_backing_file(curr, options, false, "backing", errp);
> +        qobject_unref(options);

Does this actually do anything? I don't think curr->options can contain
"backing" options when you opened it without backing files. It would
already fail during the initial open because bdrv_open_backing_file()
doesn't absorb the options, so they are considered unknown:

$ ./qemu-img info --image-opts driver=qcow2,file.filename=/tmp/overlay.qcow2,backing=foo
qemu-img: Could not open 'driver=qcow2,file.filename=/tmp/overlay.qcow2,backing=foo': Block format 'qcow2' does not support the option 'backing'

(Using 'qemu-img info' as an example that uses BDRV_O_NO_BACKING.)

In your specific 'qemu-img measure' implementation, it seems even
impossible to specify backing file options in the BDRV_O_NO_BACKING case
in the first place because --image-opts disables the partial loading.
Which might actually be another reason not to do the partial opening at
all because it's inconsistent behaviour.

> +        if (ret < 0) {
> +            return ret;
> +        }
> +
> +        bdrv_graph_rdlock_main_loop();
> +        if (!curr->backing) {
> +            bdrv_graph_rdunlock_main_loop();
> +            error_setg(errp,
> +                       "Did not find '%s' in the backing chain of '%s'",
> +                       base_filename, top_bs->filename);
> +            return -ENOENT;
> +        }
> +        bdrv_graph_rdunlock_main_loop();
> +
> +        if (bdrv_find_backing_image(curr, base_filename)) {
> +            return 0;
> +        }
> +
> +        /* Switch to the next layer */
> +        bdrv_graph_rdlock_main_loop();
> +        curr = curr->backing->bs;
> +        bdrv_graph_rdunlock_main_loop();
> +    }
> +}

Kevin



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/3] block: support measure for commit
  2026-09-15 14:04 ` [PATCH v2 2/3] block: support measure for commit Jean-Louis Dupond
@ 2026-10-30 19:06   ` Kevin Wolf
  0 siblings, 0 replies; 8+ messages in thread
From: Kevin Wolf @ 2026-10-30 19:06 UTC (permalink / raw)
  To: Jean-Louis Dupond
  Cc: qemu-devel, Pierrick Bouvier, John Snow,
	Vladimir Sementsov-Ogievskiy, Hanna Reitz, Fiona Ebner,
	qemu-block

Am 15.09.2026 um 16:04 hat Jean-Louis Dupond geschrieben:
> Add a new option to qemu-img measure to specify the base node you want
> to merge the image into.
> This will open all the images between top and base, and calculate the
> size required for merging this layer into the base image.
> 
> Also modify the calculation so it takes into account the
> discard-no-unref setting of the base image.
> If discard-no-unref is enabled on the base, discarded blocks in the
> layers above the base will not free clusters in the base image, but will
> only mark them ZERO.
> 
> [1]: https://gitlab.com/qemu-project/qemu/-/issues/2369
> 
> Signed-off-by: Jean-Louis Dupond <jean-louis@dupond.be>

I think we need to make it clearer what this interface is for, because
specifying a base file doesn't immediately make me think of committing
to that base file. 'qemu-img convert' has a -B option that specifies a
base file, and the meaning there would be that any data that can already
be read from the base file is _not_ written to the output file.

> diff --git a/block.c b/block.c
> index 9763137d8c..acc77777ed 100644
> --- a/block.c
> +++ b/block.c
> @@ -6091,7 +6091,8 @@ int64_t coroutine_fn bdrv_co_get_allocated_file_size(BlockDriverState *bs)
>   * If an error occurs the @errp pointer is set.
>   */
>  BlockMeasureInfo *bdrv_measure(BlockDriver *drv, QemuOpts *opts,
> -                               BlockDriverState *in_bs, Error **errp)
> +                               BlockDriverState *in_bs,
> +                               BlockDriverState *base_bs, Error **errp)
>  {
>      IO_CODE();
>      if (!drv->bdrv_measure) {
> @@ -6100,7 +6101,7 @@ BlockMeasureInfo *bdrv_measure(BlockDriver *drv, QemuOpts *opts,
>          return NULL;
>      }
>  
> -    return drv->bdrv_measure(opts, in_bs, errp);
> +    return drv->bdrv_measure(opts, in_bs, base_bs, errp);
>  }
>  
>  /**
> diff --git a/block/crypto.c b/block/crypto.c
> index 36abb7af46..e0236b3382 100644
> --- a/block/crypto.c
> +++ b/block/crypto.c
> @@ -634,6 +634,7 @@ block_crypto_co_getlength(BlockDriverState *bs)
>  
>  static BlockMeasureInfo *block_crypto_measure(QemuOpts *opts,
>                                                BlockDriverState *in_bs,
> +                                              BlockDriverState *base_bs,
>                                                Error **errp)
>  {
>      g_autoptr(QCryptoBlockCreateOptions) create_opts = NULL;
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 7292dd036c..ee9c815830 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -5225,7 +5225,7 @@ static coroutine_fn GRAPH_RDLOCK int qcow2_co_flush_to_os(BlockDriverState *bs)
>  }
>  
>  static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
> -                                       Error **errp)
> +                                       BlockDriverState *base_bs, Error **errp)
>  {
>      Error *local_err = NULL;
>      BlockMeasureInfo *info;
> @@ -5241,6 +5241,7 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
>      bool has_backing_file;
>      bool has_luks;
>      bool extended_l2;
> +    bool base_discard_no_unref = false;
>      size_t l2e_size;
>  
>      /* Parse image creation options */
> @@ -5321,6 +5322,13 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
>              goto err;
>          }
>  
> +        if (base_bs && base_bs->drv &&
> +            !strcmp(base_bs->drv->format_name, "qcow2")) {
> +            BDRVQcow2State *base_s = base_bs->opaque;
> +
> +            base_discard_no_unref = base_s->discard_no_unref;
> +        }

The driver check should be base_bs->drv == &bdrv_qcow2.

But more importantly, this is the only case that actually makes sense
because the output file _is_ the base image. If the base image actually
has a different format than the driver than runs bdrv_measuer(), the
result is garbage.

So how about fixing 'qemu-img measure' to forbid this case (i.e.
specifying the base automatically takes the driver from the base and
it's incompatible with specifying an output format) and just asserting
it here like this:

    if (base_bs) {
        BDRVQcow2State *base_s = base_bs->opaque;
        assert(base_bs->drv == &bdrv_qcow2);

        base_discard_no_unref = base_s->discard_no_unref;
    }

>          virtual_size = ROUND_UP(ssize, cluster_size);
>  
>          if (has_backing_file) {
> @@ -5336,8 +5344,10 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
>  
>              for (offset = 0; offset < ssize; offset += pnum) {
>                  int ret;
> +                int retp = 0;
> +                bool count = false;
>  
> -                ret = bdrv_block_status_above(in_bs, NULL, offset,
> +                ret = bdrv_block_status_above(in_bs, base_bs, offset,
>                                                ssize - offset, &pnum, NULL,
>                                                NULL);
>                  if (ret < 0) {
> @@ -5346,15 +5356,67 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
>                      goto err;
>                  }
>  
> -                if (ret & BDRV_BLOCK_ZERO) {
> +                if (ret & BDRV_BLOCK_ZERO && !base_bs) {
>                      /* Skip zero regions (safe with no backing file) */
> -                } else if ((ret & (BDRV_BLOCK_DATA | BDRV_BLOCK_ALLOCATED)) ==
> -                           (BDRV_BLOCK_DATA | BDRV_BLOCK_ALLOCATED)) {
> -                    /* Extend pnum to end of cluster for next iteration */
> -                    pnum = ROUND_UP(offset + pnum, cluster_size) - offset;
> -
> -                    /* Count clusters we've seen */
> -                    required += offset % cluster_size + pnum;
> +                } else {
> +                    /*
> +                     * If there is a base image in the chain, query its
> +                     * allocation status for this region so we can decide
> +                     * whether the cluster survives a commit into the base.
> +                     */
> +                    if (base_bs) {
> +                        int64_t pnum_base = 0;
> +                        retp = bdrv_block_status(base_bs, offset,
> +                                            ssize - offset, &pnum_base, NULL,
> +                                            NULL);
> +                        if (retp < 0) {
> +                            error_setg_errno(&local_err, -retp,
> +                                    "Unable to get block status of the base");
> +                            goto err;
> +                        }
> +                        /*
> +                         * If the base contiguous block is smaller,
> +                         * use that pnum, so the next iteration starts with
> +                         * the smallest offset.
> +                         */
> +                        if (pnum_base > 0 && pnum_base < pnum) {
> +                            pnum = pnum_base;
> +                        }
> +                    }
> +
> +                    if ((ret & (BDRV_BLOCK_DATA | BDRV_BLOCK_ALLOCATED)) ==
> +                        (BDRV_BLOCK_DATA | BDRV_BLOCK_ALLOCATED)) {

Doesn't BDRV_BLOCK_DATA automatically imply BDRV_BLOCK_ALLOCATED?

> +                        /* The overlay has its own data here; it is written. */
> +                        count = true;
> +                    } else if (base_discard_no_unref) {
> +                        /*
> +                         * With discard-no-unref enabled on the base, clusters
> +                         * allocated in the base keep their reference when the
> +                         * overlay is committed (discarded clusters are only
> +                         * marked zero), so count any cluster that has a valid
> +                         * offset in the base.
> +                         */
> +                        count = (retp & BDRV_BLOCK_OFFSET_VALID) != 0;

This condition seems wrong. Encrypted clusters don't have a valid
offset, but they still need to be counted.

Isn't the correct condition BDRV_BLOCK_DATA like above, just for retp
instead of ret?

> +                    } else {
> +                        /*
> +                         * Without discard-no-unref, committing the overlay
> +                         * frees the base clusters that the overlay discards
> +                         * (the overlay marks them zero). Every other cluster
> +                         * that is allocated in the base is retained, so count
> +                         * it unless the overlay zeroes it out.
> +                         */
> +                        count = (retp & BDRV_BLOCK_ALLOCATED) &&
> +                                !((ret & BDRV_BLOCK_ALLOCATED) &&
> +                                  (ret & BDRV_BLOCK_ZERO));

BDRV_BLOCK_ALLOCATED doesn't mean that it uses space, it could already
be a zero cluster. I think you want to check BDRV_BLOCK_DATA here again.

For the second half of the condition, BDRV_BLOCK_ZERO should imply
BDRV_BLOCK_ALLOCATED.

> +                    }
> +
> +                    if (count) {
> +                        /* Extend pnum to end of cluster for next iteration */
> +                        pnum = ROUND_UP(offset + pnum, cluster_size) - offset;
> +
> +                        /* Count clusters we've seen */
> +                        required += offset % cluster_size + pnum;
> +                    }
>                  }
>              }
>          }
> diff --git a/block/raw-format.c b/block/raw-format.c
> index a57c2922d5..8efa86c8b0 100644
> --- a/block/raw-format.c
> +++ b/block/raw-format.c
> @@ -369,7 +369,7 @@ raw_co_getlength(BlockDriverState *bs)
>  }
>  
>  static BlockMeasureInfo *raw_measure(QemuOpts *opts, BlockDriverState *in_bs,
> -                                     Error **errp)
> +                                     BlockDriverState *base_bs, Error **errp)
>  {
>      BlockMeasureInfo *info;
>      int64_t required;
> diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst
> index b0c798b77a..3d9052c4bb 100644
> --- a/docs/tools/qemu-img.rst
> +++ b/docs/tools/qemu-img.rst
> @@ -632,7 +632,7 @@ Command description:
>    For more information, consult ``include/block/block.h`` in QEMU's
>    source code.
>  
> -.. option:: measure [--output=OFMT] [-O OUTPUT_FMT] [-o OPTIONS] [--size N | [--object OBJECTDEF] [--image-opts] [-f FMT] [-l SNAPSHOT_PARAM] FILENAME]
> +.. option:: measure [--output=OFMT] [-O OUTPUT_FMT] [-o OPTIONS] [-b BASE] [--size N | [--object OBJECTDEF] [--image-opts] [-f FMT] [-l SNAPSHOT_PARAM] FILENAME]
>  
>    Calculate the file size required for a new image.  This information
>    can be used to size logical volumes or SAN LUNs appropriately for
> @@ -649,6 +649,13 @@ Command description:
>  
>    A snapshot in an existing image can be specified using *SNAPSHOT_PARAM*.
>  
> +  If *BASE* is given, open the backing chain of *FILENAME* up to and
> +  including the image *BASE*, and calculate the file size that *BASE*
> +  will occupy after *FILENAME* is committed into it (i.e. the size of
> +  the merged image).  This is different from the default behaviour,
> +  which calculates the size of a standalone copy of *FILENAME*.
> +  *BASE* has to be part of the backing chain of *FILENAME*.
> +
>    The following fields are reported:
>  
>    ::
> diff --git a/include/block/block-io.h b/include/block/block-io.h
> index d34d846bb2..c0abd646d0 100644
> --- a/include/block/block-io.h
> +++ b/include/block/block-io.h
> @@ -92,7 +92,8 @@ int64_t co_wrapper_bdrv_rdlock
>  bdrv_get_allocated_file_size(BlockDriverState *bs);
>  
>  BlockMeasureInfo *bdrv_measure(BlockDriver *drv, QemuOpts *opts,
> -                               BlockDriverState *in_bs, Error **errp);
> +                               BlockDriverState *in_bs,
> +                               BlockDriverState *base_bs, Error **errp);
>  
>  int coroutine_fn GRAPH_RDLOCK
>  bdrv_co_delete_file(BlockDriverState *bs, Error **errp);
> diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
> index 147c08155f..b99b1a7d7d 100644
> --- a/include/block/block_int-common.h
> +++ b/include/block/block_int-common.h
> @@ -731,7 +731,7 @@ struct BlockDriver {
>          BlockDriverState *bs);
>  
>      BlockMeasureInfo *(*bdrv_measure)(QemuOpts *opts, BlockDriverState *in_bs,
> -                                      Error **errp);
> +                                      BlockDriverState *base_bs, Error **errp);
>  
>      int coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_pwritev_compressed)(
>          BlockDriverState *bs, int64_t offset, int64_t bytes,
> diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
> index 6bc8265cfb..f3f5c63773 100644
> --- a/qemu-img-cmds.hx
> +++ b/qemu-img-cmds.hx
> @@ -78,9 +78,9 @@ SRST
>  ERST
>  
>  DEF("measure", img_measure,
> -"measure [--output=ofmt] [-O output_fmt] [-o options] [--size N | [--object objectdef] [--image-opts] [-f fmt] [-l snapshot_param] filename]")
> +"measure [--output=ofmt] [-O output_fmt] [-o options] [--size N | [--object objectdef] [--image-opts] [-f fmt] [-l snapshot_param] [-b base] filename]")
>  SRST
> -.. option:: measure [--output=OFMT] [-O OUTPUT_FMT] [-o OPTIONS] [--size N | [--object OBJECTDEF] [--image-opts] [-f FMT] [-l SNAPSHOT_PARAM] FILENAME]
> +.. option:: measure [--output=OFMT] [-O OUTPUT_FMT] [-o OPTIONS] [--size N | [--object OBJECTDEF] [--image-opts] [-f FMT] [-l SNAPSHOT_PARAM] [-b BASE] FILENAME]
>  ERST
>  
>  DEF("snapshot", img_snapshot,
> diff --git a/qemu-img.c b/qemu-img.c
> index 2f63d31141..0567067ec2 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -5694,6 +5694,7 @@ static int img_measure(const img_cmd_t *ccmd, int argc, char **argv)
>      BlockBackend *in_blk = NULL;
>      BlockDriver *drv;
>      const char *filename = NULL;
> +    const char *base_filename = NULL;
>      const char *fmt = NULL;
>      const char *out_fmt = "raw";
>      char *options = NULL;
> @@ -5705,6 +5706,7 @@ static int img_measure(const img_cmd_t *ccmd, int argc, char **argv)
>      QemuOptsList *create_opts = NULL;
>      bool image_opts = false;
>      int64_t img_size = -1;
> +    BlockDriverState *base_bs = NULL;
>      BlockMeasureInfo *info = NULL;
>      Error *local_err = NULL;
>      int ret = 1;
> @@ -5717,6 +5719,7 @@ static int img_measure(const img_cmd_t *ccmd, int argc, char **argv)
>          {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
>          {"source-image-opts", no_argument, 0, OPTION_IMAGE_OPTS}, /* img_convert */
>          {"snapshot", required_argument, 0, 'l'},
> +        {"base", required_argument, 0, 'b'},
>          {"target-format", required_argument, 0, 'O'},
>          {"target-format-options", required_argument, 0, 'o'}, /* img_convert */
>          {"options", required_argument, 0, 'o'},
> @@ -5727,11 +5730,11 @@ static int img_measure(const img_cmd_t *ccmd, int argc, char **argv)
>          {0, 0, 0, 0}
>      };
>  
> -    while ((c = getopt_long(argc, argv, "hf:l:O:o:Us:",
> +    while ((c = getopt_long(argc, argv, "hf:l:b:O:o:Us:",
>                              long_options, NULL)) != -1) {
>          switch (c) {
>          case 'h':
> -            cmd_help(ccmd, "[-f FMT|--image-opts] [-l SNAPSHOT]\n"
> +            cmd_help(ccmd, "[-f FMT|--image-opts] [-l SNAPSHOT] [-b BASE]\n"
>  "       [-O TARGET_FMT] [-o TARGET_FMT_OPTS] [--output human|json]\n"
>  "       [--object OBJDEF] (--size SIZE | FILE)\n"
>  ,
> @@ -5742,6 +5745,8 @@ static int img_measure(const img_cmd_t *ccmd, int argc, char **argv)
>  "     instead of a file name (incompatible with --format)\n"
>  "  -l, --snapshot SNAPSHOT\n"
>  "     use this snapshot in FILE as source\n"
> +"  -b, --base BASE\n"
> +"     open FILE backing chain up to BASE (inclusive)\n"

This is technically true in the common case (it isn't for --image-opts),
but an irrelevant implementation detail that doesn't tell anything about
what you'd use it for.

This should say something about measuring the size of BASE after
committing FILE to it.

As I said above, --base could also have a different meaning, so we might
think of a more descriptive name like --commit-base. Not sure what a
good short option name for it would be - maybe it doesn't even need one.

>  "  -O, --target-format TARGET_FMT\n"
>  "     desired target/output image format (default: raw)\n"
>  "  -o TARGET_FMT_OPTS\n"

Kevin



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 3/3] iotests/290: add test case for qemu-img measure
  2026-09-15 14:04 ` [PATCH v2 3/3] iotests/290: add test case for qemu-img measure Jean-Louis Dupond
@ 2026-10-30 19:11   ` Kevin Wolf
  0 siblings, 0 replies; 8+ messages in thread
From: Kevin Wolf @ 2026-10-30 19:11 UTC (permalink / raw)
  To: Jean-Louis Dupond
  Cc: qemu-devel, Pierrick Bouvier, John Snow,
	Vladimir Sementsov-Ogievskiy, Hanna Reitz, Fiona Ebner,
	qemu-block

Am 15.09.2026 um 16:04 hat Jean-Louis Dupond geschrieben:
> We create an image, write and discard some data in it, and then create a
> snapshot. In the snapshot we write and discard again some data.
> Then we measure the images with the base specified to calculate the
> merged image size. Finally we commit the image and check it's size.
> 
> This scenario is executed for discard-no-unref enabled and disabled.
> 
> Signed-off-by: Jean-Louis Dupond <jean-louis@dupond.be>

You could maybe add the cases I commented on in patch 2, i.e. having an
explicit zero cluster in the base file (which shouldn't count) and an
encrypted base (which should count its clusters despite not having
BDRV_BLOCK_OFFSET_VALID).

Kevin



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-10-30 19:12 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-15 14:04 [PATCH v2 0/3] block: measure for commit Jean-Louis Dupond
2026-09-15 14:04 ` [PATCH v2 1/3] block: add bdrv_open_backing_chain_until Jean-Louis Dupond
2026-10-30 17:25   ` Kevin Wolf
2026-09-15 14:04 ` [PATCH v2 2/3] block: support measure for commit Jean-Louis Dupond
2026-10-30 19:06   ` Kevin Wolf
2026-09-15 14:04 ` [PATCH v2 3/3] iotests/290: add test case for qemu-img measure Jean-Louis Dupond
2026-10-30 19:11   ` Kevin Wolf
2026-10-13 11:10 ` [PATCH v2 0/3] block: measure for commit Jean-Louis Dupond

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.