Re: [PATCH V2] virpci: support driver_override sysfs interface

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]