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

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2900 bytes --]

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 for
>>>>> 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
>>>> initialization..
>>>>
>>>>>
>>>>>> +++ b/qemu-img.c
>>>>>
>>>>>> @@ -2247,6 +2256,11 @@ static int img_convert(int argc, char **argv)
>>>>>>            warn_report("This will become an error in future QEMU
>>>>>> versions.");
>>>>>>        }
>>>>>> +    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 better
>>>> 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 insist
>>> that bdrv_has_zero_init() returns 1 (or, after my followup series,
>>> bdrv_known_zeroes() includes BDRV_ZERO_CREATE).
>>
>> Why?
>>
>> 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
>> --target-is-zero.
> 
> 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,
> when
> we know that what we are creating is not zero is misleading and should
> fail..

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.

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

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]