From cfda94eddb6c9c49b66461c950b22845a46a75c9 Mon Sep 17 00:00:00 2001 From: "Halil Oktay (oblivionsage)" Date: Tue, 10 Feb 2026 13:33:25 +0100 Subject: [PATCH 01/12] block/vmdk: fix OOB read in vmdk_read_extent() Bounds check for marker.size doesn't account for the 12-byte marker header, allowing zlib to read past the allocated buffer. Move the check inside the has_marker block and subtract the marker size. Fixes: CVE-2026-2243 Reported-by: Halil Oktay (oblivionsage) Signed-off-by: Halil Oktay (oblivionsage) Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block/vmdk.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index 89e89cd10e..cd8b4ec7c8 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1951,10 +1951,10 @@ vmdk_read_extent(VmdkExtent *extent, int64_t cluster_offset, marker = (VmdkGrainMarker *)cluster_buf; compressed_data = marker->data; data_len = le32_to_cpu(marker->size); - } - if (!data_len || data_len > buf_bytes) { - ret = -EINVAL; - goto out; + if (!data_len || data_len > buf_bytes - sizeof(VmdkGrainMarker)) { + ret = -EINVAL; + goto out; + } } ret = uncompress(uncomp_buf, &buf_len, compressed_data, data_len); if (ret != Z_OK) { From d57bdcae9a8a90b3e1ffb4d5b5ee2909ce5416c2 Mon Sep 17 00:00:00 2001 From: Peter Krempa Date: Wed, 4 Feb 2026 14:15:43 +0100 Subject: [PATCH 02/12] block: Wire up 'flat' mode also for 'query-block' Some time ago (commit facda5443f5a8) I've added 'flat' mode (which omits 'backing-image' key in reply) to 'query-named-block-nodes' to minimize the size of the returned JSON for deeper backing chains. While 'query-block' behaved slightly better it turns out that in libvirt we do call 'query-block' to figure out some information about the block device (e.g. throttling info) but we don't look at the backing chain itself. Wire up 'flat' for 'query-block' so that libvirt can ask for an abbreviated output. The implementation is much simpler as the internals are shared with 'query-named-block-nodes'. Signed-off-by: Peter Krempa Acked-by: Markus Armbruster Message-ID: Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block/monitor/block-hmp-cmds.c | 4 ++-- block/qapi.c | 8 ++++---- qapi/block-core.json | 5 +++++ ui/cocoa.m | 2 +- 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c index 3391cee4d2..bde25bb588 100644 --- a/block/monitor/block-hmp-cmds.c +++ b/block/monitor/block-hmp-cmds.c @@ -422,7 +422,7 @@ void hmp_nbd_server_start(Monitor *mon, const QDict *qdict) /* Then try adding all block devices. If one fails, close all and * exit. */ - block_list = qmp_query_block(NULL); + block_list = qmp_query_block(false, false, NULL); for (info = block_list; info; info = info->next) { if (!info->value->inserted) { @@ -741,7 +741,7 @@ void hmp_info_block(Monitor *mon, const QDict *qdict) /* Print BlockBackend information */ if (!nodes) { - block_list = qmp_query_block(NULL); + block_list = qmp_query_block(false, false, NULL); } else { block_list = NULL; } diff --git a/block/qapi.c b/block/qapi.c index 27e0ac6a32..eabfbfc258 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -456,7 +456,7 @@ fail: /* @p_info will be set only on success. */ static void GRAPH_RDLOCK -bdrv_query_info(BlockBackend *blk, BlockInfo **p_info, Error **errp) +bdrv_query_info(BlockBackend *blk, bool flat, BlockInfo **p_info, Error **errp) { BlockInfo *info = g_malloc0(sizeof(*info)); BlockDriverState *bs = blk_bs(blk); @@ -488,7 +488,7 @@ bdrv_query_info(BlockBackend *blk, BlockInfo **p_info, Error **errp) } if (bs && bs->drv) { - info->inserted = bdrv_block_device_info(blk, bs, false, errp); + info->inserted = bdrv_block_device_info(blk, bs, flat, errp); if (info->inserted == NULL) { goto err; } @@ -698,7 +698,7 @@ bdrv_query_bds_stats(BlockDriverState *bs, bool blk_level) return s; } -BlockInfoList *qmp_query_block(Error **errp) +BlockInfoList *qmp_query_block(bool has_flat, bool flat, Error **errp) { BlockInfoList *head = NULL, **p_next = &head; BlockBackend *blk; @@ -714,7 +714,7 @@ BlockInfoList *qmp_query_block(Error **errp) } info = g_malloc0(sizeof(*info)); - bdrv_query_info(blk, &info->value, &local_err); + bdrv_query_info(blk, flat, &info->value, &local_err); if (local_err) { error_propagate(errp, local_err); g_free(info); diff --git a/qapi/block-core.json b/qapi/block-core.json index b82af74256..b66bf316e2 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -855,6 +855,10 @@ # # Get a list of `BlockInfo` for all virtual block devices. # +# @flat: Omit nested data about the backing image, i.e. `BlockInfo` +# member 'inserted.image.backing-image' will be absent. +# Default is false. (Since 11.0) +# # Returns: a list describing each virtual block device. Filter nodes # that were created implicitly are skipped over. # @@ -945,6 +949,7 @@ # } ## { 'command': 'query-block', 'returns': ['BlockInfo'], + 'data': { '*flat': 'bool' }, 'allow-preconfig': true } ## diff --git a/ui/cocoa.m b/ui/cocoa.m index 23b7a736d7..5b21fe3aea 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -1849,7 +1849,7 @@ static void addRemovableDevicesMenuItems(void) BlockInfoList *currentDevice, *pointerToFree; NSString *deviceName; - currentDevice = qmp_query_block(NULL); + currentDevice = qmp_query_block(false, false, NULL); pointerToFree = currentDevice; menu = [[[NSApp mainMenu] itemWithTitle:@"Machine"] submenu]; From b2e9401cc9cec714b545bd1a5d56bc3ef4c4c6e2 Mon Sep 17 00:00:00 2001 From: Peter Krempa Date: Wed, 4 Feb 2026 14:15:44 +0100 Subject: [PATCH 03/12] hmp_nbd_server_start: Don't ask for backing image data 'hmp_nbd_server_start' uses only the device name from the data returned from 'qmp_query_block', thus no backing file information. Use the new options to suppress asking for the unused parts to save on resources. Signed-off-by: Peter Krempa Reviewed-by: Eric Blake Message-ID: Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block/monitor/block-hmp-cmds.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c index bde25bb588..1fd28d59eb 100644 --- a/block/monitor/block-hmp-cmds.c +++ b/block/monitor/block-hmp-cmds.c @@ -422,7 +422,7 @@ void hmp_nbd_server_start(Monitor *mon, const QDict *qdict) /* Then try adding all block devices. If one fails, close all and * exit. */ - block_list = qmp_query_block(false, false, NULL); + block_list = qmp_query_block(true, true, NULL); for (info = block_list; info; info = info->next) { if (!info->value->inserted) { From 6f7b0a23a6ea0cc72ad222ab37936248d99d4256 Mon Sep 17 00:00:00 2001 From: Antoine Damhet Date: Thu, 12 Feb 2026 17:27:24 +0100 Subject: [PATCH 04/12] block/curl: fix concurrent completion handling curl_multi_check_completion would bail upon the first completed transfer even if more completion messages were available thus leaving some in flight IOs stuck. Rework a bit the loop to make the iterations clearer and drop the breaks. The original hang can be somewhat reproduced with the following command: $ qemu-img convert -p -m 16 -O qcow2 -c --image-opts \ 'file.driver=https,file.url=https://scaleway.testdebit.info/10G.iso,file.readahead=1M' \ /tmp/test.qcow2 Fixes: 1f2cead32443 ("curl: Ensure all informationals are checked for completion") Cc: qemu-stable@nongnu.org Signed-off-by: Antoine Damhet Message-ID: <20260212162730.440855-2-adamhet@scaleway.com> Signed-off-by: Kevin Wolf --- block/curl.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/block/curl.c b/block/curl.c index 4e77c93b46..6dccf00256 100644 --- a/block/curl.c +++ b/block/curl.c @@ -324,17 +324,11 @@ curl_find_buf(BDRVCURLState *s, uint64_t start, uint64_t len, CURLAIOCB *acb) static void curl_multi_check_completion(BDRVCURLState *s) { int msgs_in_queue; + CURLMsg *msg; /* Try to find done transfers, so we can free the easy * handle again. */ - for (;;) { - CURLMsg *msg; - msg = curl_multi_info_read(s->multi, &msgs_in_queue); - - /* Quit when there are no more completions */ - if (!msg) - break; - + while ((msg = curl_multi_info_read(s->multi, &msgs_in_queue))) { if (msg->msg == CURLMSG_DONE) { int i; CURLState *state = NULL; @@ -397,7 +391,6 @@ static void curl_multi_check_completion(BDRVCURLState *s) } curl_clean_state(state); - break; } } } From 0f51f9c3420b31bb383e456dd7bf24d3056eeb73 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 19 Feb 2026 21:24:46 +0100 Subject: [PATCH 05/12] mirror: Fix missed dirty bitmap writes during startup Currently, mirror disables the block layer's dirty bitmap before its own replacement is working. This means that during startup, there is a window in which the allocation status of blocks in the source has already been checked, but new writes coming in aren't tracked yet, resulting in a corrupted copy: 1. Dirty bitmap is disabled in mirror_start_job() 2. Some request are started in mirror_top_bs while s->job == NULL 3. mirror_dirty_init() -> bdrv_co_is_allocated_above() runs and because the request hasn't completed yet, the block isn't allocated 4. The request completes, still sees s->job == NULL and skips the bitmap, and nothing else will mark it dirty either One ingredient is that mirror_top_opaque->job is only set after the job is fully initialized. For the rationale, see commit 32125b1460 ("mirror: Fix access of uninitialised fields during start"). Fix this by giving mirror_top_bs access to dirty_bitmap and enabling it to track writes from the beginning. Disabling the block layer's tracking and enabling the mirror_top_bs one happens in a drained section, so there is no danger of races with in-flight requests any more. All of this happens well before the block allocation status is checked, so we can be sure that no writes will be missed. Cc: qemu-stable@nongnu.org Closes: https://gitlab.com/qemu-project/qemu/-/issues/3273 Fixes: 32125b14606a ('mirror: Fix access of uninitialised fields during start') Signed-off-by: Kevin Wolf Message-ID: <20260219202446.312493-1-kwolf@redhat.com> Reviewed-by: Fiona Ebner Tested-by: Jean-Louis Dupond Signed-off-by: Kevin Wolf --- block/mirror.c | 52 +++++++++++++++++++++++++++++++------------------- 1 file changed, 32 insertions(+), 20 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index bc982cb99a..fa1d975eb9 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -99,6 +99,7 @@ typedef struct MirrorBlockJob { typedef struct MirrorBDSOpaque { MirrorBlockJob *job; + BdrvDirtyBitmap *dirty_bitmap; bool stop; bool is_commit; } MirrorBDSOpaque; @@ -1675,9 +1676,11 @@ bdrv_mirror_top_do_write(BlockDriverState *bs, MirrorMethod method, abort(); } - if (!copy_to_target && s->job && s->job->dirty_bitmap) { - qatomic_set(&s->job->actively_synced, false); - bdrv_set_dirty_bitmap(s->job->dirty_bitmap, offset, bytes); + if (!copy_to_target) { + if (s->job) { + qatomic_set(&s->job->actively_synced, false); + } + bdrv_set_dirty_bitmap(s->dirty_bitmap, offset, bytes); } if (ret < 0) { @@ -1904,13 +1907,35 @@ static BlockJob *mirror_start_job( bdrv_drained_begin(bs); ret = bdrv_append(mirror_top_bs, bs, errp); - bdrv_drained_end(bs); - if (ret < 0) { + bdrv_drained_end(bs); + bdrv_unref(mirror_top_bs); + return NULL; + } + + bs_opaque->dirty_bitmap = bdrv_create_dirty_bitmap(mirror_top_bs, + granularity, + NULL, errp); + if (!bs_opaque->dirty_bitmap) { + bdrv_drained_end(bs); bdrv_unref(mirror_top_bs); return NULL; } + /* + * The mirror job doesn't use the block layer's dirty tracking because it + * needs to be able to switch seemlessly between background copy mode (which + * does need dirty tracking) and write blocking mode (which doesn't) and + * doing that would require draining the node. Instead, mirror_top_bs takes + * care of updating the dirty bitmap as appropriate. + * + * Note that write blocking mode only becomes effective after mirror_run() + * sets mirror_top_opaque->job (see should_copy_to_target()). Until then, + * we're still in background copy mode irrespective of @copy_mode. + */ + bdrv_disable_dirty_bitmap(bs_opaque->dirty_bitmap); + bdrv_drained_end(bs); + /* Make sure that the source is not resized while the job is running */ s = block_job_create(job_id, driver, NULL, mirror_top_bs, BLK_PERM_CONSISTENT_READ, @@ -2005,24 +2030,13 @@ static BlockJob *mirror_start_job( s->base_overlay = bdrv_find_overlay(bs, base); s->granularity = granularity; s->buf_size = ROUND_UP(buf_size, granularity); + s->dirty_bitmap = bs_opaque->dirty_bitmap; s->unmap = unmap; if (auto_complete) { s->should_complete = true; } bdrv_graph_rdunlock_main_loop(); - s->dirty_bitmap = bdrv_create_dirty_bitmap(s->mirror_top_bs, granularity, - NULL, errp); - if (!s->dirty_bitmap) { - goto fail; - } - - /* - * The dirty bitmap is set by bdrv_mirror_top_do_write() when not in active - * mode. - */ - bdrv_disable_dirty_bitmap(s->dirty_bitmap); - bdrv_graph_wrlock_drained(); ret = block_job_add_bdrv(&s->common, "source", bs, 0, BLK_PERM_WRITE_UNCHANGED | BLK_PERM_WRITE | @@ -2102,9 +2116,6 @@ fail: g_free(s->replaces); blk_unref(s->target); bs_opaque->job = NULL; - if (s->dirty_bitmap) { - bdrv_release_dirty_bitmap(s->dirty_bitmap); - } job_early_fail(&s->common.job); } @@ -2118,6 +2129,7 @@ fail: bdrv_graph_wrunlock(); bdrv_drained_end(bs); + bdrv_release_dirty_bitmap(bs_opaque->dirty_bitmap); bdrv_unref(mirror_top_bs); return NULL; From d4816177654d59e26ce212c436513f01842eb410 Mon Sep 17 00:00:00 2001 From: Dmitry Guryanov Date: Mon, 8 Dec 2025 11:55:28 +0300 Subject: [PATCH 06/12] block/throttle-groups: fix deadlock with iolimits and muliple iothreads Details: https://gitlab.com/qemu-project/qemu/-/issues/3144 The function schedule_next_request is called with tg->lock held and it may call throttle_group_co_restart_queue, which takes tgm->throttled_reqs_lock, qemu_co_mutex_lock may leave current coroutine if other iothread has taken the lock. If the next coroutine will call throttle_group_co_io_limits_intercept - it will try to take the mutex tg->lock which will never be released. Here is the backtrace of the iothread: Thread 30 (Thread 0x7f8aad1fd6c0 (LWP 24240) "IO iothread2"): #0 futex_wait (futex_word=0x5611adb7d828, expected=2, private=0) at ../sysdeps/nptl/futex-internal.h:146 #1 __GI___lll_lock_wait (futex=futex@entry=0x5611adb7d828, private=0) at lowlevellock.c:49 #2 0x00007f8ab5a97501 in lll_mutex_lock_optimized (mutex=0x5611adb7d828) at pthread_mutex_lock.c:48 #3 ___pthread_mutex_lock (mutex=0x5611adb7d828) at pthread_mutex_lock.c:93 #4 0x00005611823f5482 in qemu_mutex_lock_impl (mutex=0x5611adb7d828, file=0x56118289daca "../block/throttle-groups.c", line=372) at ../util/qemu-thread-posix.c:94 #5 0x00005611822b0b39 in throttle_group_co_io_limits_intercept (tgm=0x5611af1bb4d8, bytes=4096, direction=THROTTLE_READ) at ../block/throttle-groups.c:372 #6 0x00005611822473b1 in blk_co_do_preadv_part (blk=0x5611af1bb490, offset=15972311040, bytes=4096, qiov=0x7f8aa4000f98, qiov_offset=0, flags=BDRV_REQ_REGISTERED_BUF) at ../block/block-backend.c:1354 #7 0x0000561182247fa0 in blk_aio_read_entry (opaque=0x7f8aa4005910) at ../block/block-backend.c:1619 #8 0x000056118241952e in coroutine_trampoline (i0=-1543497424, i1=32650) at ../util/coroutine-ucontext.c:175 #9 0x00007f8ab5a56f70 in ?? () at ../sysdeps/unix/sysv/linux/x86_64/__start_context.S:66 from target:/lib64/libc.so.6 #10 0x00007f8aad1ef190 in ?? () #11 0x0000000000000000 in ?? () The lock is taken in line 386: (gdb) p tg.lock $1 = {lock = {__data = {__lock = 2, __count = 0, __owner = 24240, __nusers = 1, __kind = 0, __spins = 0, __elision = 0, __list = {__prev = 0x0, __next = 0x0}}, __size = "\002\000\000\000\000\000\000\000\260^\000\000\001", '\000' , __align = 2}, file = 0x56118289daca "../block/throttle-groups.c", line = 386, initialized = true} The solution is to use tg->lock to protect both ThreadGroup fields and ThrottleGroupMember.throttled_reqs. It doesn't seem to be possible to use separate locks because we need to first manipulate ThrottleGroup fields, then schedule next coroutine using throttled_reqs and after than update token field from ThrottleGroup depending on the throttled_reqs state. Signed-off-by: Dmitry Guryanov Message-ID: <20251208085528.890098-1-dmitry.guryanov@gmail.com> Reviewed-by: Hanna Czenczek Signed-off-by: Kevin Wolf --- block/throttle-groups.c | 21 ++++++--------------- include/block/throttle-groups.h | 3 +-- 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/block/throttle-groups.c b/block/throttle-groups.c index 66fdce9a90..5329ff1fdb 100644 --- a/block/throttle-groups.c +++ b/block/throttle-groups.c @@ -295,19 +295,15 @@ static bool throttle_group_schedule_timer(ThrottleGroupMember *tgm, /* Start the next pending I/O request for a ThrottleGroupMember. Return whether * any request was actually pending. * + * This assumes that tg->lock is held. + * * @tgm: the current ThrottleGroupMember * @direction: the ThrottleDirection */ static bool coroutine_fn throttle_group_co_restart_queue(ThrottleGroupMember *tgm, ThrottleDirection direction) { - bool ret; - - qemu_co_mutex_lock(&tgm->throttled_reqs_lock); - ret = qemu_co_queue_next(&tgm->throttled_reqs[direction]); - qemu_co_mutex_unlock(&tgm->throttled_reqs_lock); - - return ret; + return qemu_co_queue_next(&tgm->throttled_reqs[direction]); } /* Look for the next pending I/O request and schedule it. @@ -378,12 +374,8 @@ void coroutine_fn throttle_group_co_io_limits_intercept(ThrottleGroupMember *tgm /* Wait if there's a timer set or queued requests of this type */ if (must_wait || tgm->pending_reqs[direction]) { tgm->pending_reqs[direction]++; - qemu_mutex_unlock(&tg->lock); - qemu_co_mutex_lock(&tgm->throttled_reqs_lock); qemu_co_queue_wait(&tgm->throttled_reqs[direction], - &tgm->throttled_reqs_lock); - qemu_co_mutex_unlock(&tgm->throttled_reqs_lock); - qemu_mutex_lock(&tg->lock); + &tg->lock); tgm->pending_reqs[direction]--; } @@ -410,15 +402,15 @@ static void coroutine_fn throttle_group_restart_queue_entry(void *opaque) ThrottleDirection direction = data->direction; bool empty_queue; + qemu_mutex_lock(&tg->lock); empty_queue = !throttle_group_co_restart_queue(tgm, direction); /* If the request queue was empty then we have to take care of * scheduling the next one */ if (empty_queue) { - qemu_mutex_lock(&tg->lock); schedule_next_request(tgm, direction); - qemu_mutex_unlock(&tg->lock); } + qemu_mutex_unlock(&tg->lock); g_free(data); @@ -569,7 +561,6 @@ void throttle_group_register_tgm(ThrottleGroupMember *tgm, read_timer_cb, write_timer_cb, tgm); - qemu_co_mutex_init(&tgm->throttled_reqs_lock); } /* Unregister a ThrottleGroupMember from its group, removing it from the list, diff --git a/include/block/throttle-groups.h b/include/block/throttle-groups.h index 2355e8d9de..7dfc81f7b5 100644 --- a/include/block/throttle-groups.h +++ b/include/block/throttle-groups.h @@ -35,8 +35,7 @@ typedef struct ThrottleGroupMember { AioContext *aio_context; - /* throttled_reqs_lock protects the CoQueues for throttled requests. */ - CoMutex throttled_reqs_lock; + /* Protected by ThrottleGroup.lock */ CoQueue throttled_reqs[THROTTLE_MAX]; /* Nonzero if the I/O limits are currently being ignored; generally From 544ddbb6373d61292a0e2dc269809cd6bd5edec6 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 4 Mar 2026 13:28:00 +0100 Subject: [PATCH 07/12] block: Never drop BLOCK_IO_ERROR with action=stop for rate limiting Commit 2155d2dd introduced rate limiting for BLOCK_IO_ERROR to emit an event only once a second. This makes sense for cases in which the guest keeps running and can submit more requests that would possibly also fail because there is a problem with the backend. However, if the error policy is configured so that the VM is stopped on errors, this is both unnecessary because stopping the VM means that the guest can't issue more requests and in fact harmful because stopping the VM is an important state change that management tools need to keep track of even if it happens more than once in a given second. If an event is dropped, the management tool would see a VM randomly going to paused state without an associated error, so it has a hard time deciding how to handle the situation. This patch disables rate limiting for action=stop by not relying on the event type alone any more in monitor_qapi_event_queue_no_reenter(), but checking action for BLOCK_IO_ERROR, too. If the error is reported to the guest or ignored, the rate limiting stays in place. Fixes: 2155d2dd7f73 ('block-backend: per-device throttling of BLOCK_IO_ERROR reports') Signed-off-by: Kevin Wolf Message-ID: <20260304122800.51923-1-kwolf@redhat.com> Signed-off-by: Kevin Wolf --- monitor/monitor.c | 21 ++++++++++++++++++++- qapi/block-core.json | 2 +- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/monitor/monitor.c b/monitor/monitor.c index 1273eb7260..37fa674cfe 100644 --- a/monitor/monitor.c +++ b/monitor/monitor.c @@ -367,14 +367,33 @@ monitor_qapi_event_queue_no_reenter(QAPIEvent event, QDict *qdict) { MonitorQAPIEventConf *evconf; MonitorQAPIEventState *evstate; + bool throttled; assert(event < QAPI_EVENT__MAX); evconf = &monitor_qapi_event_conf[event]; trace_monitor_protocol_event_queue(event, qdict, evconf->rate); + throttled = evconf->rate; + + /* + * Rate limit BLOCK_IO_ERROR only for action != "stop". + * + * If the VM is stopped after an I/O error, this is important information + * for the management tool to keep track of the state of QEMU and we can't + * merge any events. At the same time, stopping the VM means that the guest + * can't send additional requests and the number of events is already + * limited, so we can do without rate limiting. + */ + if (event == QAPI_EVENT_BLOCK_IO_ERROR) { + QDict *data = qobject_to(QDict, qdict_get(qdict, "data")); + const char *action = qdict_get_str(data, "action"); + if (!strcmp(action, "stop")) { + throttled = false; + } + } QEMU_LOCK_GUARD(&monitor_lock); - if (!evconf->rate) { + if (!throttled) { /* Unthrottled event */ monitor_qapi_event_emit(event, qdict); } else { diff --git a/qapi/block-core.json b/qapi/block-core.json index b66bf316e2..da0b36a375 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -5794,7 +5794,7 @@ # .. note:: If action is "stop", a `STOP` event will eventually follow # the `BLOCK_IO_ERROR` event. # -# .. note:: This event is rate-limited. +# .. note:: This event is rate-limited, except if action is "stop". # # Since: 0.13 # From 1d6610099bd7fc159626a38e60a3c84343ff67f7 Mon Sep 17 00:00:00 2001 From: Hanna Czenczek Date: Fri, 2 Jan 2026 16:32:46 +0100 Subject: [PATCH 08/12] block/nfs: Do not enter coroutine from CB MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reasoning I gave for why it would be safe to call aio_co_wake() despite holding the mutex was wrong: It is true that the current request will not re-acquire the mutex, but a subsequent request in the same coroutine can. Because the mutex is a non-coroutine mutex, this will result in a deadlock. Therefore, we must either not enter the coroutine here (only scheduling it), or release the mutex around aio_co_wake(). I opt for the former, as it is the behavior prior to the offending commit, and so seems safe to do. Fixes: deb35c129b859b9bec70fd42f856a0b7c1dc6e61 ("nfs: Run co BH CB in the coroutine’s AioContext") Buglink: https://gitlab.com/qemu-project/qemu/-/issues/2622#note_2965097035 Cc: qemu-stable@nongnu.org Signed-off-by: Hanna Czenczek Message-ID: <20260102153246.154207-1-hreitz@redhat.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block/nfs.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/block/nfs.c b/block/nfs.c index 1d3a34a30c..b78f4f86e8 100644 --- a/block/nfs.c +++ b/block/nfs.c @@ -249,14 +249,15 @@ nfs_co_generic_cb(int ret, struct nfs_context *nfs, void *data, } /* - * Safe to call: nfs_service(), which called us, is only run from the FD - * handlers, never from the request coroutine. The request coroutine in - * turn will yield unconditionally. - * No need to release the lock, even if we directly enter the coroutine, as - * the lock is never re-taken after yielding. (Note: If we do enter the - * coroutine, @task will probably be dangling once aio_co_wake() returns.) + * Using aio_co_wake() here could re-enter the coroutine directly, while we + * still hold the mutex. The current request will not attempt to re-take + * the mutex, so that is fine; but if the same coroutine then goes on to + * submit another request, that new request will try to re-take the mutex, + * resulting in a deadlock. + * To prevent that, only schedule the coroutine so it will be entered later, + * with the mutex released. */ - aio_co_wake(task->co); + aio_co_schedule(qemu_coroutine_get_aio_context(task->co), task->co); } static int coroutine_fn nfs_co_preadv(BlockDriverState *bs, int64_t offset, @@ -716,8 +717,8 @@ nfs_get_allocated_file_size_cb(int ret, struct nfs_context *nfs, void *data, if (task->ret < 0) { error_report("NFS Error: %s", nfs_get_error(nfs)); } - /* Safe to call, see nfs_co_generic_cb() */ - aio_co_wake(task->co); + /* Must not use aio_co_wake(), see nfs_co_generic_cb() */ + aio_co_schedule(qemu_coroutine_get_aio_context(task->co), task->co); } static int64_t coroutine_fn nfs_co_get_allocated_file_size(BlockDriverState *bs) From 910451bc5b08b45863e173b58cbf2288b82d9fd2 Mon Sep 17 00:00:00 2001 From: Hanna Czenczek Date: Fri, 30 May 2025 10:44:44 +0200 Subject: [PATCH 09/12] qcow2: Add keep_data_file command-line option Add a command-line-only option to prevent overwriting the file specified as external data file. This option is only available on the qemu-img create command line, not via blockdev-create, as it makes no sense there: That interface separates file creation and formatting, so where the external data file attached to a newly formatted qcow2 node comes from is completely up to the user. Implementation detail: Enabling this option will not only not overwrite the external data file, but also assume it already exists, for two reasons: - It is simpler than checking whether the file exists, and only skipping creating it when it does not. It is therefore also less error-prone, i.e. we can never accidentally overwrite an existing file because we made some mistake in checking whether it exists. - I think it makes sense from a user's perspective: You set this option when you want to use an existing data file, and you unset it when you want a new one. Getting an error when you expect to use an existing data file seems to me a nice warning that something is not right. Signed-off-by: Hanna Czenczek Message-ID: <20250530084448.192369-2-hreitz@redhat.com> Reviewed-by: Eric Blake [kwolf: Removed redundant has_data_file_raw check] Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block/qcow2.c | 72 ++++++++++++++++++++++++++++++-- include/block/block_int-common.h | 1 + tests/qemu-iotests/082.out | 18 ++++++++ 3 files changed, 87 insertions(+), 4 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index e29810d86a..e372880620 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -3991,6 +3991,8 @@ qcow2_co_create_opts(BlockDriver *drv, const char *filename, QemuOpts *opts, BlockDriverState *bs = NULL; BlockDriverState *data_bs = NULL; const char *val; + bool keep_data_file = false; + BlockdevCreateOptionsQcow2 *qcow2_opts; int ret; /* Only the keyval visitor supports the dotted syntax needed for @@ -4022,6 +4024,22 @@ qcow2_co_create_opts(BlockDriver *drv, const char *filename, QemuOpts *opts, qdict_put_str(qdict, BLOCK_OPT_COMPAT_LEVEL, "v3"); } + val = qdict_get_try_str(qdict, BLOCK_OPT_KEEP_DATA_FILE); + if (val) { + if (!strcmp(val, "on")) { + keep_data_file = true; + } else if (!strcmp(val, "off")) { + keep_data_file = false; + } else { + error_setg(errp, + "Invalid value '%s' for '%s': Must be 'on' or 'off'", + val, BLOCK_OPT_KEEP_DATA_FILE); + ret = -EINVAL; + goto finish; + } + qdict_del(qdict, BLOCK_OPT_KEEP_DATA_FILE); + } + /* Change legacy command line options into QMP ones */ static const QDictRenames opt_renames[] = { { BLOCK_OPT_BACKING_FILE, "backing-file" }, @@ -4058,9 +4076,11 @@ qcow2_co_create_opts(BlockDriver *drv, const char *filename, QemuOpts *opts, /* Create and open an external data file (protocol layer) */ val = qdict_get_try_str(qdict, BLOCK_OPT_DATA_FILE); if (val) { - ret = bdrv_co_create_file(val, opts, false, errp); - if (ret < 0) { - goto finish; + if (!keep_data_file) { + ret = bdrv_co_create_file(val, opts, false, errp); + if (ret < 0) { + goto finish; + } } data_bs = bdrv_co_open(val, NULL, NULL, @@ -4073,6 +4093,11 @@ qcow2_co_create_opts(BlockDriver *drv, const char *filename, QemuOpts *opts, qdict_del(qdict, BLOCK_OPT_DATA_FILE); qdict_put_str(qdict, "data-file", data_bs->node_name); + } else if (keep_data_file) { + error_setg(errp, "Must not use '%s=on' without '%s'", + BLOCK_OPT_KEEP_DATA_FILE, BLOCK_OPT_DATA_FILE); + ret = -EINVAL; + goto finish; } /* Set 'driver' and 'node' options */ @@ -4093,6 +4118,37 @@ qcow2_co_create_opts(BlockDriver *drv, const char *filename, QemuOpts *opts, goto finish; } + qcow2_opts = &create_options->u.qcow2; + + if (!qcow2_opts->has_preallocation) { + qcow2_opts->preallocation = PREALLOC_MODE_OFF; + } + + if (keep_data_file && + qcow2_opts->preallocation != PREALLOC_MODE_OFF && + qcow2_opts->preallocation != PREALLOC_MODE_METADATA) + { + error_setg(errp, "Preallocating more than only metadata would " + "overwrite the external data file's content and is " + "therefore incompatible with '%s=on'", + BLOCK_OPT_KEEP_DATA_FILE); + ret = -EINVAL; + goto finish; + } + + if (keep_data_file && + qcow2_opts->preallocation == PREALLOC_MODE_OFF && + !qcow2_opts->data_file_raw) + { + error_setg(errp, "'%s=on' requires '%s=metadata' or '%s=on', or the " + "file contents will not be visible", + BLOCK_OPT_KEEP_DATA_FILE, + BLOCK_OPT_PREALLOC, + BLOCK_OPT_DATA_FILE_RAW); + ret = -EINVAL; + goto finish; + } + /* Silently round up size */ create_options->u.qcow2.size = ROUND_UP(create_options->u.qcow2.size, BDRV_SECTOR_SIZE); @@ -4103,7 +4159,9 @@ finish: if (ret < 0) { bdrv_graph_co_rdlock(); bdrv_co_delete_file_noerr(bs); - bdrv_co_delete_file_noerr(data_bs); + if (!keep_data_file) { + bdrv_co_delete_file_noerr(data_bs); + } bdrv_graph_co_rdunlock(); } else { ret = 0; @@ -6202,6 +6260,12 @@ static QemuOptsList qcow2_create_opts = { .help = "Compression method used for image cluster " \ "compression", \ .def_value_str = "zlib" \ + }, \ + { \ + .name = BLOCK_OPT_KEEP_DATA_FILE, \ + .type = QEMU_OPT_BOOL, \ + .help = "Assume the external data file already exists and " \ + "do not overwrite it" \ }, QCOW_COMMON_OPTIONS, { /* end of list */ } diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index 9324af903d..147c08155f 100644 --- a/include/block/block_int-common.h +++ b/include/block/block_int-common.h @@ -56,6 +56,7 @@ #define BLOCK_OPT_DATA_FILE_RAW "data_file_raw" #define BLOCK_OPT_COMPRESSION_TYPE "compression_type" #define BLOCK_OPT_EXTL2 "extended_l2" +#define BLOCK_OPT_KEEP_DATA_FILE "keep_data_file" #define BLOCK_PROBE_BUF_SIZE 512 diff --git a/tests/qemu-iotests/082.out b/tests/qemu-iotests/082.out index d0dd333117..e0463815c6 100644 --- a/tests/qemu-iotests/082.out +++ b/tests/qemu-iotests/082.out @@ -66,6 +66,7 @@ Supported options: encryption= - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes) extended_l2= - Extended L2 tables extent_size_hint= - Extent size hint for the image file, 0 to disable + keep_data_file= - Assume the external data file already exists and do not overwrite it lazy_refcounts= - Postpone refcount updates nocow= - Turn off copy-on-write (valid only on btrfs) preallocation= - Preallocation mode (allowed values: off, metadata, falloc, full) @@ -92,6 +93,7 @@ Supported options: encryption= - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes) extended_l2= - Extended L2 tables extent_size_hint= - Extent size hint for the image file, 0 to disable + keep_data_file= - Assume the external data file already exists and do not overwrite it lazy_refcounts= - Postpone refcount updates nocow= - Turn off copy-on-write (valid only on btrfs) preallocation= - Preallocation mode (allowed values: off, metadata, falloc, full) @@ -118,6 +120,7 @@ Supported options: encryption= - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes) extended_l2= - Extended L2 tables extent_size_hint= - Extent size hint for the image file, 0 to disable + keep_data_file= - Assume the external data file already exists and do not overwrite it lazy_refcounts= - Postpone refcount updates nocow= - Turn off copy-on-write (valid only on btrfs) preallocation= - Preallocation mode (allowed values: off, metadata, falloc, full) @@ -144,6 +147,7 @@ Supported options: encryption= - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes) extended_l2= - Extended L2 tables extent_size_hint= - Extent size hint for the image file, 0 to disable + keep_data_file= - Assume the external data file already exists and do not overwrite it lazy_refcounts= - Postpone refcount updates nocow= - Turn off copy-on-write (valid only on btrfs) preallocation= - Preallocation mode (allowed values: off, metadata, falloc, full) @@ -170,6 +174,7 @@ Supported options: encryption= - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes) extended_l2= - Extended L2 tables extent_size_hint= - Extent size hint for the image file, 0 to disable + keep_data_file= - Assume the external data file already exists and do not overwrite it lazy_refcounts= - Postpone refcount updates nocow= - Turn off copy-on-write (valid only on btrfs) preallocation= - Preallocation mode (allowed values: off, metadata, falloc, full) @@ -196,6 +201,7 @@ Supported options: encryption= - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes) extended_l2= - Extended L2 tables extent_size_hint= - Extent size hint for the image file, 0 to disable + keep_data_file= - Assume the external data file already exists and do not overwrite it lazy_refcounts= - Postpone refcount updates nocow= - Turn off copy-on-write (valid only on btrfs) preallocation= - Preallocation mode (allowed values: off, metadata, falloc, full) @@ -222,6 +228,7 @@ Supported options: encryption= - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes) extended_l2= - Extended L2 tables extent_size_hint= - Extent size hint for the image file, 0 to disable + keep_data_file= - Assume the external data file already exists and do not overwrite it lazy_refcounts= - Postpone refcount updates nocow= - Turn off copy-on-write (valid only on btrfs) preallocation= - Preallocation mode (allowed values: off, metadata, falloc, full) @@ -248,6 +255,7 @@ Supported options: encryption= - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes) extended_l2= - Extended L2 tables extent_size_hint= - Extent size hint for the image file, 0 to disable + keep_data_file= - Assume the external data file already exists and do not overwrite it lazy_refcounts= - Postpone refcount updates nocow= - Turn off copy-on-write (valid only on btrfs) preallocation= - Preallocation mode (allowed values: off, metadata, falloc, full) @@ -288,6 +296,7 @@ Supported qcow2 options: encrypt.key-secret= - ID of secret providing qcow AES key or LUKS passphrase encryption= - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes) extended_l2= - Extended L2 tables + keep_data_file= - Assume the external data file already exists and do not overwrite it lazy_refcounts= - Postpone refcount updates preallocation= - Preallocation mode (allowed values: off, metadata, falloc, full) refcount_bits= - Width of a reference count entry in bits @@ -376,6 +385,7 @@ Supported options: encryption= - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes) extended_l2= - Extended L2 tables extent_size_hint= - Extent size hint for the image file, 0 to disable + keep_data_file= - Assume the external data file already exists and do not overwrite it lazy_refcounts= - Postpone refcount updates nocow= - Turn off copy-on-write (valid only on btrfs) preallocation= - Preallocation mode (allowed values: off, metadata, falloc, full) @@ -402,6 +412,7 @@ Supported options: encryption= - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes) extended_l2= - Extended L2 tables extent_size_hint= - Extent size hint for the image file, 0 to disable + keep_data_file= - Assume the external data file already exists and do not overwrite it lazy_refcounts= - Postpone refcount updates nocow= - Turn off copy-on-write (valid only on btrfs) preallocation= - Preallocation mode (allowed values: off, metadata, falloc, full) @@ -428,6 +439,7 @@ Supported options: encryption= - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes) extended_l2= - Extended L2 tables extent_size_hint= - Extent size hint for the image file, 0 to disable + keep_data_file= - Assume the external data file already exists and do not overwrite it lazy_refcounts= - Postpone refcount updates nocow= - Turn off copy-on-write (valid only on btrfs) preallocation= - Preallocation mode (allowed values: off, metadata, falloc, full) @@ -454,6 +466,7 @@ Supported options: encryption= - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes) extended_l2= - Extended L2 tables extent_size_hint= - Extent size hint for the image file, 0 to disable + keep_data_file= - Assume the external data file already exists and do not overwrite it lazy_refcounts= - Postpone refcount updates nocow= - Turn off copy-on-write (valid only on btrfs) preallocation= - Preallocation mode (allowed values: off, metadata, falloc, full) @@ -480,6 +493,7 @@ Supported options: encryption= - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes) extended_l2= - Extended L2 tables extent_size_hint= - Extent size hint for the image file, 0 to disable + keep_data_file= - Assume the external data file already exists and do not overwrite it lazy_refcounts= - Postpone refcount updates nocow= - Turn off copy-on-write (valid only on btrfs) preallocation= - Preallocation mode (allowed values: off, metadata, falloc, full) @@ -506,6 +520,7 @@ Supported options: encryption= - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes) extended_l2= - Extended L2 tables extent_size_hint= - Extent size hint for the image file, 0 to disable + keep_data_file= - Assume the external data file already exists and do not overwrite it lazy_refcounts= - Postpone refcount updates nocow= - Turn off copy-on-write (valid only on btrfs) preallocation= - Preallocation mode (allowed values: off, metadata, falloc, full) @@ -532,6 +547,7 @@ Supported options: encryption= - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes) extended_l2= - Extended L2 tables extent_size_hint= - Extent size hint for the image file, 0 to disable + keep_data_file= - Assume the external data file already exists and do not overwrite it lazy_refcounts= - Postpone refcount updates nocow= - Turn off copy-on-write (valid only on btrfs) preallocation= - Preallocation mode (allowed values: off, metadata, falloc, full) @@ -558,6 +574,7 @@ Supported options: encryption= - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes) extended_l2= - Extended L2 tables extent_size_hint= - Extent size hint for the image file, 0 to disable + keep_data_file= - Assume the external data file already exists and do not overwrite it lazy_refcounts= - Postpone refcount updates nocow= - Turn off copy-on-write (valid only on btrfs) preallocation= - Preallocation mode (allowed values: off, metadata, falloc, full) @@ -598,6 +615,7 @@ Supported qcow2 options: encrypt.key-secret= - ID of secret providing qcow AES key or LUKS passphrase encryption= - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes) extended_l2= - Extended L2 tables + keep_data_file= - Assume the external data file already exists and do not overwrite it lazy_refcounts= - Postpone refcount updates preallocation= - Preallocation mode (allowed values: off, metadata, falloc, full) refcount_bits= - Width of a reference count entry in bits From 695d481a12a1190e1dd71761f0393047f6e16932 Mon Sep 17 00:00:00 2001 From: Hanna Czenczek Date: Fri, 30 May 2025 10:44:45 +0200 Subject: [PATCH 10/12] qcow2: Simplify size round-up in co_create_opts Use the now-existing qcow2_opts pointer to simplify the size rounding up code. Signed-off-by: Hanna Czenczek Message-ID: <20250530084448.192369-3-hreitz@redhat.com> Reviewed-by: Eric Blake Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block/qcow2.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index e372880620..81fd299b4c 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -4150,8 +4150,7 @@ qcow2_co_create_opts(BlockDriver *drv, const char *filename, QemuOpts *opts, } /* Silently round up size */ - create_options->u.qcow2.size = ROUND_UP(create_options->u.qcow2.size, - BDRV_SECTOR_SIZE); + qcow2_opts->size = ROUND_UP(qcow2_opts->size, BDRV_SECTOR_SIZE); /* Create the qcow2 image (format layer) */ ret = qcow2_co_create(create_options, errp); From 21b4e03b7d2f12ec269168380fe1f99afeda3835 Mon Sep 17 00:00:00 2001 From: Hanna Czenczek Date: Fri, 30 May 2025 10:44:46 +0200 Subject: [PATCH 11/12] iotests/common.filter: Sort keep_data_file Sort the new keep_data_file creation option together with data_file and data_file_raw. Signed-off-by: Hanna Czenczek Message-ID: <20250530084448.192369-4-hreitz@redhat.com> Reviewed-by: Eric Blake Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- tests/qemu-iotests/common.filter | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter index 26e6b45b04..ce4213cb97 100644 --- a/tests/qemu-iotests/common.filter +++ b/tests/qemu-iotests/common.filter @@ -182,7 +182,7 @@ _do_filter_img_create() -e 's/^\(fmt\)/0-\1/' \ -e 's/^\(size\)/1-\1/' \ -e 's/^\(backing\)/2-\1/' \ - -e 's/^\(data_file\)/3-\1/' \ + -e 's/^\(\(keep_\)\?data_file\)/3-\1/' \ -e 's/^\(encryption\)/4-\1/' \ -e 's/^\(preallocation\)/8-\1/' \ | LC_ALL=C sort \ From a844ad5da2d0d16bab819c6b50c9cba0012ed202 Mon Sep 17 00:00:00 2001 From: Hanna Czenczek Date: Fri, 30 May 2025 10:44:47 +0200 Subject: [PATCH 12/12] iotests/244: Add test cases for keep_data_file Add various test cases around keep_data_file to the existing data_file test suite 244. Signed-off-by: Hanna Czenczek Message-ID: <20250530084448.192369-5-hreitz@redhat.com> Reviewed-by: Eric Blake [kwolf: Added prealloc=full to the test] Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- tests/qemu-iotests/244 | 71 ++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/244.out | 61 ++++++++++++++++++++++++++++++++ 2 files changed, 132 insertions(+) diff --git a/tests/qemu-iotests/244 b/tests/qemu-iotests/244 index bb9cc6512f..2a98ad58f5 100755 --- a/tests/qemu-iotests/244 +++ b/tests/qemu-iotests/244 @@ -384,6 +384,77 @@ $QEMU_IMG compare --image-opts \ "driver=raw,file.filename=$TEST_IMG.data" \ "file.filename=$TEST_IMG,backing.file.filename=$TEST_IMG.base" +echo +echo '=== keep_data_file tests ===' + +echo +echo '--- Creating test data file ---' + +# Easiest way to create the raw data file without having to create and +# access it manually +_make_test_img -o "data_file=$TEST_IMG.data,data_file_raw=on" 1M +# Values chosen by a fair random.org evaluation +$QEMU_IO -c 'write -P 3 0 512k' -c 'write -P 96 512k 512k' "$TEST_IMG" | + _filter_qemu_io + +echo +echo '--- Testing stand-alone option ---' + +# Cannot work, needs data file +_make_test_img -o "keep_data_file=on" 1M + +# Invalid option value +_make_test_img -o "keep_data_file=true" 1M + +# Should be the same as omitting +_make_test_img -o "keep_data_file=off" 1M + +# No preallocation is OK when also specifying data_file_raw; otherwise, none of +# the data file will be mapped, i.e. its contents will stay hidden, so +# requesting its contents to be kept (but hidden) doesn't make much sense. +# +# Metadata preallocation is OK: It will not overwrite the data file's contents, +# but ensure the contents are mapped and visible. +# +# Any data preallocation (like falloc) is not OK, as this would overwrite the +# data file's contents despite keep_data_file requesting they should not be +# overwritten. +# +# Note that all of these cases use the data file created above: This verifies +# that when passing keep_data_file=on, the data file is always kept as-is (and +# e.g. not deleted on error). +for prealloc in off metadata falloc full; do + # Without metadata preallocation, the data_file_raw flag is required so that + # the data file's contents are visible. + for data_file_raw in off on; do + echo + echo "--- Testing prealloc=$prealloc data_file_raw=$data_file_raw ---" + + # Remove previously existing qcow2 (metadata) file + _cleanup_test_img + + opts="data_file=$TEST_IMG.data,keep_data_file=on" + opts+=",preallocation=$prealloc" + opts+=",data_file_raw=$data_file_raw" + + _make_test_img -o "$opts" 1M + if [ -f "$TEST_IMG" ]; then + $QEMU_IO -c 'read -P 3 0 512k' -c 'read -P 96 512k 512k' "$TEST_IMG" | + _filter_qemu_io + fi + done +done + +echo +echo '--- Testing non-existent data file ---' + +# Maybe a matter of taste whether this should fail or create the file, but +# failing is simpler (= will always skip create) and seems safer (users may +# expect the file to exist, and the error will warn them when it does not). +_make_test_img \ + -o "data_file=$TEST_IMG.doesnotexist,keep_data_file=on,data_file_raw=on" \ + 1M + # success, all done echo "*** done" rm -f $seq.full diff --git a/tests/qemu-iotests/244.out b/tests/qemu-iotests/244.out index f46cfe93f1..c62f5aec25 100644 --- a/tests/qemu-iotests/244.out +++ b/tests/qemu-iotests/244.out @@ -197,4 +197,65 @@ wrote 1048576/1048576 bytes at offset 0 Comparing qcow2 image and raw data file: Images are identical. + +=== keep_data_file tests === + +--- Creating test data file --- +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 data_file=TEST_DIR/t.IMGFMT.data data_file_raw=on +wrote 524288/524288 bytes at offset 0 +512 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +wrote 524288/524288 bytes at offset 524288 +512 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +--- Testing stand-alone option --- +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 keep_data_file=on +qemu-img: TEST_DIR/t.IMGFMT: Must not use 'keep_data_file=on' without 'data_file' +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 keep_data_file=true +qemu-img: TEST_DIR/t.IMGFMT: Invalid value 'true' for 'keep_data_file': Must be 'on' or 'off' +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 keep_data_file=off + +--- Testing prealloc=off data_file_raw=off --- +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 data_file=TEST_DIR/t.IMGFMT.data data_file_raw=off keep_data_file=on preallocation=off +qemu-img: TEST_DIR/t.IMGFMT: 'keep_data_file=on' requires 'preallocation=metadata' or 'data_file_raw=on', or the file contents will not be visible + +--- Testing prealloc=off data_file_raw=on --- +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 data_file=TEST_DIR/t.IMGFMT.data data_file_raw=on keep_data_file=on preallocation=off +read 524288/524288 bytes at offset 0 +512 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 524288/524288 bytes at offset 524288 +512 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +--- Testing prealloc=metadata data_file_raw=off --- +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 data_file=TEST_DIR/t.IMGFMT.data data_file_raw=off keep_data_file=on preallocation=metadata +read 524288/524288 bytes at offset 0 +512 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 524288/524288 bytes at offset 524288 +512 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +--- Testing prealloc=metadata data_file_raw=on --- +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 data_file=TEST_DIR/t.IMGFMT.data data_file_raw=on keep_data_file=on preallocation=metadata +read 524288/524288 bytes at offset 0 +512 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 524288/524288 bytes at offset 524288 +512 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +--- Testing prealloc=falloc data_file_raw=off --- +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 data_file=TEST_DIR/t.IMGFMT.data data_file_raw=off keep_data_file=on preallocation=falloc +qemu-img: TEST_DIR/t.IMGFMT: Preallocating more than only metadata would overwrite the external data file's content and is therefore incompatible with 'keep_data_file=on' + +--- Testing prealloc=falloc data_file_raw=on --- +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 data_file=TEST_DIR/t.IMGFMT.data data_file_raw=on keep_data_file=on preallocation=falloc +qemu-img: TEST_DIR/t.IMGFMT: Preallocating more than only metadata would overwrite the external data file's content and is therefore incompatible with 'keep_data_file=on' + +--- Testing prealloc=full data_file_raw=off --- +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 data_file=TEST_DIR/t.IMGFMT.data data_file_raw=off keep_data_file=on preallocation=full +qemu-img: TEST_DIR/t.IMGFMT: Preallocating more than only metadata would overwrite the external data file's content and is therefore incompatible with 'keep_data_file=on' + +--- Testing prealloc=full data_file_raw=on --- +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 data_file=TEST_DIR/t.IMGFMT.data data_file_raw=on keep_data_file=on preallocation=full +qemu-img: TEST_DIR/t.IMGFMT: Preallocating more than only metadata would overwrite the external data file's content and is therefore incompatible with 'keep_data_file=on' + +--- Testing non-existent data file --- +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 data_file=TEST_DIR/t.IMGFMT.doesnotexist data_file_raw=on keep_data_file=on +qemu-img: TEST_DIR/t.IMGFMT: Could not open 'TEST_DIR/t.IMGFMT.doesnotexist': No such file or directory *** done