[libvirt] [PATCH 2/5] Log taint warnings in QEMU driver

[Daniel P. Berrange]

On Thu, May 05, 2011 at 02:55:14PM -0600, Eric Blake wrote:
> On 05/05/2011 05:51 AM, Daniel P. Berrange wrote:
> > Wire up logging of VM taintint to the QEMU driver
> 
> s/taintint/tainting/
> 
> > 
> >  - If running QEMU as root user/group or without capabilities
> >    being cleared
> >  - If passing custom QEMU command line args
> >  - If issuing custom QEMU monitor commands
> >  - If using a network interface config with an associated
> >    shell script
> >  - If using a disk config relying on format probing
> > 
> > The warnings, per-VM appear in the main libvirtd logs
> > 
> >   11:56:17.571: 10832: warning : qemuDomainObjTaint:712 : Domain id=1 name='l2' uuid=c7a3edbd-edaf-9455-926a-d65c16db1802 is tainted: high-privileges
> >   11:56:17.571: 10832: warning : qemuDomainObjTaint:712 : Domain id=1 name='l2' uuid=c7a3edbd-edaf-9455-926a-d65c16db1802 is tainted: disk-probing
> > 
> > The taint flags are reset when the VM is stopped.
> > 
> > * src/qemu/qemu_domain.c, src/qemu/qemu_domain.h: Helper APIs
> >   for logging taint warnings
> > * src/qemu/qemu_driver.c: Log tainting with custom QEMU monitor
> >   commands and disk/net hotplug with unsupported configs
> > * src/qemu/qemu_process.c: Log tainting at startup based on
> >   unsupported configs
> 
> > +++ b/src/qemu/qemu_driver.c
> > @@ -3877,6 +3877,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
> >  
> >      switch (dev->type) {
> >      case VIR_DOMAIN_DEVICE_DISK:
> > +        qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk);
> >          ret = qemuDomainAttachDeviceDiskLive(driver, vm, dev);
> 
> Do we really want to issue the taint message before attempting the
> attach?  Or are we still safe if you perform the attach first, and issue
> the taint message only on success?

Even if the attach operation gets half-way through and then fails
I think we still want to have the taint message, because even the
partial attempt may have had negative consequences. Taking that to
its logical conclusion, we should always warn on any attempt to
use a undesirable configuration, and thus might as well do it
upfront, in case we crash later.

> 
> >          if (!ret)
> >              dev->data.disk = NULL;
> > @@ -3889,6 +3890,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
> >          break;
> >  
> >      case VIR_DOMAIN_DEVICE_NET:
> > +        qemuDomainObjCheckNetTaint(driver, vm, dev->data.net);
> >          ret = qemuDomainAttachNetDevice(dom->conn, driver, vm,
> 
> Likewise.
> 
> On second thought - we can't guarantee if failure happened before a disk
> was probed, or after, so I guess doing unconditional taint is the
> conservatively safe approach, and the whole point of this is
> conservative safety even if it lets through some false positives.


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|