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 <hreitz@redhat.com>
---
include/block/block_int-common.h | 1 +
block/qcow2.c | 75 ++++++++++++++++++++++++++++++--
tests/qemu-iotests/082.out | 18 ++++++++
3 files changed, 90 insertions(+), 4 deletions(-)
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 2982dd3118..b52e441b42 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -57,6 +57,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/block/qcow2.c b/block/qcow2.c
index 66fba89b41..b11cbfd859 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3902,6 +3902,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
@@ -3933,6 +3935,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" },
@@ -3969,9 +3987,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, errp);
- if (ret < 0) {
- goto finish;
+ if (!keep_data_file) {
+ ret = bdrv_co_create_file(val, opts, errp);
+ if (ret < 0) {
+ goto finish;
+ }
}
data_bs = bdrv_co_open(val, NULL, NULL,
@@ -3984,6 +4004,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 */
@@ -4004,6 +4029,40 @@ 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 (!qcow2_opts->has_data_file_raw) {
+ qcow2_opts->data_file_raw = false;
+ }
+
+ 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);
@@ -4014,7 +4073,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;
@@ -6113,6 +6174,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/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=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
extended_l2=<bool (on/off)> - Extended L2 tables
extent_size_hint=<size> - Extent size hint for the image file, 0 to disable
+ keep_data_file=<bool (on/off)> - Assume the external data file already exists and do not overwrite it
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
@@ -92,6 +93,7 @@ Supported options:
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
extended_l2=<bool (on/off)> - Extended L2 tables
extent_size_hint=<size> - Extent size hint for the image file, 0 to disable
+ keep_data_file=<bool (on/off)> - Assume the external data file already exists and do not overwrite it
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
@@ -118,6 +120,7 @@ Supported options:
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
extended_l2=<bool (on/off)> - Extended L2 tables
extent_size_hint=<size> - Extent size hint for the image file, 0 to disable
+ keep_data_file=<bool (on/off)> - Assume the external data file already exists and do not overwrite it
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
@@ -144,6 +147,7 @@ Supported options:
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
extended_l2=<bool (on/off)> - Extended L2 tables
extent_size_hint=<size> - Extent size hint for the image file, 0 to disable
+ keep_data_file=<bool (on/off)> - Assume the external data file already exists and do not overwrite it
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
@@ -170,6 +174,7 @@ Supported options:
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
extended_l2=<bool (on/off)> - Extended L2 tables
extent_size_hint=<size> - Extent size hint for the image file, 0 to disable
+ keep_data_file=<bool (on/off)> - Assume the external data file already exists and do not overwrite it
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
@@ -196,6 +201,7 @@ Supported options:
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
extended_l2=<bool (on/off)> - Extended L2 tables
extent_size_hint=<size> - Extent size hint for the image file, 0 to disable
+ keep_data_file=<bool (on/off)> - Assume the external data file already exists and do not overwrite it
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
@@ -222,6 +228,7 @@ Supported options:
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
extended_l2=<bool (on/off)> - Extended L2 tables
extent_size_hint=<size> - Extent size hint for the image file, 0 to disable
+ keep_data_file=<bool (on/off)> - Assume the external data file already exists and do not overwrite it
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
@@ -248,6 +255,7 @@ Supported options:
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
extended_l2=<bool (on/off)> - Extended L2 tables
extent_size_hint=<size> - Extent size hint for the image file, 0 to disable
+ keep_data_file=<bool (on/off)> - Assume the external data file already exists and do not overwrite it
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
@@ -288,6 +296,7 @@ Supported qcow2 options:
encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
extended_l2=<bool (on/off)> - Extended L2 tables
+ keep_data_file=<bool (on/off)> - Assume the external data file already exists and do not overwrite it
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
refcount_bits=<num> - Width of a reference count entry in bits
@@ -376,6 +385,7 @@ Supported options:
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
extended_l2=<bool (on/off)> - Extended L2 tables
extent_size_hint=<size> - Extent size hint for the image file, 0 to disable
+ keep_data_file=<bool (on/off)> - Assume the external data file already exists and do not overwrite it
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
@@ -402,6 +412,7 @@ Supported options:
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
extended_l2=<bool (on/off)> - Extended L2 tables
extent_size_hint=<size> - Extent size hint for the image file, 0 to disable
+ keep_data_file=<bool (on/off)> - Assume the external data file already exists and do not overwrite it
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
@@ -428,6 +439,7 @@ Supported options:
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
extended_l2=<bool (on/off)> - Extended L2 tables
extent_size_hint=<size> - Extent size hint for the image file, 0 to disable
+ keep_data_file=<bool (on/off)> - Assume the external data file already exists and do not overwrite it
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
@@ -454,6 +466,7 @@ Supported options:
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
extended_l2=<bool (on/off)> - Extended L2 tables
extent_size_hint=<size> - Extent size hint for the image file, 0 to disable
+ keep_data_file=<bool (on/off)> - Assume the external data file already exists and do not overwrite it
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
@@ -480,6 +493,7 @@ Supported options:
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
extended_l2=<bool (on/off)> - Extended L2 tables
extent_size_hint=<size> - Extent size hint for the image file, 0 to disable
+ keep_data_file=<bool (on/off)> - Assume the external data file already exists and do not overwrite it
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
@@ -506,6 +520,7 @@ Supported options:
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
extended_l2=<bool (on/off)> - Extended L2 tables
extent_size_hint=<size> - Extent size hint for the image file, 0 to disable
+ keep_data_file=<bool (on/off)> - Assume the external data file already exists and do not overwrite it
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
@@ -532,6 +547,7 @@ Supported options:
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
extended_l2=<bool (on/off)> - Extended L2 tables
extent_size_hint=<size> - Extent size hint for the image file, 0 to disable
+ keep_data_file=<bool (on/off)> - Assume the external data file already exists and do not overwrite it
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
@@ -558,6 +574,7 @@ Supported options:
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
extended_l2=<bool (on/off)> - Extended L2 tables
extent_size_hint=<size> - Extent size hint for the image file, 0 to disable
+ keep_data_file=<bool (on/off)> - Assume the external data file already exists and do not overwrite it
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
@@ -598,6 +615,7 @@ Supported qcow2 options:
encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
extended_l2=<bool (on/off)> - Extended L2 tables
+ keep_data_file=<bool (on/off)> - Assume the external data file already exists and do not overwrite it
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
refcount_bits=<num> - Width of a reference count entry in bits
--
2.49.0On Fri, May 30, 2025 at 10:44:44AM +0200, Hanna Czenczek wrote:
> 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.
And it also helps if you accidentally type something like nbd://... as
the data file and expect it to exist, but qemu interprets that as a
local filename ./nbd:/... which (hopefully) does not exist.
>
> Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
> ---
> include/block/block_int-common.h | 1 +
> block/qcow2.c | 75 ++++++++++++++++++++++++++++++--
> tests/qemu-iotests/082.out | 18 ++++++++
> 3 files changed, 90 insertions(+), 4 deletions(-)
>
> @@ -3933,6 +3935,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);
Do we have a function that does this parsing so that we don't have to
open-code copy-paste (with slight differences) this sort of parsing
into similar clients? A quick grep for "'on' or 'off'" shows at least:
monitor/hmp.c: monitor_printf(mon, "Expected 'on' or 'off'\n");
net/filter.c: "should be 'on' or 'off'");
qapi/qapi-util.c: "'on' or 'off'");
qapi/qobject-input-visitor.c: full_name(qiv, name), "'on' or 'off'");
Not necessarily a show-stopper to this patch, but food for thought in
the technical debt department.
> @@ -4004,6 +4029,40 @@ 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 (!qcow2_opts->has_data_file_raw) {
> + qcow2_opts->data_file_raw = false;
> + }
> +
> + 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);
I like this safety check.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
© 2016 - 2025 Red Hat, Inc.