[libvirt] virNodeDevice APIs and VFIO

[Daniel P. Berrange]

On Wed, Apr 24, 2013 at 09:56:34AM -0400, Laine Stump wrote:
> On 04/23/2013 12:34 PM, Laine Stump wrote:
> > On 04/23/2013 11:46 AM, Daniel P. Berrange wrote:
> >> On Tue, Apr 23, 2013 at 10:52:09AM -0400, Laine Stump wrote:
> >>> Yesterday for the first time I consciously noticed the
> >>> virNodeDeviceDettach and virNodeDeviceReAttach APIs, and found that they
> >>> are hardcoded to bind to/unbind from the pci-stub driver for qemu, and
> >>> the "pciback" driver for Xen. If we want these APIs to be useful for
> >>> VFIO, they will need to bind to the vfio driver instead, but there is
> >>> currently no method in those APIs to specify which driver to bind to.
> >>>
> >>> I guess we could do this with new virNodeDeviceDetachFlags() and
> >>> virNodeDeviceReAttachFlags() APIs which have a flag to indicate vfio,
> >>> but before going to that trouble I'd like to know if these APIs are
> >>> actually used or if they are deprecated (they don't seem to be of any
> >>> use if the hostdev devices you're assigning have "managed='yes'" - as
> >>> far as I can see, setting managed='yes' just makes the bind/unbind from
> >>> the stub driver an automatic part of assigning/un-assigning the device
> >>> to a guest).
> >> The rationale for managed="no", is that in more security paranoid/locked
> >> down environments, admins will not want libvirtd screwing around with
> >> kernel module drivers. The admin would manually setup "pciback" binding
> >> ahead of time when configuring the host. I think we need to continue to
> >> support this scenario in the VFIO world.
> >
> > Okay. But then in that case, since they don't want libvirtd screwing
> > around with kernel module drivers when starting a domain, they also
> > wouldn't want libvirtd screwing around with kernel module drivers at any
> > other time either (i.e. via virNodeDeviceDetach(), right?
> >
> >
> > (I suppose once we have more fine-grained authorization of the libvirt
> > API, it might be possible for one user to call virNodeDeviceDetach() and
> > another could only start a domain with <hostdev managed='no'>, but is
> > the "fine grainedness" really going to be down to the level of which
> > attributes are allowed in certain domain XML elements?)
> >
> >
> >> If we go down the route of extending <hostdev> to have support for
> >>
> >>     <driver name="vfio|qemu"/>
> >
> > Hehe. That's *almost" exactly the patchset I'm working on (I was
> > thinking of sending out just the config-change patch now to get it
> > reviewed early and avoid the last minute rush before the freeze).
> > However, since the legacy backend in this case is handled by the kernel
> > KVM module, I had named them "vfio" and "kvm" (I originally thought to
> > use "qemu", but then considered it might confuse people into thinking
> > that it was something done by qemu in usermode).
> >
> >
> >> Then, this says that the virNodeDevice{Detach,ReAttach} methods need
> >> to have new variants which take a 'const char *drivername' parameter.
> >>
> >> NB, the values of 'drivername' would match those used in the domain
> >> XML <driver> element - they would *not* be kernel module names.
> >>
> >> So we'd call
> >>
> >>    virNodeDeviceDetachDriver(dev, "qemu");
> >>    virNodeDeviceDetachDriver(dev, "vfio");
> > (of course with a "flags" argument at the end :-)
> >
> > That's a bit confusing, since "detach" in this case really means "detach
> > it from whatever driver it's currently bound to, and bind it to the
> > 'vfio' driver". But just from reading the function name and args, it
> > *seems* to mean "detach this device from the 'vfio' driver". On the
> > other hand, I don't see a better alternative.
> >
> > And then what about the converse - virNodeDeviceAttachDriver(dev,
> > "vfio") - that really means "detach it from vfio, and attach it to....
> > well actually *nowhere*! (that's what virNodeDeviceReAttach does - it's
> > not necessarily the inverse of virNodeDeviceDettach)
> >
> > Maybe we need a totally generic virNodeDeviceAttach() that allows us to
> > specify a stub driver name, a "real" driver name, or NULL (in which case
> > it would just be unbound from the current driver)? But there again you
> > get back to the problem of specifying the actual name of a driver vs. an
> > abstracted name like "kvm" or "vfio" (the actual device to bind to for
> > vfio is called "vfio-pci").
> 
> How about this:
> 
>   int virNodeDeviceDetachFlags(virNodeDevicePtr dev,
>                                const char *stubDriver,
>                                unsigned int flags);
> 
> stubDriver will be exactly equal to whatever is in the <driver
> name='x'/>, and can also be NULL, in which case the "hypervisor default"
> will be used.

'stubDriver' implies that you're passing the kernel module name,
which this is not. It should be called 'driverName' to reflect
the XML attribute name.

> In the meantime, the virPCIDevice object (used internally for the list
> of active and inactice PCI devices) will be extended to keep track of
> both which stub driver is used, as well as the original driver (if any),
> so that we can then have:
> 
> 
>   int virNodeDeviceReAttachFlags(virNodeDEvicePtr dev,
>                                  unsigned int flags);
> 
> (it won't need a driver/stubDriver arg, since that will be recorded when
> it's detached.)
> 
> OR - we could make it more general-purpose by adding a const char
> *driver which can be NULL (in which case it's just restored to its
> original state), but also allows naming a specific driver.

Nah, we don't need to know the driver name here. In fact we don't
really need a new API at all, unless we have some need for the
flags parameter.


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 :|