[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: |
Max Reitz |
Subject: |
Re: [PATCH v3 1/1] qemu-img: Add --target-is-zero to convert |
Date: |
Fri, 7 Feb 2020 16:03:31 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1 |
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
>>>>>>> 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.
>
> 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
know.
Max
signature.asc
Description: OpenPGP digital signature
- [PATCH v3 0/1] qemu-img: Add --target-is-zero to indicate that a target is blank, David Edmondson, 2020/02/04
- [PATCH v3 1/1] qemu-img: Add --target-is-zero to convert, David Edmondson, 2020/02/04
- Re: [PATCH v3 1/1] qemu-img: Add --target-is-zero to convert, Eric Blake, 2020/02/04
- Re: [PATCH v3 1/1] qemu-img: Add --target-is-zero to convert, Vladimir Sementsov-Ogievskiy, 2020/02/04
- Re: [PATCH v3 1/1] qemu-img: Add --target-is-zero to convert, Eric Blake, 2020/02/04
- Re: [PATCH v3 1/1] qemu-img: Add --target-is-zero to convert, Max Reitz, 2020/02/07
- Re: [PATCH v3 1/1] qemu-img: Add --target-is-zero to convert, Vladimir Sementsov-Ogievskiy, 2020/02/07
- Re: [PATCH v3 1/1] qemu-img: Add --target-is-zero to convert, Max Reitz, 2020/02/07
- Re: [PATCH v3 1/1] qemu-img: Add --target-is-zero to convert, Vladimir Sementsov-Ogievskiy, 2020/02/07
- Re: [PATCH v3 1/1] qemu-img: Add --target-is-zero to convert,
Max Reitz <=
- Re: [PATCH v3 1/1] qemu-img: Add --target-is-zero to convert, Vladimir Sementsov-Ogievskiy, 2020/02/07
- Re: [PATCH v3 1/1] qemu-img: Add --target-is-zero to convert, Max Reitz, 2020/02/07
- Re: [PATCH v3 1/1] qemu-img: Add --target-is-zero to convert, Eric Blake, 2020/02/07
- Re: [PATCH v3 1/1] qemu-img: Add --target-is-zero to convert, Max Reitz, 2020/02/07
- Re: [PATCH v3 1/1] qemu-img: Add --target-is-zero to convert, Vladimir Sementsov-Ogievskiy, 2020/02/07
- Re: [PATCH v3 1/1] qemu-img: Add --target-is-zero to convert, David Edmondson, 2020/02/04
- Re: [PATCH v3 1/1] qemu-img: Add --target-is-zero to convert, Eric Blake, 2020/02/05
Re: [PATCH v3 1/1] qemu-img: Add --target-is-zero to convert, Vladimir Sementsov-Ogievskiy, 2020/02/04