[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 1/1] qemu-img: Add --target-is-zero to convert

From: Vladimir Sementsov-Ogievskiy
Subject: Re: [PATCH v3 1/1] qemu-img: Add --target-is-zero to convert
Date: Fri, 7 Feb 2020 18:16:10 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1

07.02.2020 18:03, Max Reitz wrote:
On 07.02.20 15:53, Vladimir Sementsov-Ogievskiy wrote:
07.02.2020 17:41, Max Reitz wrote:
On 07.02.20 13:07, Vladimir Sementsov-Ogievskiy wrote:
07.02.2020 13:33, Max Reitz wrote:
On 04.02.20 15:23, Eric Blake wrote:
On 2/4/20 7:59 AM, Vladimir Sementsov-Ogievskiy wrote:

I understand that it is safer to have restrictions now and lift them
later, than to allow use of the option at any time and leave room
the user to shoot themselves in the foot with no way to add safety
later.  The argument against no backing file is somewhat
understandable (technically, as long as the backing file also reads
as all zeroes, then the overall image reads as all zeroes - but why
have a backing file that has no content?); the argument requiring -n
is a bit weaker (if I'm creating an image, I _know_ it reads as all
zeroes, so the --target-is-zero argument is redundant, but it
shouldn't hurt to allow it).

I know that it reads as all zeroes, only if this format provides zero

+++ b/qemu-img.c

@@ -2247,6 +2256,11 @@ static int img_convert(int argc, char
             warn_report("This will become an error in future QEMU
+    if (s.has_zero_init && !skip_create) {
+        error_report("--target-is-zero requires use of -n flag");
+        goto fail_getopt;
+    }

So I think we could drop this hunk with no change in behavior.

I think, no we can't. If we allow target-is-zero, with -n, we'd
to check that what we are creating is zero-initialized (format has
zero-init), and if not we should report error.

Good call.  Yes, if we allow --target-is-zero without -n, we MUST
that bdrv_has_zero_init() returns 1 (or, after my followup series,
bdrv_known_zeroes() includes BDRV_ZERO_CREATE).


I could imagine a user creating a qcow2 image on some block device with
preallocation where we cannot verify that the result will be zero.  But
they want qemu not to zero the device, so they would specify

If user create image, setting --target-is-zero is always valid. But if
we in
same operation create the image automatically, having --target-is-zero,
we know that what we are creating is not zero is misleading and should

bdrv_has_zero_init() doesn’t return false only for images that we know
are not zero.  It returns true for images where we know they are.  But
if we don’t know, then it returns false also.

yes, but we don't have better check.

Correct, but maybe the user knows more, hence why it may make sense for
them to provide us with some information we don’t have.

If we want to add a behavior to skip zeros unconditionally, we should
call new
option --skip-zeroes, to clearly specify what we want.

It was my impression that this was exactly what --target-is-zero means
and implies.

For me it sounds strange that user has better knowledge about what Qemu
creates than Qemu itself. And if it so - it should be fixed in Qemu,
rather than creating user interface to hint Qemu what it does.

I brought an example where qemu cannot know whether the image is zero
(preallocation on a block device), but the user / management layer might

It sounds unsafe for me. User can't know how exactly Qemu do preallocation,
which syscalls it calls, etc. How can user be sure, that Qemu produces
all-zero image, if even Qemu doesn't sure in it?

Otherwise, we should document, how exactly (up to syscalls, their
parameters, flags, the whole logic and algorithm) preallocation is done,
so user can analyze it and be sure that produced image would be all-zero
(when Qemu can't determine it because some specific block device, for which
Qemu doesn't know that its preallocation algorithm produces all-zero, but
user is sure in it)...

Best regards,

reply via email to

[Prev in Thread] Current Thread [Next in Thread]