[PATCH v2 0/3] blockdev: mirror: do not require a root block node

[PATCH v2 0/3] blockdev: mirror: do not require a root block node

[Fiona Ebner]

Changes in v2:
* Drop two accidentally included semicolons in test.
* Update QAPI description.

The mirror block job currently only operates on a root block node. It
might be desired to use a throttle node as the root node for limiting
the guest IO, without also limiting mirror operations.

There is no obvious reason why mirror should only be able to operate
on a root block node.

For a related discussion see here:
https://lore.kernel.org/qemu-devel/9f516c2f-7dd3-446a-8495-56adac267cfe@proxmox.com/

Fiona Ebner (3):
  iotests/041: add test for mirror with throttle node as source
  blockdev: mirror: do not require a root block node
  iotests/041: add tests for mirroring below throttle node

 blockdev.c                 |  20 +++++-
 qapi/block-core.json       |   4 +-
 tests/qemu-iotests/041     | 128 +++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/041.out |   4 +-
 4 files changed, 149 insertions(+), 7 deletions(-)

-- 
2.47.3

[PATCH v2 1/3] iotests/041: add test for mirror with throttle node as source

[Fiona Ebner]

Mostly in preparation for commit "iotests/041: add tests for mirroring
below throttle node" which adds test cases for commit "blockdev:
mirror: do not require a root block node".

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Changes in v2:
* Drop two accidentally included semicolons.

 tests/qemu-iotests/041     | 88 ++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/041.out |  4 +-
 2 files changed, 90 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index 8452845f44..2552388690 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -1373,6 +1373,94 @@ class TestFilters(iotests.QMPTestCase):
 
         self.complete_and_wait('mirror')
 
+class TestThrottle(iotests.QMPTestCase):
+    image_len = 3 * 1024 * 1024
+    attachTargetImage = True
+
+    def mirror_command(self, device):
+        self.vm.cmd('blockdev-mirror',
+                    job_id='mirror',
+                    device=device,
+                    target='target',
+                    sync='full')
+
+    def setUp(self):
+        qemu_img('create', '-f', iotests.imgfmt,
+                 test_img, str(self.image_len))
+        qemu_img('create', '-f', iotests.imgfmt,
+                 target_img, str(self.image_len))
+        qemu_io('-c', 'write -P 23 0 3M', test_img)
+
+        self.vm = iotests.VM().add_device('virtio-scsi,id=vio-scsi')
+        self.vm.launch()
+
+        self.vm.cmd('object-add', {
+                        'id': 'thrgr0',
+                        'qom-type': 'throttle-group',
+                        'limits': {
+                            'bps-total': 1 * 1024
+                        }
+                    })
+
+        if self.attachTargetImage:
+            self.vm.cmd('blockdev-add', {
+                            'node-name': 'target',
+                            'driver': iotests.imgfmt,
+                            'file': {
+                                'driver': 'file',
+                                'filename': target_img
+                            },
+                        })
+
+        source = {
+            'node-name': 'source',
+            'driver': iotests.imgfmt,
+            'file': {
+                'driver': 'file',
+                'filename': test_img
+            },
+        }
+
+        self.vm.cmd('blockdev-add', {
+                        'node-name': 'throttle',
+                        'driver': 'throttle',
+                        'throttle-group': 'thrgr0',
+                        'file': source,
+                    })
+
+        self.vm.cmd('device_add',
+                    driver='scsi-hd',
+                    id='virtio',
+                    bus='vio-scsi.0',
+                    drive='throttle')
+
+    def tearDown(self):
+        os.remove(test_img)
+        os.remove(target_img)
+
+    def test_throttled(self):
+        self.mirror_command('throttle')
+
+        time.sleep(1)
+        result = self.vm.qmp('query-block-jobs')
+        self.assert_qmp(result, 'return[0]/ready', False)
+
+        self.vm.qtest(f'clock_step {4 * 1000 * 1000 * 1000 * 1000}')
+        self.complete_and_wait('mirror')
+        self.vm.shutdown()
+        self.assertTrue(iotests.compare_images(test_img, target_img),
+                        'target image does not match source after mirroring')
+
+class TestThrottleDriveMirror(TestThrottle):
+    attachTargetImage = False
+
+    def mirror_command(self, device):
+        self.vm.cmd('drive-mirror',
+                    job_id='mirror',
+                    device=device,
+                    target=target_img,
+                    sync='full',
+                    mode='existing')
 
 if __name__ == '__main__':
     iotests.main(supported_fmts=['qcow2', 'qed'],
diff --git a/tests/qemu-iotests/041.out b/tests/qemu-iotests/041.out
index 46651953e8..96a0752f44 100644
--- a/tests/qemu-iotests/041.out
+++ b/tests/qemu-iotests/041.out
@@ -1,5 +1,5 @@
-...........................................................................................................
+.............................................................................................................
 ----------------------------------------------------------------------
-Ran 107 tests
+Ran 109 tests
 
 OK
-- 
2.47.3

[PATCH v2 2/3] blockdev: mirror: do not require a root block node

[Fiona Ebner]

The mirror block job currently only operates on a root block node. It
might be desired to use a throttle node as the root node for limiting
the guest IO, without also limiting mirror operations.

There is no obvious reason why mirror should only be able to operate
on a root block node.

When there is no explicit @replaces argument, implicit filters on top
are skipped. Allowing mirror to operate on non-root nodes does not
change that fact and keeps the behavior consistent with the
documentation for @replaces.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Changes in v2:
* Update QAPI description.

 blockdev.c           | 20 +++++++++++++++++---
 qapi/block-core.json |  4 ++--
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 6e86c6262f..0f70973326 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2987,13 +2987,19 @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp)
     bool target_is_zero;
     int ret;
 
-    bs = qmp_get_root_bs(arg->device, errp);
+    bdrv_graph_rdlock_main_loop();
+    bs = bdrv_lookup_bs(arg->device, arg->device, errp);
     if (!bs) {
+        bdrv_graph_rdunlock_main_loop();
+        return;
+    }
+    if (!bdrv_is_inserted(bs)) {
+        error_setg(errp, "Device has no medium");
+        bdrv_graph_rdunlock_main_loop();
         return;
     }
 
     /* Early check to avoid creating target */
-    bdrv_graph_rdlock_main_loop();
     if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_MIRROR_SOURCE, errp)) {
         bdrv_graph_rdunlock_main_loop();
         return;
@@ -3150,10 +3156,18 @@ void qmp_blockdev_mirror(const char *job_id,
     BlockMirrorBackingMode backing_mode = MIRROR_LEAVE_BACKING_CHAIN;
     int ret;
 
-    bs = qmp_get_root_bs(device, errp);
+    bdrv_graph_rdlock_main_loop();
+    bs = bdrv_lookup_bs(device, device, errp);
     if (!bs) {
+        bdrv_graph_rdunlock_main_loop();
         return;
     }
+    if (!bdrv_is_inserted(bs)) {
+        error_setg(errp, "Device has no medium");
+        bdrv_graph_rdunlock_main_loop();
+        return;
+    }
+    bdrv_graph_rdunlock_main_loop();
 
     target_bs = bdrv_lookup_bs(target, target, errp);
     if (!target_bs) {
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 741c8165c0..469297ea70 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2250,7 +2250,7 @@
 # @job-id: identifier for the newly-created block job.  If omitted,
 #     the device name will be used.  (Since 2.7)
 #
-# @device: the device name or node-name of a root node whose writes
+# @device: the device name or node-name of a block node whose writes
 #     should be mirrored.
 #
 # @target: the target of the new image.  If the file exists, or if it
@@ -2573,7 +2573,7 @@
 # @job-id: identifier for the newly-created block job.  If omitted,
 #     the device name will be used.  (Since 2.7)
 #
-# @device: The device name or node-name of a root node whose writes
+# @device: The device name or node-name of a block node whose writes
 #     should be mirrored.
 #
 # @target: the id or node-name of the block device to mirror to.  This
-- 
2.47.3

[PATCH v2 3/3] iotests/041: add tests for mirroring below throttle node

[Fiona Ebner]

Test mirror with a non-root node as a source. This restriction was
dropped with commit "blockdev: mirror: do not require a root block
node".

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

No changes in v2.

 tests/qemu-iotests/041     | 40 ++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/041.out |  4 ++--
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index 2552388690..abc070435d 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -1434,10 +1434,19 @@ class TestThrottle(iotests.QMPTestCase):
                     bus='vio-scsi.0',
                     drive='throttle')
 
+        self.drive_qdev = '/machine/peripheral/virtio'
+
     def tearDown(self):
         os.remove(test_img)
         os.remove(target_img)
 
+    def blockstats(self, device):
+        result = self.vm.qmp("query-blockstats")
+        for r in result['return']:
+            if r['node-name'] == device:
+                return r['stats']
+        raise Exception(f'Device {device} not found when querying blockstats')
+
     def test_throttled(self):
         self.mirror_command('throttle')
 
@@ -1451,6 +1460,37 @@ class TestThrottle(iotests.QMPTestCase):
         self.assertTrue(iotests.compare_images(test_img, target_img),
                         'target image does not match source after mirroring')
 
+    def test_below_throttle(self):
+        self.mirror_command('source')
+
+        # The throttling above should not block the mirror from completing.
+        self.complete_and_wait('mirror')
+        self.vm.shutdown()
+        self.assertTrue(iotests.compare_images(test_img, target_img),
+                        'target image does not match source after mirroring')
+
+    def test_below_throttle_with_io_on_throttle(self):
+        self.mirror_command('source')
+
+        reqs = 3
+        req_len = 1 * 1024
+
+        for i in range(0, reqs):
+            req = f'aio_write -P 7 {i}M {req_len}'
+            self.vm.hmp_qemu_io(self.drive_qdev, req, qdev=True)
+
+        self.wait_ready('mirror')
+        # The throttle node above should still have in-flight requests.
+        assert self.blockstats('throttle')['wr_bytes'] <= 1 * req_len
+
+        self.complete_and_wait('mirror', wait_ready=False)
+        assert self.blockstats('throttle')['wr_bytes'] == reqs * req_len
+        self.vm.shutdown()
+        # The throttle node above is drained too when the mirror finishes, so
+        # the images should match.
+        self.assertTrue(iotests.compare_images(test_img, target_img),
+                        'target image does not match source after mirroring')
+
 class TestThrottleDriveMirror(TestThrottle):
     attachTargetImage = False
 
diff --git a/tests/qemu-iotests/041.out b/tests/qemu-iotests/041.out
index 96a0752f44..ec055a9915 100644
--- a/tests/qemu-iotests/041.out
+++ b/tests/qemu-iotests/041.out
@@ -1,5 +1,5 @@
-.............................................................................................................
+.................................................................................................................
 ----------------------------------------------------------------------
-Ran 109 tests
+Ran 113 tests
 
 OK
-- 
2.47.3

Re: [PATCH v2 1/3] iotests/041: add test for mirror with throttle node as source

[Eric Blake]

On Tue, Oct 06, 2026 at 01:42:16PM +0200, Fiona Ebner wrote:
> Mostly in preparation for commit "iotests/041: add tests for mirroring
> below throttle node" which adds test cases for commit "blockdev:
> mirror: do not require a root block node".
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
> 
> Changes in v2:
> * Drop two accidentally included semicolons.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

Re: [PATCH v2 2/3] blockdev: mirror: do not require a root block node

[Eric Blake]

On Tue, Oct 06, 2026 at 01:42:17PM +0200, Fiona Ebner wrote:
> The mirror block job currently only operates on a root block node. It
> might be desired to use a throttle node as the root node for limiting
> the guest IO, without also limiting mirror operations.
> 
> There is no obvious reason why mirror should only be able to operate
> on a root block node.
> 
> When there is no explicit @replaces argument, implicit filters on top
> are skipped. Allowing mirror to operate on non-root nodes does not
> change that fact and keeps the behavior consistent with the
> documentation for @replaces.
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

Re: [PATCH v2 3/3] iotests/041: add tests for mirroring below throttle node

[Eric Blake]

On Tue, Oct 06, 2026 at 01:42:18PM +0200, Fiona Ebner wrote:
> Test mirror with a non-root node as a source. This restriction was
> dropped with commit "blockdev: mirror: do not require a root block
> node".
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org