Re: [PATCH 8/9] virStorageFileGetMetadataRecurse: Allow format probing under special circumstances
On Wed, Feb 19, 2020 at 11:07:09 -0600, Eric Blake wrote:
On 2/19/20 10:40 AM, 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.
> > > 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.