Re: [PATCH v3 4/4] qemu-img: Deprecate use of -b without -F

[Kashyap Chamarthy <kchamart@redhat.com>]

On Mon, Mar 09, 2020 at 10:42:25AM -0500, Eric Blake wrote:
> On 3/9/20 10:31 AM, Kashyap Chamarthy wrote:
> 
> > After (with the patch series applied to QEMU Git):
> > 
> >      $> git describe
> >      v4.2.0-2204-gd6c7830114
> > 
> >      # Create; *without* specifying "-F raw"
> >      $> ~/build/qemu/qemu-img create -f qcow2 -b ./base.raw ./overlay2.qcow2
> >      qemu-img: warning: Deprecated use of backing file without explicit backing format (detected format of raw)
> >      Formatting './overlay2.qcow2', fmt=qcow2 size=4294967296 backing_file=./base.raw backing_fmt=raw cluster_size=65536 lazy_refcounts=off refcount_bits=16
> 
> If you'll note, this case _did_ write an implied backing_fmt=raw into the
> image.  

Ah, I missed to notice that.  Interesting.

> Constrast that with creating an image on a qcow2 backing file, which
> tells you it detected a format of qcow2, but does NOT write
> backing_fmt=qcow2 into the image (this was a change from v2, at Peter's
> request).  

Indeed, here we go, confirming the overlay of a QCOW2 backing file _not_
having the 'backing_fmt' written into the image:

    $> ~/build/qemu/qemu-img create -f qcow2 -b ./another_base.qcow2 ./overlay1_of_ab.qcow2
    qemu-img: warning: Deprecated use of backing file without explicit backing format (detected format of qcow2)
    Formatting './overlay1_of_ab.qcow2', fmt=qcow2 size=4294967296 backing_file=./another_base.qcow2 cluster_size=65536 lazy_refcounts=off refcount_bits=16

That's neat.

(I wonder if this bit should also be documented.)

> Thus, when the backing is raw, we warn but future use of the
> image is now safe where it previously was not; when the backing file is
> non-raw, we warn but do not change our behavior, but because the backing
> file is non-raw any future probes will not be any less safe than before.

Understood.  Easy to miss when not paying attention; thanks for pointing
it out.

[...]

> > However, for the "Convert" case, is it correct that no warning is thrown
> > for the below?
> > 
> >      $> ~/build/qemu/qemu-img info overlay1.qcow2
> >      image: overlay1.qcow2
> >      file format: qcow2
> >      virtual size: 4 GiB (4294967296 bytes)
> >      disk size: 196 KiB
> >      cluster_size: 65536
> >      backing file: base.raw
> >      Format specific information:
> >          compat: 1.1
> >          lazy refcounts: false
> >          refcount bits: 16
> >          corrupt: false
> 
> We have an image with no backing format, so we had to probe.  This patch
> series did not change the behavior of opening an existing image, only for
> creating a new image (or amending an image in-place).  So the lack of a
> warning on opening the unsafe image may be desirable, but it would be via
> even more patches.

Fair enough; it's an understandable compromise.

And your series is a strict improvement as-is; it should not be held up.

> 
> >      $> ~/build/qemu/qemu-img convert -f qcow2 -O qcow2 overlay1.qcow2 flattened.raw
> 
> Ouch - you are creating a qcow2 destination file named 'flattened.raw',
> which is rather confusing on your part.

Oops, yes it is; bad me.  Sorry for the mix-up.  I meant to amend the
format to 'raw'.

> However, as your destination file is being created without a backing image,
> it is to be expected that there is no warning (when there is no backing
> file, -F makes no sense).  

Yeah, fair enough.

> To provoke the warning during convert, you'll
> have to also pass -B (or -o backing_file), without -o backing_fmt (since
> convert lacks the -F shorthand).

Hmm, I tried the following way, but it doesn't provoke the warning:

    $> ~/build/qemu/qemu-img convert -B ./base.raw -O qcow2 overlay1.qcow2 flattened.qcow2

    $> ~/build/qemu/qemu-img info flattened.qcow2 
    image: flattened.qcow2
    file format: qcow2
    virtual size: 4 GiB (4294967296 bytes)
    disk size: 196 KiB
    cluster_size: 65536
    backing file: ./base.raw
    Format specific information:
        compat: 1.1
        lazy refcounts: false
        refcount bits: 16
        corrupt: false

What am I missing?


            - - -

<digression>

Ah, didn't realize the inconsistency of 'convert' lacking the '-F'
shorthand ... which reminds me, there are at least _three_ ways that I
know of, to specify backing file format with 'create':

    $ qemu-img create -f qcow2 -o 'backing_file=./base.raw,backing_fmt=raw' ./overlay1.qcow2
    $ qemu-img create -f qcow2 -b ./base.raw -o backing_fmt=raw overlay1.qcow2
    $ qemu-img create -f qcow2 -b ./base.raw -F raw ./overlay1.qcow2

I'm wondering about the consistency of having all the above three
supported for other operations too.  Now I at least know 'convert' lacks
the "-F".

Not sure how much people care about this :-)

</digression>


[...]

-- 
/kashyap