[PATCH 0/4] refresh filter parents when child was resized

[PATCH 0/4] refresh filter parents when child was resized

[Fiona Ebner]

Resizing a node below a filter would result in the filter still
reporting the old size. Implement a 'resize' callback for the
child_of_bds class, that refreshes filter parents recursively.

Fiona Ebner (4):
  include/block/block_int-common: document when resize callback is used
  block: make bdrv_co_parent_cb_resize() a proper IO API function
  block: implement 'resize' callback for child_of_bds class
  iotests: add test for resizing a node below filters

 block.c                                       | 12 +++
 block/io.c                                    |  9 +--
 include/block/block_int-common.h              |  5 +-
 include/block/block_int-io.h                  |  6 ++
 tests/qemu-iotests/tests/resize-below-filter  | 73 +++++++++++++++++++
 .../tests/resize-below-filter.out             |  5 ++
 6 files changed, 103 insertions(+), 7 deletions(-)
 create mode 100755 tests/qemu-iotests/tests/resize-below-filter
 create mode 100644 tests/qemu-iotests/tests/resize-below-filter.out

-- 
2.47.2

[PATCH 1/4] include/block/block_int-common: document when resize callback is used

[Fiona Ebner]

The 'resize' callback is only called by bdrv_parent_cb_resize() which
is only called by bdrv_co_write_req_finish() to notify the parent(s)
that the child was resized.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 include/block/block_int-common.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 925a3e7353..692a9696d1 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -1020,6 +1020,9 @@ struct BdrvChildClass {
      * the I/O API.
      */
 
+    /*
+     * Notifies the parent that the child was resized.
+     */
     void (*resize)(BdrvChild *child);
 
     /*
-- 
2.47.2

[PATCH 2/4] block: make bdrv_co_parent_cb_resize() a proper IO API function

[Fiona Ebner]

In preparation for calling it via the bdrv_child_cb_resize() callback
that will be added by the next commit. Rename it to include the "_co_"
part while at it.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 block/io.c                   | 9 +++------
 include/block/block_int-io.h | 6 ++++++
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/block/io.c b/block/io.c
index ac5c7174f6..2e153364a9 100644
--- a/block/io.c
+++ b/block/io.c
@@ -46,9 +46,6 @@
 /* Maximum read size for checking if data reads as zero, in bytes */
 #define MAX_ZERO_CHECK_BUFFER (128 * KiB)
 
-static void coroutine_fn GRAPH_RDLOCK
-bdrv_parent_cb_resize(BlockDriverState *bs);
-
 static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
     int64_t offset, int64_t bytes, BdrvRequestFlags flags);
 
@@ -2041,7 +2038,7 @@ bdrv_co_write_req_finish(BdrvChild *child, int64_t offset, int64_t bytes,
          end_sector > bs->total_sectors) &&
         req->type != BDRV_TRACKED_DISCARD) {
         bs->total_sectors = end_sector;
-        bdrv_parent_cb_resize(bs);
+        bdrv_co_parent_cb_resize(bs);
         bdrv_dirty_bitmap_truncate(bs, end_sector << BDRV_SECTOR_BITS);
     }
     if (req->bytes) {
@@ -3573,11 +3570,11 @@ int coroutine_fn bdrv_co_copy_range(BdrvChild *src, int64_t src_offset,
                                    bytes, read_flags, write_flags);
 }
 
-static void coroutine_fn GRAPH_RDLOCK
-bdrv_parent_cb_resize(BlockDriverState *bs)
+void coroutine_fn bdrv_co_parent_cb_resize(BlockDriverState *bs)
 {
     BdrvChild *c;
 
+    IO_CODE();
     assert_bdrv_graph_readable();
 
     QLIST_FOREACH(c, &bs->parents, next_parent) {
diff --git a/include/block/block_int-io.h b/include/block/block_int-io.h
index 4f94eb3c5a..ed8b5657d6 100644
--- a/include/block/block_int-io.h
+++ b/include/block/block_int-io.h
@@ -191,4 +191,10 @@ void bdrv_bsc_invalidate_range(BlockDriverState *bs,
  */
 void bdrv_bsc_fill(BlockDriverState *bs, int64_t offset, int64_t bytes);
 
+/*
+ * Notify all parents that the size of the child changed.
+ */
+void coroutine_fn GRAPH_RDLOCK
+bdrv_co_parent_cb_resize(BlockDriverState *bs);
+
 #endif /* BLOCK_INT_IO_H */
-- 
2.47.2

[PATCH 3/4] block: implement 'resize' callback for child_of_bds class

[Fiona Ebner]

If a node below a filter node is resized, the size of the filter node
is now also refreshed (recursively for filter parents).

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 block.c                          | 12 ++++++++++++
 include/block/block_int-common.h |  2 +-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index bfd4340b24..449f814ebe 100644
--- a/block.c
+++ b/block.c
@@ -1497,6 +1497,17 @@ static void GRAPH_WRLOCK bdrv_child_cb_detach(BdrvChild *child)
     }
 }
 
+static void coroutine_fn GRAPH_RDLOCK bdrv_child_cb_resize(BdrvChild *child)
+{
+    BlockDriverState *bs = child->opaque;
+
+    if (bs->drv && bs->drv->is_filter) {
+        /* Best effort, ignore errors. */
+        bdrv_co_refresh_total_sectors(bs, bs->total_sectors);
+        bdrv_co_parent_cb_resize(bs);
+    }
+}
+
 static int bdrv_child_cb_update_filename(BdrvChild *c, BlockDriverState *base,
                                          const char *filename,
                                          bool backing_mask_protocol,
@@ -1529,6 +1540,7 @@ const BdrvChildClass child_of_bds = {
     .detach          = bdrv_child_cb_detach,
     .inactivate      = bdrv_child_cb_inactivate,
     .change_aio_ctx  = bdrv_child_cb_change_aio_ctx,
+    .resize          = bdrv_child_cb_resize,
     .update_filename = bdrv_child_cb_update_filename,
     .get_parent_aio_context = child_of_bds_get_parent_aio_context,
 };
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 692a9696d1..6670db2f85 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -1023,7 +1023,7 @@ struct BdrvChildClass {
     /*
      * Notifies the parent that the child was resized.
      */
-    void (*resize)(BdrvChild *child);
+    void GRAPH_RDLOCK_PTR (*resize)(BdrvChild *child);
 
     /*
      * Returns a name that is supposedly more useful for human users than the
-- 
2.47.2

[PATCH 4/4] iotests: add test for resizing a node below filters

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 tests/qemu-iotests/tests/resize-below-filter  | 73 +++++++++++++++++++
 .../tests/resize-below-filter.out             |  5 ++
 2 files changed, 78 insertions(+)
 create mode 100755 tests/qemu-iotests/tests/resize-below-filter
 create mode 100644 tests/qemu-iotests/tests/resize-below-filter.out

diff --git a/tests/qemu-iotests/tests/resize-below-filter b/tests/qemu-iotests/tests/resize-below-filter
new file mode 100755
index 0000000000..f55619cf34
--- /dev/null
+++ b/tests/qemu-iotests/tests/resize-below-filter
@@ -0,0 +1,73 @@
+#!/usr/bin/env python3
+# group: rw quick
+#
+# Test what happens when a node below filter nodes is resized.
+#
+# Copyright (C) Proxmox Server Solutions GmbH
+#
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+import os
+import iotests
+from iotests import imgfmt, qemu_img_create, QMPTestCase
+
+image_size = 1 * 1024 * 1024
+image = os.path.join(iotests.test_dir, 'test.img')
+
+class TestResizeBelowFilter(QMPTestCase):
+    def setUp(self) -> None:
+        qemu_img_create('-f', imgfmt, image, str(image_size))
+
+        self.vm = iotests.VM()
+        self.vm.add_blockdev(self.vm.qmp_to_opts({
+            'driver': imgfmt,
+            'node-name': 'node0',
+            'file': {
+                'driver': 'file',
+                'filename': image,
+            }
+        }))
+        self.vm.add_blockdev(self.vm.qmp_to_opts({
+            'driver': 'compress',
+            'node-name': 'comp0',
+            'file': 'node0',
+        }))
+        self.vm.add_object('throttle-group,id=thrgr0')
+        self.vm.add_blockdev(self.vm.qmp_to_opts({
+            'driver': 'throttle',
+            'node-name': 'thr0',
+            'throttle-group': 'thrgr0',
+            'file': 'comp0',
+        }))
+        self.vm.add_object('throttle-group,id=thrgr1')
+        self.vm.add_blockdev(self.vm.qmp_to_opts({
+            'driver': 'throttle',
+            'node-name': 'thr1',
+            'throttle-group': 'thrgr1',
+            'file': 'node0',
+        }))
+        self.vm.launch()
+
+    def tearDown(self) -> None:
+        self.vm.shutdown()
+        os.remove(image)
+
+    def assert_size(self, size: int) -> None:
+        nodes = self.vm.qmp('query-named-block-nodes', flat=True)['return']
+        self.assertEqual(len(nodes), 5)
+        for node in nodes:
+            if node['drv'] == 'file':
+                continue
+            self.assertEqual(node['image']['virtual-size'], size)
+
+    def test_resize_below_filter(self) -> None:
+        self.assert_size(image_size)
+        self.vm.qmp('block_resize', node_name='thr0', size=2*image_size)
+        self.assert_size(2*image_size)
+        self.vm.qmp('block_resize', node_name='comp0', size=3*image_size)
+        self.assert_size(3*image_size)
+        self.vm.qmp('block_resize', node_name='node0', size=4*image_size)
+        self.assert_size(4*image_size)
+
+if __name__ == '__main__':
+    iotests.main(supported_fmts=['qcow2'], supported_protocols=['file'])
diff --git a/tests/qemu-iotests/tests/resize-below-filter.out b/tests/qemu-iotests/tests/resize-below-filter.out
new file mode 100644
index 0000000000..ae1213e6f8
--- /dev/null
+++ b/tests/qemu-iotests/tests/resize-below-filter.out
@@ -0,0 +1,5 @@
+.
+----------------------------------------------------------------------
+Ran 1 tests
+
+OK
-- 
2.47.2

Re: [PATCH 3/4] block: implement 'resize' callback for child_of_bds class

[Hanna Czenczek]

On 30.06.25 13:27, Fiona Ebner wrote:
> If a node below a filter node is resized, the size of the filter node
> is now also refreshed (recursively for filter parents).
>
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>   block.c                          | 12 ++++++++++++
>   include/block/block_int-common.h |  2 +-
>   2 files changed, 13 insertions(+), 1 deletion(-)

What does this do for block job filter nodes, like mirror?  If they 
changed their length, we might have to consider whether the job also 
needs to be amended somehow.

However, I assume it’s a safe (conservative) change for them because 
such drivers don’t implement bdrv_co_getlength(), so 
bdrv_co_refresh_total_sectors() will not change anything.  Is that right 
and intended?

Reviewed-by: Hanna Czenczek <hreitz@redhat.com>

(Babbling below.)

> diff --git a/block.c b/block.c
> index bfd4340b24..449f814ebe 100644
> --- a/block.c
> +++ b/block.c
> @@ -1497,6 +1497,17 @@ static void GRAPH_WRLOCK bdrv_child_cb_detach(BdrvChild *child)
>       }
>   }
>   
> +static void coroutine_fn GRAPH_RDLOCK bdrv_child_cb_resize(BdrvChild *child)
> +{
> +    BlockDriverState *bs = child->opaque;
> +
> +    if (bs->drv && bs->drv->is_filter) {

Checking the role for BDRV_CHILD_FILTERED would be more generic; but it 
would cause 'raw' nodes to be updated, too.  But I don’t know whether we 
want that or not, and excluding raw (i.e. not changing behavior there) 
is certainly the safe option.

> +        /* Best effort, ignore errors. */
> +        bdrv_co_refresh_total_sectors(bs, bs->total_sectors);
> +        bdrv_co_parent_cb_resize(bs);

This makes me wonder whether bdrv_co_refresh_total_sectors() should 
itself call bdrv_co_parent_cb_resize().  Still not quite sure; if the 
underlying image file is resized by an external party (and we notice 
this by accident, more or less), maybe the guest device should be informed.

(One thing to consider, maybe, are nodes that unshare the resize 
permission for a child.  It’s probably not clever to call the resize CB 
if that permission is unshared, so maybe just from that perspective, we 
should keep that CB strictly inside of that explicit truncate path that 
checks that permission (at least when growing images...).)

Anyway, again, this is the safe option.

Hanna

> +    }
> +}
> +
>   static int bdrv_child_cb_update_filename(BdrvChild *c, BlockDriverState *base,
>                                            const char *filename,
>                                            bool backing_mask_protocol,
> @@ -1529,6 +1540,7 @@ const BdrvChildClass child_of_bds = {
>       .detach          = bdrv_child_cb_detach,
>       .inactivate      = bdrv_child_cb_inactivate,
>       .change_aio_ctx  = bdrv_child_cb_change_aio_ctx,
> +    .resize          = bdrv_child_cb_resize,
>       .update_filename = bdrv_child_cb_update_filename,
>       .get_parent_aio_context = child_of_bds_get_parent_aio_context,
>   };
> diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
> index 692a9696d1..6670db2f85 100644
> --- a/include/block/block_int-common.h
> +++ b/include/block/block_int-common.h
> @@ -1023,7 +1023,7 @@ struct BdrvChildClass {
>       /*
>        * Notifies the parent that the child was resized.
>        */
> -    void (*resize)(BdrvChild *child);
> +    void GRAPH_RDLOCK_PTR (*resize)(BdrvChild *child);
>   
>       /*
>        * Returns a name that is supposedly more useful for human users than the

Re: [PATCH 1/4] include/block/block_int-common: document when resize callback is used

[Hanna Czenczek]

On 30.06.25 13:27, Fiona Ebner wrote:
> The 'resize' callback is only called by bdrv_parent_cb_resize() which
> is only called by bdrv_co_write_req_finish() to notify the parent(s)
> that the child was resized.
>
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>   include/block/block_int-common.h | 3 +++
>   1 file changed, 3 insertions(+)

Reviewed-by: Hanna Czenczek <hreitz@redhat.com>

Re: [PATCH 2/4] block: make bdrv_co_parent_cb_resize() a proper IO API function

[Hanna Czenczek]

On 30.06.25 13:27, Fiona Ebner wrote:
> In preparation for calling it via the bdrv_child_cb_resize() callback
> that will be added by the next commit. Rename it to include the "_co_"
> part while at it.
>
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>   block/io.c                   | 9 +++------
>   include/block/block_int-io.h | 6 ++++++
>   2 files changed, 9 insertions(+), 6 deletions(-)

Reviewed-by: Hanna Czenczek <hreitz@redhat.com>

Re: [PATCH 4/4] iotests: add test for resizing a node below filters

[Hanna Czenczek]

On 30.06.25 13:27, Fiona Ebner wrote:
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>   tests/qemu-iotests/tests/resize-below-filter  | 73 +++++++++++++++++++
>   .../tests/resize-below-filter.out             |  5 ++
>   2 files changed, 78 insertions(+)
>   create mode 100755 tests/qemu-iotests/tests/resize-below-filter
>   create mode 100644 tests/qemu-iotests/tests/resize-below-filter.out

Reviewed-by: Hanna Czenczek <hreitz@redhat.com>

Re: [PATCH 3/4] block: implement 'resize' callback for child_of_bds class

[Kevin Wolf]

Am 01.07.2025 um 16:46 hat Hanna Czenczek geschrieben:
> On 30.06.25 13:27, Fiona Ebner wrote:
> > If a node below a filter node is resized, the size of the filter node
> > is now also refreshed (recursively for filter parents).
> > 
> > Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> > ---
> >   block.c                          | 12 ++++++++++++
> >   include/block/block_int-common.h |  2 +-
> >   2 files changed, 13 insertions(+), 1 deletion(-)
> 
> What does this do for block job filter nodes, like mirror?  If they changed
> their length, we might have to consider whether the job also needs to be
> amended somehow.

mirror doesn't share BLK_PERM_RESIZE in its block_job_create() call, so
can this even happen? (If yes, that sounds bad.)

> However, I assume it’s a safe (conservative) change for them because such
> drivers don’t implement bdrv_co_getlength(), so
> bdrv_co_refresh_total_sectors() will not change anything.  Is that right and
> intended?
> 
> Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
> 
> (Babbling below.)
> 
> > diff --git a/block.c b/block.c
> > index bfd4340b24..449f814ebe 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -1497,6 +1497,17 @@ static void GRAPH_WRLOCK bdrv_child_cb_detach(BdrvChild *child)
> >       }
> >   }
> > +static void coroutine_fn GRAPH_RDLOCK bdrv_child_cb_resize(BdrvChild *child)
> > +{
> > +    BlockDriverState *bs = child->opaque;
> > +
> > +    if (bs->drv && bs->drv->is_filter) {
> 
> Checking the role for BDRV_CHILD_FILTERED would be more generic; but it
> would cause 'raw' nodes to be updated, too.  But I don’t know whether we
> want that or not, and excluding raw (i.e. not changing behavior there) is
> certainly the safe option.

If the size is not explicitly overridden in the node configuration, I
would certainly expect that a raw node reflects the size of its file
node.

Seems this is exactly the condition that makes it use
BDRV_CHILD_FILTERED, so it would probably be a good change?

> > +        /* Best effort, ignore errors. */
> > +        bdrv_co_refresh_total_sectors(bs, bs->total_sectors);
> > +        bdrv_co_parent_cb_resize(bs);
> 
> This makes me wonder whether bdrv_co_refresh_total_sectors() should itself
> call bdrv_co_parent_cb_resize().  Still not quite sure; if the underlying
> image file is resized by an external party (and we notice this by accident,
> more or less), maybe the guest device should be informed.
> 
> (One thing to consider, maybe, are nodes that unshare the resize permission
> for a child.  It’s probably not clever to call the resize CB if that
> permission is unshared, so maybe just from that perspective, we should keep
> that CB strictly inside of that explicit truncate path that checks that
> permission (at least when growing images...).)
> 
> Anyway, again, this is the safe option.

The traditional solution for nodes that unshare resize, but get resized
in the background anyway would be bs->drv = NULL, and we probably still
aren't certain what happens after that. :-)

Kevin