[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:26:27 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1

07.02.2020 18:12, Max Reitz wrote:
On 07.02.20 15:57, Eric Blake wrote:
On 2/7/20 8:41 AM, Max Reitz wrote:

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.


bdrv_has_zero_init() currently returns 1 if a driver knows that creating
an image results in that image reading as 0.  That means it can return 1
even for non-zero images that were not just created.  Thus, that
interface has both false positives (returning 1 for a non-zero image if
the driver hard-codes it to 1) and false negatives (returning 0 for an
image that happens to read as zero).  The false negatives are less
important (if we don't know if the image is already zero, then zeroing
it again is a waste of time but not semantically wrong) than the false
positives (but as long as you don't rely on bdrv_has_zero_init() unless
you also know the image was just created, you are safely avoiding the
false positives).

And that's the whole point of my series to add a qcow2 persistent bit to
track whether an image has known-zero contents: qemu-img should not be
calling bdrv_has_zero_init(), since it is so finicky on what it means.

Sorry, I was unclear.  I meant “that we know are not zero immediately
after creation”.

My point that it may return false even for (newly created) images that
are zero stands.  One could also say it returns only “yes” (is zero) or
“maybe”, and not “yes” or “no”.

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.

--target-is-zero turns into the behavior of 'skip a pre-zeroing pass'.
If the destination is already zero, then copying zeroes from the source
is a waste of time. If the destination is not already zero, then zeroes
from the source are not copied, and the destination will not be
identical to the source.  We then have a choice of whether
--target-is-zero is merely a way to tell qemu something that it couldn't
learn otherwise but still be as safe as possible (if we can quickly
prove the target has non-zero data, the user lied about it being already
zero, so fail the command, so add yet another option to bypass the
safety check), or whether it really is synonymous with 'only copy the
non-zero portions of the source, and if the destination was not all zero
the copy is not faithful but I meant for it to be that way'.

If you claim that it isn’t supposed to be an unsafe override, and if we
plan to take your series in some form or another, it follows that we
will have to drop this patch here.

Because after your series, qemu can have some insight into existing
images (either in the driver’s implementation of make_zero, or in
qemu-img itself by virtue of some is_zero function).  Therefore, we
could do the same “safety check” and see whether our insight agrees on
what the user told us.

This would make the flag completely superfluous, though, because when
qemu knows the image to be zero, it does the right thing anyway.

Therefore, I see this flag to be for cases where qemu doesn’t know.  And
that makes it an unsafe override.

IMHO, the flag just sounds wrong for images created by Qemu, and sounds OK
for images provided by user (still, it can never be safe, as user may mistake).

Still, as I understand, in most of real cases, Qemu actually can determine
is-zero by itself, we just didn't have this coded (Eric's series does). And
may be we really don't need this arguable flag..

So, really, may be rename it to --skip-zeroes, to make obvious that it is unsafe
and user is responsible for the result? Or even to --x-skip-zeroes, and drop it
when all needed cases are covered by auto detection?

Best regards,

reply via email to

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