From 6b89e851fabf78d7fb090bcdc71789ea1ef55c9b Mon Sep 17 00:00:00 2001 From: Fiona Ebner Date: Fri, 30 May 2025 17:11:01 +0200 Subject: block: add bdrv_graph_wrlock_drained() convenience wrapper Many write-locked sections are also drained sections. A new bdrv_graph_wrunlock_drained() wrapper around bdrv_graph_wrunlock() is introduced, which will begin a drained section first. A global variable is used so bdrv_graph_wrunlock() knows if it also needs to end such a drained section. Both the aio_poll call in bdrv_graph_wrlock() and the aio_bh_poll() in bdrv_graph_wrunlock() can re-enter a write-locked section. While for the latter, ending the drain could be moved to before the call, the former requires that the variable is a counter and not just a boolean. Since the wrapper calls bdrv_drain_all_begin(), which must be called with the graph unlocked, mark the wrapper as GRAPH_UNLOCKED too. The switch to the new helpers was generated with the following commands and then manually checked: find . -name '*.c' -exec sed -i -z 's/bdrv_drain_all_begin();\n\s*bdrv_graph_wrlock();/bdrv_graph_wrlock_drained();/g' {} ';' find . -name '*.c' -exec sed -i -z 's/bdrv_graph_wrunlock();\n\s*bdrv_drain_all_end();/bdrv_graph_wrunlock();/g' {} ';' Suggested-by: Kevin Wolf Signed-off-by: Fiona Ebner Message-ID: <20250530151125.955508-25-f.ebner@proxmox.com> [kwolf: Removed redundant GRAPH_UNLOCKED] Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- tests/unit/test-bdrv-drain.c | 36 +++++++++--------------------------- tests/unit/test-bdrv-graph-mod.c | 20 +++++--------------- 2 files changed, 14 insertions(+), 42 deletions(-) (limited to 'tests') diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c index 59c2793..3369c2c 100644 --- a/tests/unit/test-bdrv-drain.c +++ b/tests/unit/test-bdrv-drain.c @@ -772,11 +772,9 @@ static void test_blockjob_common_drain_node(enum drain_type drain_type, tjob->bs = src; job = &tjob->common; - bdrv_drain_all_begin(); - bdrv_graph_wrlock(); + bdrv_graph_wrlock_drained(); block_job_add_bdrv(job, "target", target, 0, BLK_PERM_ALL, &error_abort); bdrv_graph_wrunlock(); - bdrv_drain_all_end(); switch (result) { case TEST_JOB_SUCCESS: @@ -955,13 +953,11 @@ static void bdrv_test_top_close(BlockDriverState *bs) { BdrvChild *c, *next_c; - bdrv_drain_all_begin(); - bdrv_graph_wrlock(); + bdrv_graph_wrlock_drained(); QLIST_FOREACH_SAFE(c, &bs->children, next, next_c) { bdrv_unref_child(bs, c); } bdrv_graph_wrunlock(); - bdrv_drain_all_end(); } static int coroutine_fn GRAPH_RDLOCK @@ -1053,12 +1049,10 @@ static void do_test_delete_by_drain(bool detach_instead_of_delete, null_bs = bdrv_open("null-co://", NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL, &error_abort); - bdrv_drain_all_begin(); - bdrv_graph_wrlock(); + bdrv_graph_wrlock_drained(); bdrv_attach_child(bs, null_bs, "null-child", &child_of_bds, BDRV_CHILD_DATA, &error_abort); bdrv_graph_wrunlock(); - bdrv_drain_all_end(); /* This child will be the one to pass to requests through to, and * it will stall until a drain occurs */ @@ -1066,25 +1060,21 @@ static void do_test_delete_by_drain(bool detach_instead_of_delete, &error_abort); child_bs->total_sectors = 65536 >> BDRV_SECTOR_BITS; /* Takes our reference to child_bs */ - bdrv_drain_all_begin(); - bdrv_graph_wrlock(); + bdrv_graph_wrlock_drained(); tts->wait_child = bdrv_attach_child(bs, child_bs, "wait-child", &child_of_bds, BDRV_CHILD_DATA | BDRV_CHILD_PRIMARY, &error_abort); bdrv_graph_wrunlock(); - bdrv_drain_all_end(); /* This child is just there to be deleted * (for detach_instead_of_delete == true) */ null_bs = bdrv_open("null-co://", NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL, &error_abort); - bdrv_drain_all_begin(); - bdrv_graph_wrlock(); + bdrv_graph_wrlock_drained(); bdrv_attach_child(bs, null_bs, "null-child", &child_of_bds, BDRV_CHILD_DATA, &error_abort); bdrv_graph_wrunlock(); - bdrv_drain_all_end(); blk = blk_new(qemu_get_aio_context(), BLK_PERM_ALL, BLK_PERM_ALL); blk_insert_bs(blk, bs, &error_abort); @@ -1167,8 +1157,7 @@ static void no_coroutine_fn detach_indirect_bh(void *opaque) bdrv_dec_in_flight(data->child_b->bs); - bdrv_drain_all_begin(); - bdrv_graph_wrlock(); + bdrv_graph_wrlock_drained(); bdrv_unref_child(data->parent_b, data->child_b); bdrv_ref(data->c); @@ -1176,7 +1165,6 @@ static void no_coroutine_fn detach_indirect_bh(void *opaque) &child_of_bds, BDRV_CHILD_DATA, &error_abort); bdrv_graph_wrunlock(); - bdrv_drain_all_end(); } static void coroutine_mixed_fn detach_by_parent_aio_cb(void *opaque, int ret) @@ -1274,8 +1262,7 @@ static void TSA_NO_TSA test_detach_indirect(bool by_parent_cb) /* Set child relationships */ bdrv_ref(b); bdrv_ref(a); - bdrv_drain_all_begin(); - bdrv_graph_wrlock(); + bdrv_graph_wrlock_drained(); child_b = bdrv_attach_child(parent_b, b, "PB-B", &child_of_bds, BDRV_CHILD_DATA, &error_abort); child_a = bdrv_attach_child(parent_b, a, "PB-A", &child_of_bds, @@ -1286,7 +1273,6 @@ static void TSA_NO_TSA test_detach_indirect(bool by_parent_cb) by_parent_cb ? &child_of_bds : &detach_by_driver_cb_class, BDRV_CHILD_DATA, &error_abort); bdrv_graph_wrunlock(); - bdrv_drain_all_end(); g_assert_cmpint(parent_a->refcnt, ==, 1); g_assert_cmpint(parent_b->refcnt, ==, 1); @@ -1699,8 +1685,7 @@ static void test_drop_intermediate_poll(void) * Establish the chain last, so the chain links are the first * elements in the BDS.parents lists */ - bdrv_drain_all_begin(); - bdrv_graph_wrlock(); + bdrv_graph_wrlock_drained(); for (i = 0; i < 3; i++) { if (i) { /* Takes the reference to chain[i - 1] */ @@ -1709,7 +1694,6 @@ static void test_drop_intermediate_poll(void) } } bdrv_graph_wrunlock(); - bdrv_drain_all_end(); job = block_job_create("job", &test_simple_job_driver, NULL, job_node, 0, BLK_PERM_ALL, 0, 0, NULL, NULL, &error_abort); @@ -1956,12 +1940,10 @@ static void do_test_replace_child_mid_drain(int old_drain_count, new_child_bs->total_sectors = 1; bdrv_ref(old_child_bs); - bdrv_drain_all_begin(); - bdrv_graph_wrlock(); + bdrv_graph_wrlock_drained(); bdrv_attach_child(parent_bs, old_child_bs, "child", &child_of_bds, BDRV_CHILD_COW, &error_abort); bdrv_graph_wrunlock(); - bdrv_drain_all_end(); parent_s->setup_completed = true; for (i = 0; i < old_drain_count; i++) { diff --git a/tests/unit/test-bdrv-graph-mod.c b/tests/unit/test-bdrv-graph-mod.c index 7b03ebe..b077f0e 100644 --- a/tests/unit/test-bdrv-graph-mod.c +++ b/tests/unit/test-bdrv-graph-mod.c @@ -137,12 +137,10 @@ static void test_update_perm_tree(void) blk_insert_bs(root, bs, &error_abort); - bdrv_drain_all_begin(); - bdrv_graph_wrlock(); + bdrv_graph_wrlock_drained(); bdrv_attach_child(filter, bs, "child", &child_of_bds, BDRV_CHILD_DATA, &error_abort); bdrv_graph_wrunlock(); - bdrv_drain_all_end(); ret = bdrv_append(filter, bs, NULL); g_assert_cmpint(ret, <, 0); @@ -206,13 +204,11 @@ static void test_should_update_child(void) bdrv_set_backing_hd(target, bs, &error_abort); - bdrv_drain_all_begin(); - bdrv_graph_wrlock(); + bdrv_graph_wrlock_drained(); g_assert(target->backing->bs == bs); bdrv_attach_child(filter, target, "target", &child_of_bds, BDRV_CHILD_DATA, &error_abort); bdrv_graph_wrunlock(); - bdrv_drain_all_end(); bdrv_append(filter, bs, &error_abort); bdrv_graph_rdlock_main_loop(); @@ -248,8 +244,7 @@ static void test_parallel_exclusive_write(void) bdrv_ref(base); bdrv_ref(fl1); - bdrv_drain_all_begin(); - bdrv_graph_wrlock(); + bdrv_graph_wrlock_drained(); bdrv_attach_child(top, fl1, "backing", &child_of_bds, BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY, &error_abort); @@ -262,7 +257,6 @@ static void test_parallel_exclusive_write(void) bdrv_replace_node(fl1, fl2, &error_abort); bdrv_graph_wrunlock(); - bdrv_drain_all_end(); bdrv_drained_end(fl2); bdrv_drained_end(fl1); @@ -369,8 +363,7 @@ static void test_parallel_perm_update(void) */ bdrv_ref(base); - bdrv_drain_all_begin(); - bdrv_graph_wrlock(); + bdrv_graph_wrlock_drained(); bdrv_attach_child(top, ws, "file", &child_of_bds, BDRV_CHILD_DATA, &error_abort); c_fl1 = bdrv_attach_child(ws, fl1, "first", &child_of_bds, @@ -384,7 +377,6 @@ static void test_parallel_perm_update(void) BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY, &error_abort); bdrv_graph_wrunlock(); - bdrv_drain_all_end(); /* Select fl1 as first child to be active */ s->selected = c_fl1; @@ -438,13 +430,11 @@ static void test_append_greedy_filter(void) BlockDriverState *base = no_perm_node("base"); BlockDriverState *fl = exclusive_writer_node("fl1"); - bdrv_drain_all_begin(); - bdrv_graph_wrlock(); + bdrv_graph_wrlock_drained(); bdrv_attach_child(top, base, "backing", &child_of_bds, BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY, &error_abort); bdrv_graph_wrunlock(); - bdrv_drain_all_end(); bdrv_append(fl, base, &error_abort); bdrv_unref(fl); -- cgit v1.1 From 54eb59d668d6e4e7584188628ca44f3e9bd39d17 Mon Sep 17 00:00:00 2001 From: Fiona Ebner Date: Fri, 30 May 2025 17:11:07 +0200 Subject: block: drop wrapper for bdrv_set_backing_hd_drained() Nearly all callers (outside of the tests) are already using the _drained() variant of the function. It doesn't seem worth keeping. Simply adapt the remaining callers of bdrv_set_backing_hd() and rename bdrv_set_backing_hd_drained() to bdrv_set_backing_hd(). Signed-off-by: Fiona Ebner Message-ID: <20250530151125.955508-31-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- tests/unit/test-bdrv-drain.c | 12 +++++++++++- tests/unit/test-bdrv-graph-mod.c | 2 +- 2 files changed, 12 insertions(+), 2 deletions(-) (limited to 'tests') diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c index 3369c2c..43b0ba8 100644 --- a/tests/unit/test-bdrv-drain.c +++ b/tests/unit/test-bdrv-drain.c @@ -193,7 +193,9 @@ static BlockBackend * no_coroutine_fn test_setup(void) blk_insert_bs(blk, bs, &error_abort); backing = bdrv_new_open_driver(&bdrv_test, "backing", 0, &error_abort); + bdrv_graph_wrlock_drained(); bdrv_set_backing_hd(bs, backing, &error_abort); + bdrv_graph_wrunlock(); bdrv_unref(backing); bdrv_unref(bs); @@ -386,7 +388,9 @@ static void test_nested(void) backing = bdrv_new_open_driver(&bdrv_test, "backing", 0, &error_abort); backing_s = backing->opaque; + bdrv_graph_wrlock_drained(); bdrv_set_backing_hd(bs, backing, &error_abort); + bdrv_graph_wrunlock(); for (outer = 0; outer < DRAIN_TYPE_MAX; outer++) { for (inner = 0; inner < DRAIN_TYPE_MAX; inner++) { @@ -733,10 +737,12 @@ static void test_blockjob_common_drain_node(enum drain_type drain_type, src_overlay = bdrv_new_open_driver(&bdrv_test, "source-overlay", BDRV_O_RDWR, &error_abort); + bdrv_graph_wrlock_drained(); bdrv_set_backing_hd(src_overlay, src, &error_abort); bdrv_unref(src); bdrv_set_backing_hd(src, src_backing, &error_abort); bdrv_unref(src_backing); + bdrv_graph_wrunlock(); blk_src = blk_new(qemu_get_aio_context(), BLK_PERM_ALL, BLK_PERM_ALL); blk_insert_bs(blk_src, src_overlay, &error_abort); @@ -1436,8 +1442,10 @@ static void test_drop_backing_job_commit(Job *job) TestDropBackingBlockJob *s = container_of(job, TestDropBackingBlockJob, common.job); + bdrv_graph_wrlock_drained(); bdrv_set_backing_hd(s->bs, NULL, &error_abort); bdrv_set_backing_hd(s->detach_also, NULL, &error_abort); + bdrv_graph_wrunlock(); *s->did_complete = true; } @@ -1530,7 +1538,9 @@ static void test_blockjob_commit_by_drained_end(void) snprintf(name, sizeof(name), "parent-node-%i", i); bs_parents[i] = bdrv_new_open_driver(&bdrv_test, name, BDRV_O_RDWR, &error_abort); + bdrv_graph_wrlock_drained(); bdrv_set_backing_hd(bs_parents[i], bs_child, &error_abort); + bdrv_graph_wrunlock(); } job = block_job_create("job", &test_drop_backing_job_driver, NULL, @@ -1679,13 +1689,13 @@ static void test_drop_intermediate_poll(void) job_node = bdrv_new_open_driver(&bdrv_test, "job-node", BDRV_O_RDWR, &error_abort); + bdrv_graph_wrlock_drained(); bdrv_set_backing_hd(job_node, chain[1], &error_abort); /* * Establish the chain last, so the chain links are the first * elements in the BDS.parents lists */ - bdrv_graph_wrlock_drained(); for (i = 0; i < 3; i++) { if (i) { /* Takes the reference to chain[i - 1] */ diff --git a/tests/unit/test-bdrv-graph-mod.c b/tests/unit/test-bdrv-graph-mod.c index b077f0e..567db99 100644 --- a/tests/unit/test-bdrv-graph-mod.c +++ b/tests/unit/test-bdrv-graph-mod.c @@ -202,9 +202,9 @@ static void test_should_update_child(void) blk_insert_bs(root, bs, &error_abort); + bdrv_graph_wrlock_drained(); bdrv_set_backing_hd(target, bs, &error_abort); - bdrv_graph_wrlock_drained(); g_assert(target->backing->bs == bs); bdrv_attach_child(filter, target, "target", &child_of_bds, BDRV_CHILD_DATA, &error_abort); -- cgit v1.1 From cfac5a963e4bf287a194b5df80b7984bc8e41221 Mon Sep 17 00:00:00 2001 From: Fiona Ebner Date: Wed, 2 Jul 2025 14:31:27 +0200 Subject: block/qapi: include child references in block device info In combination with using a throttle filter to enforce IO limits for a guest device, knowing the 'file' child of a block device can be useful. If the throttle filter is only intended for guest IO, block jobs should not also be limited by the throttle filter, so the block operations need to be done with the 'file' child of the top throttle node as the target. In combination with mirroring, the name of that child is not fixed. Another scenario is when unplugging a guest device after mirroring below a top throttle node, where the mirror target is added explicitly via blockdev-add. After mirroring, the target becomes the new 'file' child of the throttle node. For unplugging, both the top throttle node and the mirror target need to be deleted, because only implicitly added child nodes are deleted automatically, and the current 'file' child of the throttle node was explicitly added (as the mirror target). In other scenarios, it could be useful to follow the backing chain. Note that iotests 191 and 273 use _filter_img_info, so the 'children' information is filtered out there. Signed-off-by: Fiona Ebner Message-ID: <20250702123204.325470-2-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- tests/qemu-iotests/184.out | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'tests') diff --git a/tests/qemu-iotests/184.out b/tests/qemu-iotests/184.out index 52692b6..ef99bb2 100644 --- a/tests/qemu-iotests/184.out +++ b/tests/qemu-iotests/184.out @@ -41,6 +41,12 @@ Testing: }, "iops_wr": 0, "ro": false, + "children": [ + { + "node-name": "disk0", + "child": "file" + } + ], "node-name": "throttle0", "backing_file_depth": 1, "drv": "throttle", @@ -69,6 +75,8 @@ Testing: }, "iops_wr": 0, "ro": false, + "children": [ + ], "node-name": "disk0", "backing_file_depth": 0, "drv": "null-co", -- cgit v1.1 From 96acc034ffffc8d7dc0cf3dfbf2996cbdbe3dde2 Mon Sep 17 00:00:00 2001 From: Fiona Ebner Date: Thu, 5 Jun 2025 12:09:38 +0200 Subject: iotests: add test for changing the 'drive' property via 'qom-set' Signed-off-by: Fiona Ebner Message-ID: <20250605100938.43133-1-f.ebner@proxmox.com> [kwolf: Fixed up pylint warnings flagged by 297] Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- tests/qemu-iotests/tests/qom-set-drive | 75 ++++++++++++++++++++++++++++++ tests/qemu-iotests/tests/qom-set-drive.out | 11 +++++ 2 files changed, 86 insertions(+) create mode 100755 tests/qemu-iotests/tests/qom-set-drive create mode 100644 tests/qemu-iotests/tests/qom-set-drive.out (limited to 'tests') diff --git a/tests/qemu-iotests/tests/qom-set-drive b/tests/qemu-iotests/tests/qom-set-drive new file mode 100755 index 0000000..ec8ddac --- /dev/null +++ b/tests/qemu-iotests/tests/qom-set-drive @@ -0,0 +1,75 @@ +#!/usr/bin/env python3 +# group: quick +# +# Test how changing the 'drive' property via 'qom-set' behaves. +# +# Copyright (C) Proxmox Server Solutions GmbH +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +import os +import iotests +from iotests import imgfmt, log, qemu_img_create, QMPTestCase + +image_size = 1 * 1024 * 1024 +images = [os.path.join(iotests.test_dir, f'{i}.img') for i in range(0, 4)] + +class TestQOMSetDrive(QMPTestCase): + def setUp(self) -> None: + for image in images: + qemu_img_create('-f', imgfmt, image, str(image_size)) + + self.vm = iotests.VM() + for i, image in enumerate(images): + self.vm.add_blockdev(self.vm.qmp_to_opts({ + 'driver': imgfmt, + 'node-name': f'node{i}', + 'file': { + 'driver': 'file', + 'filename': image, + } + })) + self.vm.add_object('iothread,id=iothread0') + self.vm.add_device('virtio-scsi,iothread=iothread0') + self.vm.add_device('scsi-hd,id=iot,drive=node0') + self.vm.add_device('virtio-scsi') + self.vm.add_device('scsi-hd,id=no-iot,drive=node1') + self.vm.launch() + + def tearDown(self) -> None: + self.vm.shutdown() + for image in images: + os.remove(image) + + def test_qom_set_drive(self) -> None: + log(self.vm.qmp('qom-get', path='/machine/peripheral/iot', + property='drive')) + log(self.vm.qmp('qom-set', path='/machine/peripheral/iot', + property='drive', value='node2')) + log(self.vm.qmp('qom-get', path='/machine/peripheral/iot', + property='drive')) + + log(self.vm.qmp('qom-get', path='/machine/peripheral/no-iot', + property='drive')) + log(self.vm.qmp('qom-set', path='/machine/peripheral/no-iot', + property='drive', value='node3')) + log(self.vm.qmp('qom-get', path='/machine/peripheral/no-iot', + property='drive')) + +if __name__ == '__main__': + iotests.activate_logging() + # LUKS would require special key-secret handling in add_blockdevs() + iotests.main(supported_fmts=['generic'], + unsupported_fmts=['luks']) diff --git a/tests/qemu-iotests/tests/qom-set-drive.out b/tests/qemu-iotests/tests/qom-set-drive.out new file mode 100644 index 0000000..7fc243d --- /dev/null +++ b/tests/qemu-iotests/tests/qom-set-drive.out @@ -0,0 +1,11 @@ +{"return": "node0"} +{"error": {"class": "GenericError", "desc": "Different aio context is not supported for new node"}} +{"return": "node0"} +{"return": "node1"} +{"return": {}} +{"return": "node3"} +. +---------------------------------------------------------------------- +Ran 1 tests + +OK -- cgit v1.1 From a03cd2982d84c6901f3583e31ef12ac7fd170052 Mon Sep 17 00:00:00 2001 From: Michael Tokarev Date: Sat, 31 May 2025 20:15:48 +0300 Subject: qemu-img: factor out parse_output_format() and use it in the code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use common code and simplify error message Signed-off-by: Michael Tokarev Reviewed-by: Daniel P. Berrangé Reviewed-by: Kevin Wolf Message-ID: <20250531171609.197078-7-mjt@tls.msk.ru> Signed-off-by: Kevin Wolf --- tests/qemu-iotests/178 | 2 +- tests/qemu-iotests/178.out.qcow2 | 3 ++- tests/qemu-iotests/178.out.raw | 3 ++- tests/qemu-iotests/common.filter | 6 ++++++ 4 files changed, 11 insertions(+), 3 deletions(-) (limited to 'tests') diff --git a/tests/qemu-iotests/178 b/tests/qemu-iotests/178 index 8df241e..463c59a 100755 --- a/tests/qemu-iotests/178 +++ b/tests/qemu-iotests/178 @@ -58,7 +58,7 @@ $QEMU_IMG measure -f qcow2 # missing filename $QEMU_IMG measure -l snap1 # missing filename $QEMU_IMG measure -o , # invalid option list $QEMU_IMG measure -l snapshot.foo=bar # invalid snapshot option -$QEMU_IMG measure --output foo # invalid output format +$QEMU_IMG measure --output foo 2>&1 | _filter_qemu_img # invalid output format $QEMU_IMG measure --size -1 # invalid image size $QEMU_IMG measure -O foo "$TEST_IMG" # unknown image file format diff --git a/tests/qemu-iotests/178.out.qcow2 b/tests/qemu-iotests/178.out.qcow2 index fe193fd..61506b5 100644 --- a/tests/qemu-iotests/178.out.qcow2 +++ b/tests/qemu-iotests/178.out.qcow2 @@ -12,7 +12,8 @@ qemu-img: --image-opts, -f, and -l 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' -qemu-img: --output must be used with human or json as argument. +qemu-img: --output expects 'human' or 'json', not 'foo' +Try 'qemu-img measure --help' for more information qemu-img: Invalid image size specified. Must be between 0 and 9223372036854775807. qemu-img: Unknown file format 'foo' diff --git a/tests/qemu-iotests/178.out.raw b/tests/qemu-iotests/178.out.raw index 445e460..6d994a4 100644 --- a/tests/qemu-iotests/178.out.raw +++ b/tests/qemu-iotests/178.out.raw @@ -12,7 +12,8 @@ qemu-img: --image-opts, -f, and -l 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' -qemu-img: --output must be used with human or json as argument. +qemu-img: --output expects 'human' or 'json', not 'foo' +Try 'qemu-img measure --help' for more information qemu-img: Invalid image size specified. Must be between 0 and 9223372036854775807. qemu-img: Unknown file format 'foo' diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter index fc3c64b..67f819d 100644 --- a/tests/qemu-iotests/common.filter +++ b/tests/qemu-iotests/common.filter @@ -86,6 +86,12 @@ _filter_qemu() -e $'s#\r##' # QEMU monitor uses \r\n line endings } +# replace occurrences of QEMU_IMG_PROG with "qemu-img" +_filter_qemu_img() +{ + sed -e "s#$QEMU_IMG_PROG#qemu-img#g" +} + # replace problematic QMP output like timestamps _filter_qmp() { -- cgit v1.1 From a79d282fc9e1dfa887b13e079279277ea257162a Mon Sep 17 00:00:00 2001 From: Michael Tokarev Date: Sat, 31 May 2025 20:15:51 +0300 Subject: qemu-img: commit: refresh options/--help Add missing long options and --help output, reorder options for consistency. Signed-off-by: Michael Tokarev Message-ID: <20250531171609.197078-10-mjt@tls.msk.ru> [kwolf: Fixed up qemu-iotests] Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- tests/qemu-iotests/153 | 2 +- tests/qemu-iotests/153.out | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) (limited to 'tests') diff --git a/tests/qemu-iotests/153 b/tests/qemu-iotests/153 index 9bc3be8..1e02f6a 100755 --- a/tests/qemu-iotests/153 +++ b/tests/qemu-iotests/153 @@ -63,7 +63,7 @@ _supported_proto file _run_cmd() { echo - (echo "$@"; "$@" 2>&1 1>/dev/null) | _filter_testdir + (echo "$@"; "$@" 2>&1 1>/dev/null) | _filter_testdir | _filter_qemu_img } _do_run_qemu() diff --git a/tests/qemu-iotests/153.out b/tests/qemu-iotests/153.out index ff8e558..a8ffd01 100644 --- a/tests/qemu-iotests/153.out +++ b/tests/qemu-iotests/153.out @@ -124,8 +124,8 @@ qemu-img: unrecognized option '-U' Try 'qemu-img --help' for more information _qemu_img_wrapper commit -U TEST_DIR/t.qcow2 -qemu-img: unrecognized option '-U' -Try 'qemu-img --help' for more information +qemu-img commit: invalid option -- 'U' +Try 'qemu-img commit --help' for more information _qemu_img_wrapper resize -U TEST_DIR/t.qcow2 32M qemu-img: unrecognized option '-U' @@ -248,8 +248,8 @@ qemu-img: unrecognized option '-U' Try 'qemu-img --help' for more information _qemu_img_wrapper commit -U TEST_DIR/t.qcow2 -qemu-img: unrecognized option '-U' -Try 'qemu-img --help' for more information +qemu-img commit: invalid option -- 'U' +Try 'qemu-img commit --help' for more information _qemu_img_wrapper resize -U TEST_DIR/t.qcow2 32M qemu-img: unrecognized option '-U' @@ -353,8 +353,8 @@ qemu-img: unrecognized option '-U' Try 'qemu-img --help' for more information _qemu_img_wrapper commit -U TEST_DIR/t.qcow2 -qemu-img: unrecognized option '-U' -Try 'qemu-img --help' for more information +qemu-img commit: invalid option -- 'U' +Try 'qemu-img commit --help' for more information _qemu_img_wrapper resize -U TEST_DIR/t.qcow2 32M qemu-img: unrecognized option '-U' -- cgit v1.1 From a8ce9adfe1b30999b2f51cec61d44e525365077d Mon Sep 17 00:00:00 2001 From: Michael Tokarev Date: Sat, 31 May 2025 20:16:02 +0300 Subject: qemu-img: resize: refresh options/--help Add missing long options and --help output, reorder options for consistency. Signed-off-by: Michael Tokarev Message-ID: <20250531171609.197078-21-mjt@tls.msk.ru> [kwolf: Fixed up qemu-iotests] Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- tests/qemu-iotests/153.out | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'tests') diff --git a/tests/qemu-iotests/153.out b/tests/qemu-iotests/153.out index a8ffd01..ac3e340 100644 --- a/tests/qemu-iotests/153.out +++ b/tests/qemu-iotests/153.out @@ -128,8 +128,8 @@ qemu-img commit: invalid option -- 'U' Try 'qemu-img commit --help' for more information _qemu_img_wrapper resize -U TEST_DIR/t.qcow2 32M -qemu-img: unrecognized option '-U' -Try 'qemu-img --help' for more information +qemu-img resize: invalid option -- 'U' +Try 'qemu-img resize --help' for more information _qemu_img_wrapper rebase -U TEST_DIR/t.qcow2 -b TEST_DIR/t.qcow2.base -F qcow2 qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to get "write" lock @@ -252,8 +252,8 @@ qemu-img commit: invalid option -- 'U' Try 'qemu-img commit --help' for more information _qemu_img_wrapper resize -U TEST_DIR/t.qcow2 32M -qemu-img: unrecognized option '-U' -Try 'qemu-img --help' for more information +qemu-img resize: invalid option -- 'U' +Try 'qemu-img resize --help' for more information _qemu_img_wrapper rebase -U TEST_DIR/t.qcow2 -b TEST_DIR/t.qcow2.base -F qcow2 qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to get "write" lock @@ -357,8 +357,8 @@ qemu-img commit: invalid option -- 'U' Try 'qemu-img commit --help' for more information _qemu_img_wrapper resize -U TEST_DIR/t.qcow2 32M -qemu-img: unrecognized option '-U' -Try 'qemu-img --help' for more information +qemu-img resize: invalid option -- 'U' +Try 'qemu-img resize --help' for more information _qemu_img_wrapper rebase -U TEST_DIR/t.qcow2 -b TEST_DIR/t.qcow2.base -F qcow2 -- cgit v1.1 From 03b5de62d2af64c8349c62909bc7d11641491b24 Mon Sep 17 00:00:00 2001 From: Michael Tokarev Date: Sat, 31 May 2025 20:16:03 +0300 Subject: qemu-img: amend: refresh options/--help Add missing long options and --help output, reorder options for consistency. Signed-off-by: Michael Tokarev Message-ID: <20250531171609.197078-22-mjt@tls.msk.ru> [kwolf: Fixed up qemu-iotests] Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- tests/qemu-iotests/153.out | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'tests') diff --git a/tests/qemu-iotests/153.out b/tests/qemu-iotests/153.out index ac3e340..28e1a22 100644 --- a/tests/qemu-iotests/153.out +++ b/tests/qemu-iotests/153.out @@ -120,8 +120,8 @@ _qemu_img_wrapper compare -U TEST_DIR/t.qcow2 TEST_DIR/t.qcow2 _qemu_img_wrapper map -U TEST_DIR/t.qcow2 _qemu_img_wrapper amend -o size=32M -U TEST_DIR/t.qcow2 -qemu-img: unrecognized option '-U' -Try 'qemu-img --help' for more information +qemu-img amend: invalid option -- 'U' +Try 'qemu-img amend --help' for more information _qemu_img_wrapper commit -U TEST_DIR/t.qcow2 qemu-img commit: invalid option -- 'U' @@ -244,8 +244,8 @@ _qemu_img_wrapper compare -U TEST_DIR/t.qcow2 TEST_DIR/t.qcow2 _qemu_img_wrapper map -U TEST_DIR/t.qcow2 _qemu_img_wrapper amend -o size=32M -U TEST_DIR/t.qcow2 -qemu-img: unrecognized option '-U' -Try 'qemu-img --help' for more information +qemu-img amend: invalid option -- 'U' +Try 'qemu-img amend --help' for more information _qemu_img_wrapper commit -U TEST_DIR/t.qcow2 qemu-img commit: invalid option -- 'U' @@ -349,8 +349,8 @@ _qemu_img_wrapper compare -U TEST_DIR/t.qcow2 TEST_DIR/t.qcow2 _qemu_img_wrapper map -U TEST_DIR/t.qcow2 _qemu_img_wrapper amend -o size=32M -U TEST_DIR/t.qcow2 -qemu-img: unrecognized option '-U' -Try 'qemu-img --help' for more information +qemu-img amend: invalid option -- 'U' +Try 'qemu-img amend --help' for more information _qemu_img_wrapper commit -U TEST_DIR/t.qcow2 qemu-img commit: invalid option -- 'U' -- cgit v1.1 From d7bd47bf84707d21324ad079c5aa8cca16fd138f Mon Sep 17 00:00:00 2001 From: Michael Tokarev Date: Sat, 31 May 2025 20:16:09 +0300 Subject: qemu-img: extend cvtnum() and use it in more places MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cvtnum() expects input string to specify some sort of size (optionally with KMG... suffix). However, there are a lot of other number conversions in there (using qemu_strtol &Co), also, not all conversions which use cvtnum, actually expects size, - like dd count=nn. Add bool is_size argument to cvtnum() to specify if it should treat the argument as a size or something else, - this changes conversion routine in use and error text. Use the new cvtnum() in more places (like where strtol were used), since it never return negative number in successful conversion. When it makes sense, also specify upper or lower bounds at the same time. This simplifies option processing in multiple places, removing the need of local temporary variables and longer error reporting code. While at it, fix errors, like depth in measure must be >= 1, while the previous code allowed it to be 0. In a few places, change unsigned variables (like of type size_t) to be signed instead, - to avoid the need of temporary conversion variable. All these variables are okay to be signed, we never assign <0 value to them except of the cases of conversion error, where we return immediately. While at it, remove allowed size suffixes from the error message as it makes no sense most of the time (should be in help instead). Signed-off-by: Michael Tokarev Reviewed-by: Daniel P. Berrangé Message-ID: <20250531171609.197078-28-mjt@tls.msk.ru> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- tests/qemu-iotests/049.out | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'tests') diff --git a/tests/qemu-iotests/049.out b/tests/qemu-iotests/049.out index 34e1b45..70c6275 100644 --- a/tests/qemu-iotests/049.out +++ b/tests/qemu-iotests/049.out @@ -98,8 +98,7 @@ qemu-img create -f qcow2 -o size=-1024 TEST_DIR/t.qcow2 qemu-img: TEST_DIR/t.qcow2: Value '-1024' is out of range for parameter 'size' qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- -1k -qemu-img: Invalid image size specified. You may use k, M, G, T, P or E suffixes for -qemu-img: kilobytes, megabytes, gigabytes, terabytes, petabytes and exabytes. +qemu-img: Invalid image size specified: '-1k' qemu-img create -f qcow2 -o size=-1k TEST_DIR/t.qcow2 qemu-img: TEST_DIR/t.qcow2: Parameter 'size' expects a non-negative number below 2^64 @@ -107,8 +106,7 @@ Optional suffix k, M, G, T, P or E means kilo-, mega-, giga-, tera-, peta- and exabytes, respectively. qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- 1kilobyte -qemu-img: Invalid image size specified. You may use k, M, G, T, P or E suffixes for -qemu-img: kilobytes, megabytes, gigabytes, terabytes, petabytes and exabytes. +qemu-img: Invalid image size specified: '1kilobyte' qemu-img create -f qcow2 -o size=1kilobyte TEST_DIR/t.qcow2 qemu-img: TEST_DIR/t.qcow2: Parameter 'size' expects a non-negative number below 2^64 @@ -116,8 +114,7 @@ Optional suffix k, M, G, T, P or E means kilo-, mega-, giga-, tera-, peta- and exabytes, respectively. qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- foobar -qemu-img: Invalid image size specified. You may use k, M, G, T, P or E suffixes for -qemu-img: kilobytes, megabytes, gigabytes, terabytes, petabytes and exabytes. +qemu-img: Invalid image size specified: 'foobar' qemu-img create -f qcow2 -o size=foobar TEST_DIR/t.qcow2 qemu-img: TEST_DIR/t.qcow2: Parameter 'size' expects a non-negative number below 2^64 -- cgit v1.1