[libvirt] [PATCH 1/9] pci: Make reattach work for unbound devices

[Michal Privoznik]

On 17.01.2014 11:39, Jiri Denemark wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1046919
> 
> When a PCI device is not bound to any driver, reattach should just
> trigger driver probe rather than failing with
> 
>     Invalid device 0000:00:19.0 driver file
>     /sys/bus/pci/devices/0000:00:19.0/driver is not a symlink
> 
> While virPCIDeviceGetDriverPathAndName was documented to return success
> and NULL driver and path when a device is not attached to any driver but
> didn't do so. Thus callers could not distinguish unbound devices from
> failures.
> 
> Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
> ---
>  src/util/virpci.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/src/util/virpci.c b/src/util/virpci.c
> index 51dbee6..8577fd4 100644
> --- a/src/util/virpci.c
> +++ b/src/util/virpci.c
> @@ -236,6 +236,11 @@ virPCIDeviceGetDriverPathAndName(virPCIDevicePtr dev, char **path, char **name)
>      if (virPCIFile(&drvlink, dev->name, "driver") < 0)
>          goto cleanup;
>  
> +    if (!virFileExists(drvlink)) {
> +        ret = 0;
> +        goto cleanup;
> +    }
> +
>      if (virFileIsLink(drvlink) != 1) {
>          virReportError(VIR_ERR_INTERNAL_ERROR,
>                         _("Invalid device %s driver file %s is not a symlink"),
> @@ -1023,6 +1028,11 @@ virPCIDeviceUnbindFromStub(virPCIDevicePtr dev)
>      if (virPCIDeviceGetDriverPathAndName(dev, &drvdir, &driver) < 0)
>          goto cleanup;
>  
> +    if (!driver) {
> +        /* The device is not bound to any driver and we are almost done. */
> +        goto reprobe;
> +    }
> +
>      if (!dev->unbind_from_stub)
>          goto remove_slot;
>  
> @@ -1079,11 +1089,10 @@ reprobe:
>       * available, then re-probing would just cause the device to be
>       * re-bound to the stub.
>       */
> -    if (virPCIDriverFile(&path, driver, "remove_id") < 0) {
> +    if (driver && virPCIDriverFile(&path, driver, "remove_id") < 0)
>          goto cleanup;
> -    }
>  

Up to here I understand the patch.

> -    if (!virFileExists(drvdir) || virFileExists(path)) {
> +    if (!driver || !virFileExists(drvdir) || virFileExists(path)) {
>          if (virFileWriteStr(PCI_SYSFS "drivers_probe", dev->name, 0) < 0) {
>              virReportSystemError(errno,
>                                   _("Failed to trigger a re-probe for PCI device '%s'"),
> 

But this bit doesn't make much sense to me. If a device is bound to a
stub driver, say pci-stub, then the
@path="/sys/bus/pci/drivers/pci-stub/remove_id"; I don't see a reason
how/why this should have any affect on decision whether to write into
"/sys/bus/pci/drivers_probe" or not.

Even though it's pre-existing, now that you're touching the line it
makes sense to do it right.

Michal