[libvirt] backing file chains, -blockdev, and storage file autodetection

[Daniel P. Berrangé]

On Thu, Jan 09, 2020 at 02:47:22PM +0000, Martin Wilck wrote:
> On Thu, 2020-01-09 at 14:34 +0000, Daniel P. Berrangé wrote:
> > 
> > IIUC there are three scenarios at play here
> > 
> >  1. qcow2 file, first level raw backing
> >  2. qcow2 file, first level qcow2 backing, no  backing
> >  3. qcow2 file, first level qcow2 backing, with second level backing
> > 
> > Historically with the SELinux driver, if no backing_fmt is set,
> > then we blindly assume that scenario (1) is in effect.
> > 
> > We cannot tell QEMU that we treated it as scenario (1) though,
> > so QEMU will still probe format and potentially open the
> > first level backing as qcow2 still.
> > 
> > IOW, despite our SELinux & QEMU driver assumption for scenario (1),
> > scenario (2) would still suceed in the past.  Scenario (3) would
> > always have failed, because SELinux won't have labelled the second
> > level backing.
> 
> But it only failed with SELinux, right? Not every distro is mandating
> SELinux usage with libvirt. SUSE distros don't, for example.

No, that's just one example. It should also fail with AppArmor,
and should also fail with system libvirtd, since libvirt DAC
driver chowns files from root -> qemu user ID.

Only case where all three scenarios would work are if using
the session libvirtd as an unprivileged user, or if using
systemd libvirtd with QEMU configured to run as root (a
really bad idea).

> 
> > Essentially scenario (2) worked by accident, not design, but
> > IIUC we have not been warning users about this.
> > 
> > With new libvirt and -blockdev we still assume the backing_fmt
> > is raw if not set in the SELinux driver, but we now have the
> > ability to tell QEMU about our assumption via -blockdev. As
> > such we've not ensured scenario (2) fails.
> > 
> > 
> > Users who were silently relying on scenario (2) are now broken
> > and have three options IIUC
> > 
> >  - Run qemu-img rebase to set the backing_fmt
> > 
> > or
> > 
> >  - Update the guest XML to set the <backingStore> format value
> > 
> > or
> > 
> >  - Update the /etc/libvirt/qemu.conf to set capability_filters
> >    to disable "blockdev"
> 
> I wasn't aware of this option, thanks. I'd actually looked for
> a way to revert libvirt's behavior to what it did in previous versions.

Note, that we call that a hack. There's no guarantee it will
continue working correctly long term & there's no testing of
it in general.

So at most it should be used as a short term solution until you
can use qemu-img rebase or update the XML backingStore for all
affected VM.

> 
> > Always assuming the format is raw certainly avoids the security
> > danger, but is unhelpful to users who relied on scenario (2).
> > 
> > I'm inclined to say we should make scenario (2)/(3) into a hard
> > error. Only allow scenario (1) to run normally.
> > 
> > ie, we should probe the disk format for the backing file, and
> > if it is *not* raw, then report an immediate error, with the
> > error message pointing to the kbase page explaining how to
> > fix this. This will help the 99% common case identify the
> > fix more quickly, with no obvious downside that I see.
> 
> Sounds good. I'd appreciate mention of the capability_filters option on
> the kbase page. 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|