Re: [libvirt PATCH v3 7/8] node_device: support binding other drivers with virNodeDeviceDetachFlags()

On 8/21/23 21:32, Laine Stump wrote: > In the past, the only allowable values for the "driver" field of > virNodeDeviceDetachFlags() were "kvm" or "vfio" for the QEMU driver, > and "xen" for the libxl driver. Then "kvm" was deprecated and removed, > so the driver name became essentially irrelevant (because it is always > called via a particular hypervisor driver, and so the "xen" or "vfio" > can be (and almost always is) implied. > > With the advent of VFIO variant drivers, the ability to explicitly > specify a driver name once again becomes useful - it can be used to > name the exact VFIO driver that we want bound to the device in place > of vfio-pci, so this patch allows those other names to be passed down > the call chain, where the code in virpci.c can make use of them. > > The names "vfio", "kvm", and "xen" retain their special meaning, though: > > 1) because there may be some application or configuration that still > calls virNodeDeviceDetachFlags() with driverName="vfio", this > single value is substituted with the synonym of NULL, which means > "bind the default driver for this device and hypervisor". This > will currently result in the vfio-pci driver being bound to the > device. > > 2) in the case of the libxl driver, "xen" means to use the standard > driver used in the case of Xen ("pciback"). > > 3) "kvm" as a driver name always results in an error, as legacy KVM > device assignment was removed from the kernel around 10 years ago. > > Signed-off-by: Laine Stump <laine(a)redhat.com&gt; > --- > src/hypervisor/domain_driver.c | 9 ++++----- > src/hypervisor/domain_driver.h | 2 ++ > src/libxl/libxl_driver.c | 3 ++- > src/qemu/qemu_driver.c | 33 +++++++++++++++++++-------------- > 4 files changed, 27 insertions(+), 20 deletions(-) > > diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c > index a70f75f3ae..429784292a 100644 > --- a/src/hypervisor/domain_driver.c > +++ b/src/hypervisor/domain_driver.c > @@ -462,6 +462,7 @@ virDomainDriverNodeDeviceReAttach(virNodeDevicePtr dev, > int > virDomainDriverNodeDeviceDetachFlags(virNodeDevicePtr dev, > virHostdevManager *hostdevMgr, > + virPCIStubDriver driverType, > const char *driverName) > { > g_autoptr(virPCIDevice) pci = NULL; > @@ -471,7 +472,7 @@ virDomainDriverNodeDeviceDetachFlags(virNodeDevicePtr dev, > g_autoptr(virConnect) nodeconn = NULL; > g_autoptr(virNodeDevice) nodedev = NULL; > > - if (!driverName) > + if (driverType == VIR_PCI_STUB_DRIVER_NONE) > return -1; Even though, we are very unlikely to hit this check, the rest of the function reports an error on error paths. Please virReportError() here too.