[libvirt] [PATCH v2 4/5] virpci: Don't error on not binded devices

[Laine Stump]

On 10/31/2013 01:23 PM, Michal Privoznik wrote:
> If a PCI deivce is not binded to any driver (e.g. there's yet no PCI
> driver in the linux kernel) but still users want to passthru the device
> we fail the whole operation as we fail to resolve the 'driver' link
> under the PCI device sysfs tree. Obviously, this is not a fatal error
> and it shouldn't be error at all.
>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/util/virpci.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/src/util/virpci.c b/src/util/virpci.c
> index 65d7168..0727085 100644
> --- a/src/util/virpci.c
> +++ b/src/util/virpci.c
> @@ -1095,10 +1095,16 @@ virPCIDeviceBindToStub(virPCIDevicePtr dev,
>      const char *newDriverName = NULL;
>  
>      if (virPCIDriverDir(&stubDriverPath, stubDriverName) < 0 ||
> -        virPCIFile(&driverLink, dev->name, "driver") < 0 ||
> -        virPCIDeviceGetDriverPathAndName(dev, &oldDriverPath,
> -                                         &oldDriverName) < 0) {
> +        virPCIFile(&driverLink, dev->name, "driver") < 0)
>          goto cleanup;
> +
> +    if (virPCIDeviceGetDriverPathAndName(dev, &oldDriverPath,
> +                                         &oldDriverName) < 0) {
> +        /* It's okay if device is not binded to any driver. If that's the case,
> +         * there's no /sys/bus/pci/devices/.../driver symlink. */
> +        if (errno != ENOENT)
> +            goto cleanup;
> +        virResetLastError();

The problem with fixing it like this is that the log will already have
an error message, and we'll end up getting a bug report about it even
though the operation is successful.

I think this should instead be fixed by having
virPCIDeviceDriverPathAndName() return success with NULL path and name
in the case that the device isn't bound to anything, then check for that
specific condition in all the callers of
virPCIDeviceDriverPathAndName(), and change the behavior accordingly.

In virPCIDeviceBindToStub and virPCIDeviceReset, no change would be
needed. In virPCIDeviceUnbindFromStub, I'm not sure what would be the
appropriate action - doing a "drivers_reprobe" might be correct, but
depending on what led to the current state, that might end up re-binding
the stub driver; it almost seems that what's needed in that case is to
do the reprobe, then do the entire function a 2nd time in case the first
try caused a re-bind to the stub (and then generate an error if it has
failed after the 2nd time).

>      }
>  
>      if (virFileExists(driverLink)) {