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 <kwolf@redhat.com>

Message-ID: <20240313153000.33121-1-kwolf@redhat.com>

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

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

---

include/qemu/job.h | 2 +-

block/mirror.c | 10 ++++++----

2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/include/qemu/job.h b/include/qemu/job.h

index XXXXXXX..XXXXXXX 100644

--- a/include/qemu/job.h

+++ b/include/qemu/job.h

@@ -XXX,XX +XXX,XX @@ 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.

diff --git a/block/mirror.c b/block/mirror.c

index XXXXXXX..XXXXXXX 100644

--- a/block/mirror.c

+++ b/block/mirror.c

@@ -XXX,XX +XXX,XX @@ 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. */

@@ -XXX,XX +XXX,XX @@ 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) {

@@ -XXX,XX +XXX,XX @@ 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();

}

}

--

2.44.0

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 <kwolf@redhat.com>

Message-ID: <20240314165825.40261-2-kwolf@redhat.com>

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

---

nbd/server.c | 15 +++++++--------

1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/nbd/server.c b/nbd/server.c

index XXXXXXX..XXXXXXX 100644

--- a/nbd/server.c

+++ b/nbd/server.c

@@ -XXX,XX +XXX,XX @@ 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;

@@ -XXX,XX +XXX,XX @@ 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

@@ -XXX,XX +XXX,XX @@ static coroutine_fn void nbd_trip(void *opaque)

}

done:

- if (req) {

- nbd_request_put(req);

- }

+ nbd_request_put(req);

qemu_mutex_unlock(&client->lock);

@@ -XXX,XX +XXX,XX @@ 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);

}

}

--

2.44.0

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 <kwolf@redhat.com>

Message-ID: <20240314165825.40261-3-kwolf@redhat.com>

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

---

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 XXXXXXX..XXXXXXX

--- /dev/null

+++ b/tests/qemu-iotests/tests/iothreads-nbd-export

@@ -XXX,XX +XXX,XX @@

+#!/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 <http://www.gnu.org/licenses/>.

+#

+# Creator/Owner: Kevin Wolf <kwolf@redhat.com>

+

+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 XXXXXXX..XXXXXXX

--- /dev/null

+++ b/tests/qemu-iotests/tests/iothreads-nbd-export.out

@@ -XXX,XX +XXX,XX @@

+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"}}

--

2.44.0

From: Markus Armbruster <armbru@redhat.com>

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 <armbru@redhat.com>

Message-ID: <20240306142831.2514431-1-armbru@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

---

blockdev.c | 3 ++-

1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/blockdev.c b/blockdev.c

index XXXXXXX..XXXXXXX 100644

--- a/blockdev.c

+++ b/blockdev.c

@@ -XXX,XX +XXX,XX @@ 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;

}

--

2.44.0

From: Abhiram Tilak <atp.exp@gmail.com>

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 <atp.exp@gmail.com>

Message-ID: <20240123050354.22152-2-atp.exp@gmail.com>

[kwolf: Fixed up qemu-iotests 261 and 286]

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

---

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 XXXXXXX..XXXXXXX 100644

--- a/block/qapi.c

+++ b/block/qapi.c

@@ -XXX,XX +XXX,XX @@ 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),

@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/176.out

+++ b/tests/qemu-iotests/176.out

@@ -XXX,XX +XXX,XX @@ 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 ===

@@ -XXX,XX +XXX,XX @@ 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 ===

@@ -XXX,XX +XXX,XX @@ 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 ===

@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/261

+++ b/tests/qemu-iotests/261

@@ -XXX,XX +XXX,XX @@ _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

@@ -XXX,XX +XXX,XX @@ _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 XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/267.out

+++ b/tests/qemu-iotests/267.out

@@ -XXX,XX +XXX,XX @@ 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

@@ -XXX,XX +XXX,XX @@ 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

@@ -XXX,XX +XXX,XX @@ 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

@@ -XXX,XX +XXX,XX @@ 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

@@ -XXX,XX +XXX,XX @@ 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

@@ -XXX,XX +XXX,XX @@ 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

@@ -XXX,XX +XXX,XX @@ 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

@@ -XXX,XX +XXX,XX @@ 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 ===

@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/286

+++ b/tests/qemu-iotests/286

@@ -XXX,XX +XXX,XX @@ $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 XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/286.out

+++ b/tests/qemu-iotests/286.out

@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/tests/qcow2-internal-snapshots.out

+++ b/tests/qemu-iotests/tests/qcow2-internal-snapshots.out

@@ -XXX,XX +XXX,XX @@ 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 ===

@@ -XXX,XX +XXX,XX @@ 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 ===

@@ -XXX,XX +XXX,XX @@ 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 ===

--

2.44.0

From: Thomas Huth <thuth@redhat.com>

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 <thuth@redhat.com>

Message-ID: <20240315111108.153201-2-thuth@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

---

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 XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/033

+++ b/tests/qemu-iotests/033

@@ -XXX,XX +XXX,XX @@ 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

--

2.44.0

From: Thomas Huth <thuth@redhat.com>

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 <thuth@redhat.com>

Message-ID: <20240315111108.153201-3-thuth@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

---

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 XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/066

+++ b/tests/qemu-iotests/066

@@ -XXX,XX +XXX,XX @@ 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)

--

2.44.0

From: Thomas Huth <thuth@redhat.com>

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 <thuth@redhat.com>

Message-ID: <20240315111108.153201-4-thuth@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

---

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 XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/114

+++ b/tests/qemu-iotests/114

@@ -XXX,XX +XXX,XX @@ 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

--

2.44.0

From: Thomas Huth <thuth@redhat.com>

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 <thuth@redhat.com>

Message-ID: <20240315111108.153201-5-thuth@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

---

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 XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/130

+++ b/tests/qemu-iotests/130

@@ -XXX,XX +XXX,XX @@ 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'

--

2.44.0

From: Thomas Huth <thuth@redhat.com>

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 <thuth@redhat.com>

Message-ID: <20240315111108.153201-6-thuth@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

---

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 XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/134

+++ b/tests/qemu-iotests/134

@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/158

+++ b/tests/qemu-iotests/158

@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15

. ./common.filter

_supported_fmt qcow qcow2

-_supported_proto generic

+_supported_proto file

size=128M

--

2.44.0

From: Thomas Huth <thuth@redhat.com>

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 <thuth@redhat.com>

Message-ID: <20240315111108.153201-7-thuth@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

---

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 XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/156

+++ b/tests/qemu-iotests/156

@@ -XXX,XX +XXX,XX @@ 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

--

2.44.0

From: Thomas Huth <thuth@redhat.com>

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 <thuth@redhat.com>

Message-ID: <20240315111108.153201-8-thuth@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

---

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 XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/188

+++ b/tests/qemu-iotests/188

@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/189

+++ b/tests/qemu-iotests/189

@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/198

+++ b/tests/qemu-iotests/198

@@ -XXX,XX +XXX,XX @@ 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

--

2.44.0

From: Thomas Huth <thuth@redhat.com>

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 <thuth@redhat.com>

Message-ID: <20240315111108.153201-9-thuth@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

---

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 XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/263

+++ b/tests/qemu-iotests/263

@@ -XXX,XX +XXX,XX @@ _cleanup()

}

trap "_cleanup; exit \$status" 0 1 2 3 15

+IMGOPTSSYNTAX=true

+

# get standard environment, filters and checks

. ./common.rc

. ./common.filter

@@ -XXX,XX +XXX,XX @@ 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

@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/284

+++ b/tests/qemu-iotests/284

@@ -XXX,XX +XXX,XX @@ _cleanup()

}

trap "_cleanup; exit \$status" 0 1 2 3 15

+IMGOPTSSYNTAX=true

+

# get standard environment, filters and checks

. ./common.rc

. ./common.filter

@@ -XXX,XX +XXX,XX @@ 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

@@ -XXX,XX +XXX,XX @@ _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 XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/tests/detect-zeroes-registered-buf

+++ b/tests/qemu-iotests/tests/detect-zeroes-registered-buf

@@ -XXX,XX +XXX,XX @@ _cleanup()

}

trap "_cleanup; exit \$status" 0 1 2 3 15

+IMGOPTSSYNTAX=true

+

# get standard environment, filters and checks

cd ..

. ./common.rc

@@ -XXX,XX +XXX,XX @@ _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 =="

--

2.44.0

From: Thomas Huth <thuth@redhat.com>

Tests that use "--blockdev" with the "file" driver cannot work with

other protocols, so we should mark them accordingly.

Signed-off-by: Thomas Huth <thuth@redhat.com>

Message-ID: <20240315111108.153201-10-thuth@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

---

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 XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/tests/qcow2-internal-snapshots

+++ b/tests/qemu-iotests/tests/qcow2-internal-snapshots

@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/tests/qsd-jobs

+++ b/tests/qemu-iotests/tests/qsd-jobs

@@ -XXX,XX +XXX,XX @@ cd ..

. ./common.filter

_supported_fmt qcow2

-_supported_proto generic

+_supported_proto file

size=128M

--

2.44.0

From: Fiona Ebner <f.ebner@proxmox.com>

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 <f.ebner@proxmox.com>

Message-ID: <20240216101415.293769-1-f.ebner@proxmox.com>

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

---

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 XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/198.out

+++ b/tests/qemu-iotests/198.out

@@ -XXX,XX +XXX,XX @@ 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

@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/206.out

+++ b/tests/qemu-iotests/206.out

@@ -XXX,XX +XXX,XX @@ 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

--

2.44.0