Hi Laine, Did you have a chance to look at V2 of this patch? As discussed in V1, I left the existing code untouched and added new functions for the driver_override interface. Thanks! Regards, Jim Jim Fehlig wrote: > libvirt uses the new_id PCI sysfs interface to bind a PCI stub driver > to a PCI device. The new_id interface is known to be buggy and racey, > hence a more deterministic interface was introduced in the 3.12 kernel: > driver_override. For more details see > > https://www.redhat.com/archives/libvir-list/2016-June/msg02124.html > > This patch adds support for the driver_override interface by > > - adding new virPCIDevice{BindTo,UnbindFrom}StubWithOverride functions > that use the driver_override interface > - renames the existing virPCIDevice{BindTo,UnbindFrom}Stub functions > to virPCIDevice{BindTo,UnbindFrom}StubWithNewid to perserve existing > behavior on new_id interface > - changes virPCIDevice{BindTo,UnbindFrom}Stub function to call one of > the above depending on availability of driver_override > > The patch includes a bit of duplicate code, but allows for easily > dropping the new_id code once support for older kernels is no > longer desired. > > Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx> > --- > > V1: > > https://www.redhat.com/archives/libvir-list/2016-July/msg00370.html > > Changes since V1: > - drop patch1 > - change patch2 to preserve the existing new_id code and add new > functions to implement the driver_override interface > > src/util/virpci.c | 151 +++++++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 149 insertions(+), 2 deletions(-) > > diff --git a/src/util/virpci.c b/src/util/virpci.c > index 132948d..6c8174a 100644 > --- a/src/util/virpci.c > +++ b/src/util/virpci.c > @@ -1089,8 +1089,54 @@ virPCIDeviceUnbind(virPCIDevicePtr dev) > return ret; > } > > +/* > + * Bind a PCI device to a driver using driver_override sysfs interface. > + * E.g. > + * > + * echo driver-name > /sys/bus/pci/devices/0000:03:00.0/driver_override > + * echo 0000:03:00.0 > /sys/bus/pci/devices/0000:03:00.0/driver/unbind > + * echo 0000:03:00.0 > /sys/bus/pci/drivers_probe > + * > + * An empty driverName will cause the device to be bound to its > + * preferred driver. > + */ > static int > -virPCIDeviceUnbindFromStub(virPCIDevicePtr dev) > +virPCIDeviceBindWithDriverOverride(virPCIDevicePtr dev, > + const char *driverName) > +{ > + int ret = -1; > + char *path; > + > + if (!(path = virPCIFile(dev->name, "driver_override"))) > + return -1; > + > + if (virFileWriteStr(path, driverName, 0) < 0) { > + virReportSystemError(errno, > + _("Failed to add driver '%s' to driver_override " > + " interface of PCI device '%s'"), > + driverName, dev->name); > + goto cleanup; > + } > + > + if (virPCIDeviceUnbind(dev) < 0) > + goto cleanup; > + > + if (virFileWriteStr(PCI_SYSFS "drivers_probe", dev->name, 0) < 0) { > + virReportSystemError(errno, > + _("Failed to trigger a probe for PCI device '%s'"), > + dev->name); > + goto cleanup; > + } > + > + ret = 0; > + > + cleanup: > + VIR_FREE(path); > + return ret; > +} > + > +static int > +virPCIDeviceUnbindFromStubWithNewid(virPCIDevicePtr dev) > { > int result = -1; > char *drvdir = NULL; > @@ -1191,9 +1237,41 @@ virPCIDeviceUnbindFromStub(virPCIDevicePtr dev) > return result; > } > > +static int > +virPCIDeviceUnbindFromStubWithOverride(virPCIDevicePtr dev) > +{ > + if (!dev->unbind_from_stub) { > + VIR_DEBUG("Unbind from stub skipped for PCI device %s", dev->name); > + return 0; > + } > + > + return virPCIDeviceBindWithDriverOverride(dev, "\n"); > +} > + > +static int > +virPCIDeviceUnbindFromStub(virPCIDevicePtr dev) > +{ > + int ret; > + char *path; > + > + /* > + * Prefer using the device's driver_override interface, falling back > + * to the unpleasant new_id interface. > + */ > + if (!(path = virPCIFile(dev->name, "driver_override"))) > + return -1; > + > + if (virFileExists(path)) > + ret = virPCIDeviceUnbindFromStubWithOverride(dev); > + else > + ret = virPCIDeviceUnbindFromStubWithNewid(dev); > + > + VIR_FREE(path); > + return ret; > +} > > static int > -virPCIDeviceBindToStub(virPCIDevicePtr dev) > +virPCIDeviceBindToStubWithNewid(virPCIDevicePtr dev) > { > int result = -1; > bool reprobe = false; > @@ -1345,6 +1423,75 @@ virPCIDeviceBindToStub(virPCIDevicePtr dev) > return result; > } > > +static int > +virPCIDeviceBindToStubWithOverride(virPCIDevicePtr dev) > +{ > + int ret = -1; > + const char *stubDriverName; > + char *stubDriverPath = NULL; > + char *driverLink = NULL; > + > + /* Check the device is configured to use one of the known stub drivers */ > + if (dev->stubDriver == VIR_PCI_STUB_DRIVER_NONE) { > + virReportError(VIR_ERR_INTERNAL_ERROR, > + _("No stub driver configured for PCI device %s"), > + dev->name); > + return -1; > + } else if (!(stubDriverName = virPCIStubDriverTypeToString(dev->stubDriver))) { > + virReportError(VIR_ERR_INTERNAL_ERROR, > + _("Unknown stub driver configured for PCI device %s"), > + dev->name); > + return -1; > + } > + > + if (!(stubDriverPath = virPCIDriverDir(stubDriverName)) || > + !(driverLink = virPCIFile(dev->name, "driver"))) > + goto cleanup; > + > + if (virFileExists(driverLink)) { > + if (virFileLinkPointsTo(driverLink, stubDriverPath)) { > + /* The device is already bound to the correct driver */ > + VIR_DEBUG("Device %s is already bound to %s", > + dev->name, stubDriverName); > + ret = 0; > + goto cleanup; > + } > + } > + > + if (virPCIDeviceBindWithDriverOverride(dev, stubDriverName) < 0) > + goto cleanup; > + > + dev->unbind_from_stub = true; > + ret = 0; > + > + cleanup: > + VIR_FREE(stubDriverPath); > + VIR_FREE(driverLink); > + return ret; > +} > + > +static int > +virPCIDeviceBindToStub(virPCIDevicePtr dev) > +{ > + int ret; > + char *path; > + > + /* > + * Prefer using the device's driver_override interface, falling back > + * to the unpleasant new_id interface. > + */ > + if (!(path = virPCIFile(dev->name, "driver_override"))) > + return -1; > + > + if (virFileExists(path)) > + ret = virPCIDeviceBindToStubWithOverride(dev); > + else > + ret = virPCIDeviceBindToStubWithNewid(dev); > + > + VIR_FREE(path); > + return ret; > +} > + > /* virPCIDeviceDetach: > * > * Detach this device from the host driver, attach it to the stub -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list