From ae5a40e8581185654a667fbbf7e4adbc2a2a3e45 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 13 Mar 2024 16:30:00 +0100 Subject: [PATCH 01/15] mirror: Don't call job_pause_point() under graph lock Calling job_pause_point() while holding the graph reader lock potentially results in a deadlock: bdrv_graph_wrlock() first drains everything, including the mirror job, which pauses it. The job is only unpaused at the end of the drain section, which is when the graph writer lock has been successfully taken. However, if the job happens to be paused at a pause point where it still holds the reader lock, the writer lock can't be taken as long as the job is still paused. Mark job_pause_point() as GRAPH_UNLOCKED and fix mirror accordingly. Cc: qemu-stable@nongnu.org Buglink: https://issues.redhat.com/browse/RHEL-28125 Fixes: 004915a96a7a ("block: Protect bs->backing with graph_lock") Signed-off-by: Kevin Wolf Message-ID: <20240313153000.33121-1-kwolf@redhat.com> Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- block/mirror.c | 10 ++++++---- include/qemu/job.h | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index 5145eb53e1..1bdce3b657 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -479,9 +479,9 @@ static unsigned mirror_perform(MirrorBlockJob *s, int64_t offset, return bytes_handled; } -static void coroutine_fn GRAPH_RDLOCK mirror_iteration(MirrorBlockJob *s) +static void coroutine_fn GRAPH_UNLOCKED mirror_iteration(MirrorBlockJob *s) { - BlockDriverState *source = s->mirror_top_bs->backing->bs; + BlockDriverState *source; MirrorOp *pseudo_op; int64_t offset; /* At least the first dirty chunk is mirrored in one iteration. */ @@ -489,6 +489,10 @@ static void coroutine_fn GRAPH_RDLOCK mirror_iteration(MirrorBlockJob *s) bool write_zeroes_ok = bdrv_can_write_zeroes_with_unmap(blk_bs(s->target)); int max_io_bytes = MAX(s->buf_size / MAX_IN_FLIGHT, MAX_IO_BYTES); + bdrv_graph_co_rdlock(); + source = s->mirror_top_bs->backing->bs; + bdrv_graph_co_rdunlock(); + bdrv_dirty_bitmap_lock(s->dirty_bitmap); offset = bdrv_dirty_iter_next(s->dbi); if (offset < 0) { @@ -1066,9 +1070,7 @@ static int coroutine_fn mirror_run(Job *job, Error **errp) mirror_wait_for_free_in_flight_slot(s); continue; } else if (cnt != 0) { - bdrv_graph_co_rdlock(); mirror_iteration(s); - bdrv_graph_co_rdunlock(); } } diff --git a/include/qemu/job.h b/include/qemu/job.h index 9ea98b5927..2b873f2576 100644 --- a/include/qemu/job.h +++ b/include/qemu/job.h @@ -483,7 +483,7 @@ void job_enter(Job *job); * * Called with job_mutex *not* held. */ -void coroutine_fn job_pause_point(Job *job); +void coroutine_fn GRAPH_UNLOCKED job_pause_point(Job *job); /** * @job: The job that calls the function. From 9c707525cbb1dd1e56876e45c70c0c08f2876d41 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 14 Mar 2024 17:58:24 +0100 Subject: [PATCH 02/15] nbd/server: Fix race in draining the export When draining an NBD export, nbd_drained_begin() first sets client->quiescing so that nbd_client_receive_next_request() won't start any new request coroutines. Then nbd_drained_poll() tries to makes sure that we wait for any existing request coroutines by checking that client->nb_requests has become 0. However, there is a small window between creating a new request coroutine and increasing client->nb_requests. If a coroutine is in this state, it won't be waited for and drain returns too early. In the context of switching to a different AioContext, this means that blk_aio_attached() will see client->recv_coroutine != NULL and fail its assertion. Fix this by increasing client->nb_requests immediately when starting the coroutine. Doing this after the checks if we should create a new coroutine is okay because client->lock is held. Cc: qemu-stable@nongnu.org Fixes: fd6afc501a01 ("nbd/server: Use drained block ops to quiesce the server") Signed-off-by: Kevin Wolf Message-ID: <20240314165825.40261-2-kwolf@redhat.com> Signed-off-by: Kevin Wolf --- nbd/server.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/nbd/server.c b/nbd/server.c index 941832f178..c3484cc1eb 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -3007,8 +3007,8 @@ static coroutine_fn int nbd_handle_request(NBDClient *client, /* Owns a reference to the NBDClient passed as opaque. */ static coroutine_fn void nbd_trip(void *opaque) { - NBDClient *client = opaque; - NBDRequestData *req = NULL; + NBDRequestData *req = opaque; + NBDClient *client = req->client; NBDRequest request = { 0 }; /* GCC thinks it can be used uninitialized */ int ret; Error *local_err = NULL; @@ -3037,8 +3037,6 @@ static coroutine_fn void nbd_trip(void *opaque) goto done; } - req = nbd_request_get(client); - /* * nbd_co_receive_request() returns -EAGAIN when nbd_drained_begin() has * set client->quiescing but by the time we get back nbd_drained_end() may @@ -3112,9 +3110,7 @@ static coroutine_fn void nbd_trip(void *opaque) } done: - if (req) { - nbd_request_put(req); - } + nbd_request_put(req); qemu_mutex_unlock(&client->lock); @@ -3143,10 +3139,13 @@ disconnect: */ static void nbd_client_receive_next_request(NBDClient *client) { + NBDRequestData *req; + if (!client->recv_coroutine && client->nb_requests < MAX_NBD_REQUESTS && !client->quiescing) { nbd_client_get(client); - client->recv_coroutine = qemu_coroutine_create(nbd_trip, client); + req = nbd_request_get(client); + client->recv_coroutine = qemu_coroutine_create(nbd_trip, req); aio_co_schedule(client->exp->common.ctx, client->recv_coroutine); } } From e8fce34eccf68a32f4ecf2c6f121ff2ac383d6bf Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 14 Mar 2024 17:58:25 +0100 Subject: [PATCH 03/15] iotests: Add test for reset/AioContext switches with NBD exports This replicates the scenario in which the bug was reported. Unfortunately this relies on actually executing a guest (so that the firmware initialises the virtio-blk device and moves it to its configured iothread), so this can't make use of the qtest accelerator like most other test cases. I tried to find a different easy way to trigger the bug, but couldn't find one. Signed-off-by: Kevin Wolf Message-ID: <20240314165825.40261-3-kwolf@redhat.com> Signed-off-by: Kevin Wolf --- tests/qemu-iotests/tests/iothreads-nbd-export | 66 +++++++++++++++++++ .../tests/iothreads-nbd-export.out | 19 ++++++ 2 files changed, 85 insertions(+) create mode 100755 tests/qemu-iotests/tests/iothreads-nbd-export create mode 100644 tests/qemu-iotests/tests/iothreads-nbd-export.out diff --git a/tests/qemu-iotests/tests/iothreads-nbd-export b/tests/qemu-iotests/tests/iothreads-nbd-export new file mode 100755 index 0000000000..037260729c --- /dev/null +++ b/tests/qemu-iotests/tests/iothreads-nbd-export @@ -0,0 +1,66 @@ +#!/usr/bin/env python3 +# group: rw quick +# +# Copyright (C) 2024 Red Hat, Inc. +# +# 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 . +# +# Creator/Owner: Kevin Wolf + +import time +import qemu +import iotests + +iotests.script_initialize(supported_fmts=['qcow2'], + supported_platforms=['linux']) + +with iotests.FilePath('disk1.img') as path, \ + iotests.FilePath('nbd.sock', base_dir=iotests.sock_dir) as nbd_sock, \ + qemu.machine.QEMUMachine(iotests.qemu_prog) as vm: + + img_size = '10M' + + iotests.log('Preparing disk...') + iotests.qemu_img_create('-f', iotests.imgfmt, path, img_size) + vm.add_args('-blockdev', f'file,node-name=disk-file,filename={path}') + vm.add_args('-blockdev', 'qcow2,node-name=disk,file=disk-file') + vm.add_args('-object', 'iothread,id=iothread0') + vm.add_args('-device', + 'virtio-blk,drive=disk,iothread=iothread0,share-rw=on') + + iotests.log('Launching VM...') + vm.add_args('-accel', 'kvm', '-accel', 'tcg') + #vm.add_args('-accel', 'qtest') + vm.launch() + + iotests.log('Exporting to NBD...') + iotests.log(vm.qmp('nbd-server-start', + addr={'type': 'unix', 'data': {'path': nbd_sock}})) + iotests.log(vm.qmp('block-export-add', type='nbd', id='exp0', + node_name='disk', writable=True)) + + iotests.log('Connecting qemu-img...') + qemu_io = iotests.QemuIoInteractive('-f', 'raw', + f'nbd+unix:///disk?socket={nbd_sock}') + + iotests.log('Moving the NBD export to a different iothread...') + for i in range(0, 10): + iotests.log(vm.qmp('system_reset')) + time.sleep(0.1) + + iotests.log('Checking that it is still alive...') + iotests.log(vm.qmp('query-status')) + + qemu_io.close() + vm.shutdown() diff --git a/tests/qemu-iotests/tests/iothreads-nbd-export.out b/tests/qemu-iotests/tests/iothreads-nbd-export.out new file mode 100644 index 0000000000..bc514e35e5 --- /dev/null +++ b/tests/qemu-iotests/tests/iothreads-nbd-export.out @@ -0,0 +1,19 @@ +Preparing disk... +Launching VM... +Exporting to NBD... +{"return": {}} +{"return": {}} +Connecting qemu-img... +Moving the NBD export to a different iothread... +{"return": {}} +{"return": {}} +{"return": {}} +{"return": {}} +{"return": {}} +{"return": {}} +{"return": {}} +{"return": {}} +{"return": {}} +{"return": {}} +Checking that it is still alive... +{"return": {"running": true, "status": "running"}} From 7fcb8c89f063122864bd274fd673a70a0a802d93 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 6 Mar 2024 15:28:31 +0100 Subject: [PATCH 04/15] blockdev: Fix blockdev-snapshot-sync error reporting for no medium When external_snapshot_abort() rejects a BlockDriverState without a medium, it creates an error like this: error_setg(errp, "Device '%s' has no medium", device); Trouble is @device can be null. My system formats null as "(null)", but other systems might crash. Reproducer: 1. Create a block device without a medium -> {"execute": "blockdev-add", "arguments": {"driver": "host_cdrom", "node-name": "blk0", "filename": "/dev/sr0"}} <- {"return": {}} 3. Attempt to snapshot it -> {"execute":"blockdev-snapshot-sync", "arguments": { "node-name": "blk0", "snapshot-file":"/tmp/foo.qcow2","format":"qcow2"}} <- {"error": {"class": "GenericError", "desc": "Device '(null)' has no medium"}} Broken when commit 0901f67ecdb made @device optional. Use bdrv_get_device_or_node_name() instead. Now it fails as it should: <- {"error": {"class": "GenericError", "desc": "Device 'blk0' has no medium"}} Fixes: 0901f67ecdb7 ("qmp: Allow to take external snapshots on bs graphs node.") Signed-off-by: Markus Armbruster Message-ID: <20240306142831.2514431-1-armbru@redhat.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- blockdev.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/blockdev.c b/blockdev.c index d8fb3399f5..057601dcf0 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1395,7 +1395,8 @@ static void external_snapshot_action(TransactionAction *action, bdrv_drained_begin(state->old_bs); if (!bdrv_is_inserted(state->old_bs)) { - error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, device); + error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, + bdrv_get_device_or_node_name(state->old_bs)); return; } From 52df1a5b613a06273e92e926a242af69f3f39871 Mon Sep 17 00:00:00 2001 From: Abhiram Tilak Date: Tue, 23 Jan 2024 10:33:55 +0530 Subject: [PATCH 05/15] qemu-img: Fix Column Width and Improve Formatting in snapshot list When running the command `qemu-img snapshot -l SNAPSHOT` the output of VM_CLOCK (measures the offset between host and VM clock) cannot to accommodate values in the order of thousands (4-digit). This line [1] hints on the problem. Additionally, the column width for the VM_CLOCK field was reduced from 15 to 13 spaces in commit b39847a5 in line [2], resulting in a shortage of space. [1]: https://gitlab.com/qemu-project/qemu/-/blob/master/block/qapi.c?ref_type=heads#L753 [2]: https://gitlab.com/qemu-project/qemu/-/blob/master/block/qapi.c?ref_type=heads#L763 This patch restores the column width to 15 spaces and makes adjustments to the affected iotests accordingly. Furthermore, addresses a potential source of confusion by removing whitespace in column headers. Example, VM CLOCK is modified to VM_CLOCK. Additionally a '--' symbol is introduced when ICOUNT returns no output for clarity. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2062 Fixes: b39847a50553 ("migration: introduce icount field for snapshots") Signed-off-by: Abhiram Tilak Message-ID: <20240123050354.22152-2-atp.exp@gmail.com> [kwolf: Fixed up qemu-iotests 261 and 286] Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block/qapi.c | 10 ++-- tests/qemu-iotests/176.out | 16 +++---- tests/qemu-iotests/261 | 4 +- tests/qemu-iotests/267.out | 48 +++++++++---------- tests/qemu-iotests/286 | 3 +- tests/qemu-iotests/286.out | 2 +- .../tests/qcow2-internal-snapshots.out | 14 +++--- 7 files changed, 50 insertions(+), 47 deletions(-) diff --git a/block/qapi.c b/block/qapi.c index 31183d4933..2b5793f1d9 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -742,15 +742,15 @@ void bdrv_snapshot_dump(QEMUSnapshotInfo *sn) char *sizing = NULL; if (!sn) { - qemu_printf("%-10s%-17s%8s%20s%13s%11s", - "ID", "TAG", "VM SIZE", "DATE", "VM CLOCK", "ICOUNT"); + qemu_printf("%-7s %-16s %8s %19s %15s %10s", + "ID", "TAG", "VM_SIZE", "DATE", "VM_CLOCK", "ICOUNT"); } else { g_autoptr(GDateTime) date = g_date_time_new_from_unix_local(sn->date_sec); g_autofree char *date_buf = g_date_time_format(date, "%Y-%m-%d %H:%M:%S"); secs = sn->vm_clock_nsec / 1000000000; snprintf(clock_buf, sizeof(clock_buf), - "%02d:%02d:%02d.%03d", + "%04d:%02d:%02d.%03d", (int)(secs / 3600), (int)((secs / 60) % 60), (int)(secs % 60), @@ -759,8 +759,10 @@ void bdrv_snapshot_dump(QEMUSnapshotInfo *sn) if (sn->icount != -1ULL) { snprintf(icount_buf, sizeof(icount_buf), "%"PRId64, sn->icount); + } else { + snprintf(icount_buf, sizeof(icount_buf), "--"); } - qemu_printf("%-9s %-16s %8s%20s%13s%11s", + qemu_printf("%-7s %-16s %8s %19s %15s %10s", sn->id_str, sn->name, sizing, date_buf, diff --git a/tests/qemu-iotests/176.out b/tests/qemu-iotests/176.out index 45e9153ef3..9c73ef2eea 100644 --- a/tests/qemu-iotests/176.out +++ b/tests/qemu-iotests/176.out @@ -37,8 +37,8 @@ Offset Length File 0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd 0x83400000 0x200 TEST_DIR/t.IMGFMT.itmd Snapshot list: -ID TAG -1 snap +ID TAG +1 snap === Test pass snapshot.1 === @@ -78,8 +78,8 @@ Offset Length File 0x7fff0000 0x10000 TEST_DIR/t.IMGFMT 0x83400000 0x200 TEST_DIR/t.IMGFMT Snapshot list: -ID TAG -1 snap +ID TAG +1 snap === Test pass snapshot.2 === @@ -119,8 +119,8 @@ Offset Length File 0x7fff0000 0x10000 TEST_DIR/t.IMGFMT 0x83400000 0x200 TEST_DIR/t.IMGFMT Snapshot list: -ID TAG -1 snap +ID TAG +1 snap === Test pass snapshot.3 === @@ -157,8 +157,8 @@ Offset Length File 0x7fff0000 0x10000 TEST_DIR/t.IMGFMT 0x83400000 0x200 TEST_DIR/t.IMGFMT Snapshot list: -ID TAG -1 snap +ID TAG +1 snap === Test pass bitmap.0 === diff --git a/tests/qemu-iotests/261 b/tests/qemu-iotests/261 index b73da565da..22b969d310 100755 --- a/tests/qemu-iotests/261 +++ b/tests/qemu-iotests/261 @@ -393,7 +393,7 @@ _check_test_img -r all echo echo "$((sn_count - 1)) snapshots should remain:" -echo " qemu-img info reports $(_img_info | grep -c '^ \{32\}') snapshots" +echo " qemu-img info reports $(_img_info | grep -c '^ \{30\}') snapshots" echo " Image header reports $(peek_file_be "$TEST_IMG" 60 4) snapshots" echo @@ -520,7 +520,7 @@ _check_test_img -r all echo echo '65536 snapshots should remain:' -echo " qemu-img info reports $(_img_info | grep -c '^ \{32\}') snapshots" +echo " qemu-img info reports $(_img_info | grep -c '^ \{30\}') snapshots" echo " Image header reports $(peek_file_be "$TEST_IMG" 60 4) snapshots" # success, all done diff --git a/tests/qemu-iotests/267.out b/tests/qemu-iotests/267.out index 7176e376e1..f6f5d8715a 100644 --- a/tests/qemu-iotests/267.out +++ b/tests/qemu-iotests/267.out @@ -33,8 +33,8 @@ QEMU X.Y.Z monitor - type 'help' for more information (qemu) savevm snap0 (qemu) info snapshots List of snapshots present on all disks: -ID TAG VM SIZE DATE VM CLOCK ICOUNT --- snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000 +ID TAG VM_SIZE DATE VM_CLOCK ICOUNT +-- snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 -- (qemu) loadvm snap0 (qemu) quit @@ -44,8 +44,8 @@ QEMU X.Y.Z monitor - type 'help' for more information (qemu) savevm snap0 (qemu) info snapshots List of snapshots present on all disks: -ID TAG VM SIZE DATE VM CLOCK ICOUNT --- snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000 +ID TAG VM_SIZE DATE VM_CLOCK ICOUNT +-- snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 -- (qemu) loadvm snap0 (qemu) quit @@ -69,8 +69,8 @@ QEMU X.Y.Z monitor - type 'help' for more information (qemu) savevm snap0 (qemu) info snapshots List of snapshots present on all disks: -ID TAG VM SIZE DATE VM CLOCK ICOUNT --- snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000 +ID TAG VM_SIZE DATE VM_CLOCK ICOUNT +-- snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 -- (qemu) loadvm snap0 (qemu) quit @@ -94,8 +94,8 @@ QEMU X.Y.Z monitor - type 'help' for more information (qemu) savevm snap0 (qemu) info snapshots List of snapshots present on all disks: -ID TAG VM SIZE DATE VM CLOCK ICOUNT --- snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000 +ID TAG VM_SIZE DATE VM_CLOCK ICOUNT +-- snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 -- (qemu) loadvm snap0 (qemu) quit @@ -105,8 +105,8 @@ QEMU X.Y.Z monitor - type 'help' for more information (qemu) savevm snap0 (qemu) info snapshots List of snapshots present on all disks: -ID TAG VM SIZE DATE VM CLOCK ICOUNT --- snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000 +ID TAG VM_SIZE DATE VM_CLOCK ICOUNT +-- snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 -- (qemu) loadvm snap0 (qemu) quit @@ -119,8 +119,8 @@ QEMU X.Y.Z monitor - type 'help' for more information (qemu) savevm snap0 (qemu) info snapshots List of snapshots present on all disks: -ID TAG VM SIZE DATE VM CLOCK ICOUNT --- snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000 +ID TAG VM_SIZE DATE VM_CLOCK ICOUNT +-- snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 -- (qemu) loadvm snap0 (qemu) quit @@ -134,8 +134,8 @@ QEMU X.Y.Z monitor - type 'help' for more information (qemu) savevm snap0 (qemu) info snapshots List of snapshots present on all disks: -ID TAG VM SIZE DATE VM CLOCK ICOUNT --- snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000 +ID TAG VM_SIZE DATE VM_CLOCK ICOUNT +-- snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 -- (qemu) loadvm snap0 (qemu) quit @@ -145,15 +145,15 @@ QEMU X.Y.Z monitor - type 'help' for more information (qemu) savevm snap0 (qemu) info snapshots List of snapshots present on all disks: -ID TAG VM SIZE DATE VM CLOCK ICOUNT --- snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000 +ID TAG VM_SIZE DATE VM_CLOCK ICOUNT +-- snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 -- (qemu) loadvm snap0 (qemu) quit Internal snapshots on overlay: Snapshot list: -ID TAG VM SIZE DATE VM CLOCK ICOUNT -1 snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000 +ID TAG VM_SIZE DATE VM_CLOCK ICOUNT +1 snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 -- Internal snapshots on backing file: === -blockdev with NBD server on the backing file === @@ -166,17 +166,17 @@ QEMU X.Y.Z monitor - type 'help' for more information (qemu) savevm snap0 (qemu) info snapshots List of snapshots present on all disks: -ID TAG VM SIZE DATE VM CLOCK ICOUNT --- snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000 +ID TAG VM_SIZE DATE VM_CLOCK ICOUNT +-- snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 -- (qemu) loadvm snap0 (qemu) quit Internal snapshots on overlay: Snapshot list: -ID TAG VM SIZE DATE VM CLOCK ICOUNT -1 snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000 +ID TAG VM_SIZE DATE VM_CLOCK ICOUNT +1 snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 -- Internal snapshots on backing file: Snapshot list: -ID TAG VM SIZE DATE VM CLOCK ICOUNT -1 snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000 +ID TAG VM_SIZE DATE VM_CLOCK ICOUNT +1 snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 -- *** done diff --git a/tests/qemu-iotests/286 b/tests/qemu-iotests/286 index 120a8375b7..38216c2a0e 100755 --- a/tests/qemu-iotests/286 +++ b/tests/qemu-iotests/286 @@ -69,7 +69,8 @@ $QEMU_IMG snapshot -l "$TEST_IMG" | tail -n 1 | tr -s ' ' \ -e 's/\./(VM state size unit)/' \ -e 's/\./(snapshot date)/' \ -e 's/\./(snapshot time)/' \ - -e 's/\./(VM clock)/' + -e 's/\./(VM clock)/' \ + -e 's/\./(icount)/' # success, all done echo "*** done" diff --git a/tests/qemu-iotests/286.out b/tests/qemu-iotests/286.out index 39ff07e12c..bb04748e08 100644 --- a/tests/qemu-iotests/286.out +++ b/tests/qemu-iotests/286.out @@ -4,5 +4,5 @@ QEMU X.Y.Z monitor - type 'help' for more information (qemu) savevm abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz (qemu) quit Output structure: -(snapshot ID) (snapshot name) (VM state size value) (VM state size unit) (snapshot date) (snapshot time) (VM clock) +(snapshot ID) (snapshot name) (VM state size value) (VM state size unit) (snapshot date) (snapshot time) (VM clock) (icount) *** done diff --git a/tests/qemu-iotests/tests/qcow2-internal-snapshots.out b/tests/qemu-iotests/tests/qcow2-internal-snapshots.out index 438f535e6a..fedb09224e 100644 --- a/tests/qemu-iotests/tests/qcow2-internal-snapshots.out +++ b/tests/qemu-iotests/tests/qcow2-internal-snapshots.out @@ -14,8 +14,8 @@ wrote 524288/524288 bytes at offset 0 (qemu) quit Snapshot list: -ID TAG VM SIZE DATE VM CLOCK ICOUNT -1 snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000 +ID TAG VM_SIZE DATE VM_CLOCK ICOUNT +1 snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 -- No errors were found on the image. === Verify that loading the snapshot reverts to the old content === @@ -47,9 +47,9 @@ read 64512/64512 bytes at offset 66560 (qemu) quit Snapshot list: -ID TAG VM SIZE DATE VM CLOCK ICOUNT -1 snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000 -2 snap1 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000 +ID TAG VM_SIZE DATE VM_CLOCK ICOUNT +1 snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 -- +2 snap1 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 -- No errors were found on the image. === qemu-img snapshot can revert to snapshots === @@ -79,8 +79,8 @@ read 64512/64512 bytes at offset 66560 (qemu) quit Snapshot list: -ID TAG VM SIZE DATE VM CLOCK ICOUNT -1 snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000 +ID TAG VM_SIZE DATE VM_CLOCK ICOUNT +1 snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 -- No errors were found on the image. === Error cases === From 7987a3138acbc899c90c15ebf788753ae06713be Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 15 Mar 2024 12:11:00 +0100 Subject: [PATCH 06/15] tests/qemu-iotests: Fix test 033 for running with non-file protocols When running iotest 033 with the ssh protocol, it fails with: 033 fail [14:48:31] [14:48:41] 10.2s output mismatch --- /.../tests/qemu-iotests/033.out +++ /.../tests/qemu-iotests/scratch/qcow2-ssh-033/033.out.bad @@ -174,6 +174,7 @@ 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) wrote 512/512 bytes at offset 2097152 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +qemu-io: warning: Failed to truncate the tail of the image: ssh driver does not support shrinking files read 512/512 bytes at offset 0 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) We already check for the qcow2 format here, so let's simply also add a check for the protocol here, too, to only test the truncation with the file protocol. Signed-off-by: Thomas Huth Message-ID: <20240315111108.153201-2-thuth@redhat.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- tests/qemu-iotests/033 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/qemu-iotests/033 b/tests/qemu-iotests/033 index da9133c44b..4bc7a071bd 100755 --- a/tests/qemu-iotests/033 +++ b/tests/qemu-iotests/033 @@ -123,9 +123,9 @@ do_test 512 "write -P 1 0 0x200" "$TEST_IMG" | _filter_qemu_io # next L2 table do_test 512 "write -P 1 $L2_COVERAGE 0x200" "$TEST_IMG" | _filter_qemu_io -# only interested in qcow2 here; also other formats might respond with -# "not supported" error message -if [ $IMGFMT = "qcow2" ]; then +# only interested in qcow2 with file protocol here; also other formats +# might respond with "not supported" error message +if [ $IMGFMT = "qcow2" ] && [ $IMGPROTO = "file" ]; then do_test 512 "truncate $L2_COVERAGE" "$TEST_IMG" | _filter_qemu_io fi From 1a74b0151707697470ecacbdbb1c47880b7cf3b1 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 15 Mar 2024 12:11:01 +0100 Subject: [PATCH 07/15] tests/qemu-iotests: Restrict test 066 to the 'file' protocol The hand-crafted json statement in this test only works if the test is run with the "file" protocol, so mark this test accordingly. Signed-off-by: Thomas Huth Message-ID: <20240315111108.153201-3-thuth@redhat.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- tests/qemu-iotests/066 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemu-iotests/066 b/tests/qemu-iotests/066 index cf63144cb9..336d8565dd 100755 --- a/tests/qemu-iotests/066 +++ b/tests/qemu-iotests/066 @@ -39,7 +39,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 # This tests qcow2-specific low-level functionality _supported_fmt qcow2 -_supported_proto generic +_supported_proto file # We need zero clusters and snapshots # (TODO: Consider splitting the snapshot part into a separate test # file, so this one runs with refcount_bits=1 and data_file) From 70877f21e746e2420727628661a506ce95da4b32 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 15 Mar 2024 12:11:02 +0100 Subject: [PATCH 08/15] tests/qemu-iotests: Restrict test 114 to the 'file' protocol iotest 114 uses "truncate" and the qcow2.py script on the destination file, which both cannot deal with URIs. Thus this test needs the "file" protocol, otherwise it fails with an error message like this: truncate: cannot open 'ssh://127.0.0.1/tmp/qemu-build/tests/qemu-iotests/scratch/qcow2-ssh-114/t.qcow2.orig' for writing: No such file or directory Thus mark this test for "file protocol only" accordingly. Signed-off-by: Thomas Huth Message-ID: <20240315111108.153201-4-thuth@redhat.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- tests/qemu-iotests/114 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemu-iotests/114 b/tests/qemu-iotests/114 index de6fd327ee..dccc71008b 100755 --- a/tests/qemu-iotests/114 +++ b/tests/qemu-iotests/114 @@ -38,7 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 . ./common.filter _supported_fmt qcow2 -_supported_proto generic +_supported_proto file # At least OpenBSD doesn't seem to have truncate _supported_os Linux # qcow2.py does not work too well with external data files From 0988928e92959cf69d441c5bb01d0c22c101f113 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 15 Mar 2024 12:11:03 +0100 Subject: [PATCH 09/15] tests/qemu-iotests: Restrict test 130 to the 'file' protocol Using "-drive ...,backing.file.filename=..." only works with the file protocol, but not with URIs, so mark this test accordingly. Signed-off-by: Thomas Huth Message-ID: <20240315111108.153201-5-thuth@redhat.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- tests/qemu-iotests/130 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemu-iotests/130 b/tests/qemu-iotests/130 index 7257f09677..7af85d20a8 100755 --- a/tests/qemu-iotests/130 +++ b/tests/qemu-iotests/130 @@ -42,7 +42,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 . ./common.qemu _supported_fmt qcow2 -_supported_proto generic +_supported_proto file _supported_os Linux # We are going to use lazy-refcounts _unsupported_imgopts 'compat=0.10' From 9e39544465dce44467cc33114d4e1d43316f559d Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 15 Mar 2024 12:11:04 +0100 Subject: [PATCH 10/15] tests/qemu-iotests: Restrict test 134 and 158 to the 'file' protocol Commit b25b387fa592 updated the iotests 134 and 158 to use the --image-opts parameter for qemu-io with file protocol related options, but forgot to update the _supported_proto line accordingly. So let's do that now. Fixes: b25b387fa5 ("qcow2: convert QCow2 to use QCryptoBlock for encryption") Signed-off-by: Thomas Huth Message-ID: <20240315111108.153201-6-thuth@redhat.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- tests/qemu-iotests/134 | 2 +- tests/qemu-iotests/158 | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/qemu-iotests/134 b/tests/qemu-iotests/134 index ded153c0b9..b2c3c03f08 100755 --- a/tests/qemu-iotests/134 +++ b/tests/qemu-iotests/134 @@ -38,7 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 . ./common.filter _supported_fmt qcow qcow2 -_supported_proto generic +_supported_proto file size=128M diff --git a/tests/qemu-iotests/158 b/tests/qemu-iotests/158 index a95878e4ce..3a9ad7eed0 100755 --- a/tests/qemu-iotests/158 +++ b/tests/qemu-iotests/158 @@ -38,7 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 . ./common.filter _supported_fmt qcow qcow2 -_supported_proto generic +_supported_proto file size=128M From e7a271bee991ed3db338e1575ba512a1bbe3ef50 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 15 Mar 2024 12:11:05 +0100 Subject: [PATCH 11/15] tests/qemu-iotests: Restrict test 156 to the 'file' protocol The test fails completely when you try to use it with a different protocol, e.g. with "./check -ssh -qcow2 156". The test uses some hand-crafted JSON statements which cannot work with other protocols, thus let's change this test to only support the 'file' protocol. Signed-off-by: Thomas Huth Message-ID: <20240315111108.153201-7-thuth@redhat.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- tests/qemu-iotests/156 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemu-iotests/156 b/tests/qemu-iotests/156 index a9540bd80d..97c2d86ce5 100755 --- a/tests/qemu-iotests/156 +++ b/tests/qemu-iotests/156 @@ -50,7 +50,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 . ./common.qemu _supported_fmt qcow2 qed -_supported_proto generic +_supported_proto file # Copying files around with cp does not work with external data files _unsupported_imgopts data_file From 9677061ef1b7c6bef321feb4dc82eb7e5e024bcd Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 15 Mar 2024 12:11:06 +0100 Subject: [PATCH 12/15] tests/qemu-iotests: Restrict tests that use --image-opts to the 'file' protocol These tests 188, 189 and 198 use qemu-io with --image-opts with additional hard-coded parameters for the file protocol, so they cannot work for other protocols. Thus we have to limit these tests to the file protocol only. Signed-off-by: Thomas Huth Message-ID: <20240315111108.153201-8-thuth@redhat.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- tests/qemu-iotests/188 | 2 +- tests/qemu-iotests/189 | 2 +- tests/qemu-iotests/198 | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/qemu-iotests/188 b/tests/qemu-iotests/188 index ce087d1873..2950b1dc31 100755 --- a/tests/qemu-iotests/188 +++ b/tests/qemu-iotests/188 @@ -38,7 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 . ./common.filter _supported_fmt qcow2 -_supported_proto generic +_supported_proto file _supported_os Linux _require_working_luks diff --git a/tests/qemu-iotests/189 b/tests/qemu-iotests/189 index 801494c6b9..008f73b07d 100755 --- a/tests/qemu-iotests/189 +++ b/tests/qemu-iotests/189 @@ -38,7 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 . ./common.filter _supported_fmt qcow2 -_supported_proto generic +_supported_proto file _supported_os Linux _require_working_luks diff --git a/tests/qemu-iotests/198 b/tests/qemu-iotests/198 index 1c93dea1f7..6ddeffddd2 100755 --- a/tests/qemu-iotests/198 +++ b/tests/qemu-iotests/198 @@ -38,7 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 . ./common.filter _supported_fmt qcow2 -_supported_proto generic +_supported_proto file _supported_os Linux _require_working_luks From cff614087dcfa239259dc94dc5101f55bef6f117 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 15 Mar 2024 12:11:07 +0100 Subject: [PATCH 13/15] tests/qemu-iotests: Fix some tests that use --image-opts for other protocols Tests 263, 284 and detect-zeroes-registered-buf use qemu-io with --image-opts so we have to enforce IMGOPTSSYNTAX=true here to get $TEST_IMG in shape for other protocols than "file". Signed-off-by: Thomas Huth Message-ID: <20240315111108.153201-9-thuth@redhat.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- tests/qemu-iotests/263 | 6 ++++-- tests/qemu-iotests/284 | 7 +++---- tests/qemu-iotests/tests/detect-zeroes-registered-buf | 4 +++- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/tests/qemu-iotests/263 b/tests/qemu-iotests/263 index ec09b41405..44fdada0d6 100755 --- a/tests/qemu-iotests/263 +++ b/tests/qemu-iotests/263 @@ -34,6 +34,8 @@ _cleanup() } trap "_cleanup; exit \$status" 0 1 2 3 15 +IMGOPTSSYNTAX=true + # get standard environment, filters and checks . ./common.rc . ./common.filter @@ -73,7 +75,7 @@ echo "testing LUKS qcow2 encryption" echo _make_test_img --object $SECRET -o "encrypt.format=luks,encrypt.key-secret=sec0,encrypt.iter-time=10,cluster_size=64K" $size -_run_test "driver=$IMGFMT,encrypt.key-secret=sec0,file.filename=$TEST_IMG" +_run_test "$TEST_IMG,encrypt.key-secret=sec0" _cleanup_test_img echo @@ -82,7 +84,7 @@ echo _make_test_img --object $SECRET -o "encrypt.format=aes,encrypt.key-secret=sec0,cluster_size=64K" $size -_run_test "driver=$IMGFMT,encrypt.key-secret=sec0,file.filename=$TEST_IMG" +_run_test "$TEST_IMG,encrypt.key-secret=sec0" _cleanup_test_img diff --git a/tests/qemu-iotests/284 b/tests/qemu-iotests/284 index 5a82639e7f..722267486d 100755 --- a/tests/qemu-iotests/284 +++ b/tests/qemu-iotests/284 @@ -33,6 +33,8 @@ _cleanup() } trap "_cleanup; exit \$status" 0 1 2 3 15 +IMGOPTSSYNTAX=true + # get standard environment, filters and checks . ./common.rc . ./common.filter @@ -47,14 +49,12 @@ size=1M SECRET="secret,id=sec0,data=astrochicken" -IMGSPEC="driver=$IMGFMT,file.filename=$TEST_IMG,encrypt.key-secret=sec0" QEMU_IO_OPTIONS=$QEMU_IO_OPTIONS_NO_FMT _run_test() { - IMGOPTSSYNTAX=true OLD_TEST_IMG="$TEST_IMG" - TEST_IMG="driver=$IMGFMT,file.filename=$TEST_IMG,encrypt.key-secret=sec0" + TEST_IMG="$TEST_IMG,encrypt.key-secret=sec0" QEMU_IMG_EXTRA_ARGS="--image-opts --object $SECRET" echo @@ -78,7 +78,6 @@ _run_test() TEST_IMG="$OLD_TEST_IMG" QEMU_IMG_EXTRA_ARGS= - IMGOPTSSYNTAX= } diff --git a/tests/qemu-iotests/tests/detect-zeroes-registered-buf b/tests/qemu-iotests/tests/detect-zeroes-registered-buf index edb5f2cee5..5eaf34e5a6 100755 --- a/tests/qemu-iotests/tests/detect-zeroes-registered-buf +++ b/tests/qemu-iotests/tests/detect-zeroes-registered-buf @@ -36,6 +36,8 @@ _cleanup() } trap "_cleanup; exit \$status" 0 1 2 3 15 +IMGOPTSSYNTAX=true + # get standard environment, filters and checks cd .. . ./common.rc @@ -46,7 +48,7 @@ _supported_proto generic size=128M _make_test_img $size -IMGSPEC="driver=$IMGFMT,file.filename=$TEST_IMG,discard=unmap,detect-zeroes=unmap" +IMGSPEC="$TEST_IMG,discard=unmap,detect-zeroes=unmap" echo echo "== writing zero buffer to image ==" From a9fdde400aed53248d6d1dea1d6c9dc6102ee4ac Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 15 Mar 2024 12:11:08 +0100 Subject: [PATCH 14/15] tests/qemu-iotests: Restrict tests using "--blockdev file" to the file protocol Tests that use "--blockdev" with the "file" driver cannot work with other protocols, so we should mark them accordingly. Signed-off-by: Thomas Huth Message-ID: <20240315111108.153201-10-thuth@redhat.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- tests/qemu-iotests/tests/qcow2-internal-snapshots | 2 +- tests/qemu-iotests/tests/qsd-jobs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/qemu-iotests/tests/qcow2-internal-snapshots b/tests/qemu-iotests/tests/qcow2-internal-snapshots index 36523aba06..9f83aa8903 100755 --- a/tests/qemu-iotests/tests/qcow2-internal-snapshots +++ b/tests/qemu-iotests/tests/qcow2-internal-snapshots @@ -39,7 +39,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 # This tests qcow2-specific low-level functionality _supported_fmt qcow2 -_supported_proto generic +_supported_proto file # Internal snapshots are (currently) impossible with refcount_bits=1, # and generally impossible with external data files _unsupported_imgopts 'compat=0.10' 'refcount_bits=1[^0-9]' data_file diff --git a/tests/qemu-iotests/tests/qsd-jobs b/tests/qemu-iotests/tests/qsd-jobs index 510bf0a9dc..9b843af631 100755 --- a/tests/qemu-iotests/tests/qsd-jobs +++ b/tests/qemu-iotests/tests/qsd-jobs @@ -40,7 +40,7 @@ cd .. . ./common.filter _supported_fmt qcow2 -_supported_proto generic +_supported_proto file size=128M From 39a94d7c34ce9d222fa9c0c99a14e20a567456d7 Mon Sep 17 00:00:00 2001 From: Fiona Ebner Date: Fri, 16 Feb 2024 11:14:15 +0100 Subject: [PATCH 15/15] iotests: adapt to output change for recently introduced 'detached header' field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Failure was noticed when running the tests for the qcow2 image format. Fixes: 0bd779e27e ("crypto: Introduce 'detached-header' field in QCryptoBlockInfoLUKS") Signed-off-by: Fiona Ebner Message-ID: <20240216101415.293769-1-f.ebner@proxmox.com> Reviewed-by: Daniel P. Berrangé Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- tests/qemu-iotests/198.out | 2 ++ tests/qemu-iotests/206.out | 1 + 2 files changed, 3 insertions(+) diff --git a/tests/qemu-iotests/198.out b/tests/qemu-iotests/198.out index 805494916f..62fb73fa3e 100644 --- a/tests/qemu-iotests/198.out +++ b/tests/qemu-iotests/198.out @@ -39,6 +39,7 @@ Format specific information: compression type: COMPRESSION_TYPE encrypt: ivgen alg: plain64 + detached header: false hash alg: sha256 cipher alg: aes-256 uuid: 00000000-0000-0000-0000-000000000000 @@ -84,6 +85,7 @@ Format specific information: compression type: COMPRESSION_TYPE encrypt: ivgen alg: plain64 + detached header: false hash alg: sha256 cipher alg: aes-256 uuid: 00000000-0000-0000-0000-000000000000 diff --git a/tests/qemu-iotests/206.out b/tests/qemu-iotests/206.out index 7e95694777..979f00f9bf 100644 --- a/tests/qemu-iotests/206.out +++ b/tests/qemu-iotests/206.out @@ -114,6 +114,7 @@ Format specific information: refcount bits: 16 encrypt: ivgen alg: plain64 + detached header: false hash alg: sha1 cipher alg: aes-128 uuid: XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX