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