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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



[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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux