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

[Vladimir Sementsov-Ogievskiy]

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

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

> OTOH, we can always choose to allow that behavior at a later point.
>
> Max


--
Best regards,
Vladimir