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
