[libvirt] [PATCH 21/24] hostdev: Stop early if unmanaged devices have not been detached

[John Ferlan]

On 03/07/2016 12:24 PM, Andrea Bolognani wrote:
> Unmanaged devices, as the name suggests, are not detached
> automatically from the host by libvirt before being attached to a
> guest: it's the user's responsability to detach them manually
> beforehand. If that preliminary step has not been performed, the
> attach operation can't complete successfully.
> 
> Instead of relying on the lower layers to error out with cryptic
> messages such as
> 
>   error: Failed to attach device from /tmp/hostdev.xml
>   error: Path '/dev/vfio/12' is not accessible: No such file or directory
> 
> prevent the situation altogether and provide the user with a more
> useful error message.
> ---
>  src/util/virhostdev.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
> index 03c3445..d1529c5 100644
> --- a/src/util/virhostdev.c
> +++ b/src/util/virhostdev.c
> @@ -576,6 +576,13 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr,
>                                     mgr->inactivePCIHostdevs) < 0)


Is this in the right place?

Consider a few lines above:

    /* Step 1: validate that non-managed device isn't in use, ...

Second question - how would the device get on the inactiveList
initially? Looking at virPCIDeviceListAdd calls for inactiveList.

'pcidevs' is the list of all devices both managed and unmanaged

'activeList' is populated in step 5 and virHostdevUpdateActivePCIDevices

'inactiveList' is populated in 'inactivedevs:' label, in step 2 of
virHostdevReAttachPCIDevices, and in virPCIDeviceDetach via
virPCIDeviceListAddCopy *if* the device is managed in step 2 of Prepare.


I don't disagree this is an important step, but it's the "how" we
determine this that I'm questioning.

John


>                  goto reattachdevs;
>          } else {
> +            if (!virPCIDeviceListFind(mgr->inactivePCIHostdevs, pci)) {
> +                virReportError(VIR_ERR_OPERATION_INVALID,
> +                               _("Unmanaged PCI device %s must be manually "
> +                               "detached from the host"),
> +                               virPCIDeviceGetName(pci));
> +                goto reattachdevs;
> +            }
>              VIR_DEBUG("Not detaching unmanaged PCI device %s",
>                        virPCIDeviceGetName(pci));
>          }
>