[libvirt] [PATCH V2] virpci: support driver_override sysfs interface
Jim Fehlig
jfehlig at suse.com
Mon Aug 29 21:20:27 UTC 2016
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 at suse.com>
> ---
>
> 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
More information about the libvir-list
mailing list