Re: [PATCH 8/9] virStorageFileGetMetadataRecurse: Allow format probing under special circumstances

[Eric Blake]

[adding qemu]

On 2/19/20 12:57 PM, Peter Krempa wrote:

> > > Namely a user creates an overlay on top of single raw/qcow2 image and
> > > expects it to work. And it's not just random users, libguestfs and
> > > openstack also neglected to set the backing format.
> >
> > Yes, and they are getting patched.  Belatedly, but better late than never.
>
> In that case, qemu-img should also be fixed to forbid 'create' without
> -F. Otherwise it's hard to argue that it's a wrong thing to do.

Allowing -b without -F when the backing file probes as raw is safe, but 
yes, I agree qemu-img create should start a deprecation period of 
warning if -F is omitted, and turn it into a hard error when enough time 
elapses.

> > > > > The price for this is that libvirt will need to keep the image format
> > > > > detector still current and working or replace it by invocation of
> > > > > qemu-img.
> > > >
> > > > Maybe.  Any format that qemu recognizes but libvirt does not risks a case
> > > > where libvirt probes the image as raw but lets qemu re-probe the image and
> > >
> > > That doesn't happen with blockdev. We dont' let qemu probe.
> >
> > We are just shifting the burden on who causes the data corruption when a
> > probe goes wrong - it used to be qemu, now it is libvirt.
> >
> >
> > > > I disagree with the logic here.  What we really need is:
> > > >
> > > > If the backing format was not specified, we probe to see what is there. If
> > > > the result of that probe is raw, it is safe to treat the image as raw.  If
> > > > the result is anything else, we must report an error stating that what we
> > > > probed could not be trusted unless the user adds an explicit backing format
> > > > (either confirming that our probe was correct, or with the correct format
> > > > overriding what we mis-probed).
> > >
> > > As noted above, using this logic would be pointless. We are better off
> > > just reporting the error always if we also don't allow qcow2 without
> > > backing file to be used as it was previously used.
> >
> > Then report the error always.
>
> Well that's what we do right now. It seems kind of tempting to call this
> a right thing but let me summarize the semantics:
>
> - The "true" detection cases are not problematic
>      - advantage is that existing (arguably suboptimal) configurations
>        will keep working if we detect
> - For the "false" detection cases:
>    - misdetection does not breach security (given that sVirt is used)
>    - data corruption can occur if libvirt detects something else than
>      qemu
>      - this is true both directions (qcow2->raw, raw->qcow2)
>
> and the tradeoff:
>
> 1) If we allow detection, we trade compatibility for the (small)
> possibility of having to deal with corruption.
>
> 2) If we disallow detection we trade regression of behaviour for data
> corruption not being our problem.
>
> I started this trhead because I feel that the value of 1) is more than
> 2). Especially short term since qemu-img's default behaviour is allowing
> creation of images which would break with libvirt and the fact that
> we've tolerated the wrong behaviour for years.
>
> Additionally I think that we could just get rid of the copy of the image
> detection copy in libvirt and replace it by invocation of qemu-img. That
> way we could avoid any discrepancies in the detection process in the
> first place.

Now there's an interesting thought.  Since data corruption occurs when 
there is disagreement about which mode to use, getting libvirt out of 
the probing business by deferring all decisions to qemu-img info is a 
smart move - if qemu says an image probes as qcow2 (in an environment 
where probing is safe), then libvirt passing an explicit qcow2 to qemu 
for guest usage (in an environment where probing is not safe) will at 
least see the same guest-visible data.  Less code to maintain in 
libvirt, and no chance for a mismatch between the two projects on which 
format a probe should result in.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org