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 at redhat.com>
> ---
> 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.
Michal