[libvirt] [PATCH 2/2] check IOMMU group devices usage when preparing device for vfio passthrough

Michal Privoznik mprivozn at redhat.com
Mon Jan 12 09:13:32 UTC 2015


On 04.12.2014 08:28, Shivaprasad G Bhat wrote:
> The virsh start <domain> fails with qemu error when the hostdevices of the
> same iommu group are used actively by other vms. It is not clear which
> hostdev from the same iommu group is used by any of the running guests.
> User has to go through every guest xml to figure out who is using the
> hostdev of same iommu group.
>
> Solution:
> Iterate the iommu group of the hostdev and error our neatly in case a
> device in the same iommu group is busy. Reattach code also does the same
> kind of check, remove duplicate code as well.
>
> Signed-off-by: Shivaprasad G Bhat <sbhat at linux.vnet.ibm.com>
> ---
>   src/util/virhostdev.c |   87 +++++++++++++++++++++++++++++--------------------
>   1 file changed, 52 insertions(+), 35 deletions(-)
>
> diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
> index 18ff96b..da40030 100644
> --- a/src/util/virhostdev.c
> +++ b/src/util/virhostdev.c
> @@ -54,6 +54,42 @@ static virClassPtr virHostdevManagerClass;
>   static void virHostdevManagerDispose(void *obj);
>   static virHostdevManagerPtr virHostdevManagerNew(void);
>
> +static int virHostdevIsPCINodeDeviceUsed(virPCIDeviceAddressPtr devAddr, void *opaque)
> +{
> +    virPCIDevicePtr other;
> +    int ret = -1;
> +    virPCIDevicePtr pci;
> +    virHostdevManagerPtr hostdev_mgr = opaque;
> +
> +    if (!(pci = virPCIDeviceNew(devAddr->domain, devAddr->bus,
> +                                   devAddr->slot, devAddr->function)))

Indentation's off here and in other places too. But that's not a show 
stopper.

> +        goto cleanup;
> +
> +    other = virPCIDeviceListFind(hostdev_mgr->activePCIHostdevs, pci);
> +    if (other) {
> +        const char *other_drvname = NULL;
> +        const char *other_domname = NULL;
> +        virPCIDeviceGetUsedBy(other, &other_drvname, &other_domname);
> +
> +        if (other_drvname && other_domname)
> +            virReportError(VIR_ERR_OPERATION_INVALID,
> +                           _("PCI device %s is in use by "
> +                             "driver %s, domain %s"),
> +                           virPCIDeviceGetName(pci),
> +                           other_drvname, other_domname);
> +        else
> +            virReportError(VIR_ERR_OPERATION_INVALID,
> +                           _("PCI device %s is in use"),
> +                           virPCIDeviceGetName(pci));
> +        ret = -1;
> +        goto cleanup;
> +    }
> +    ret = 0;
> + cleanup:
> +    virPCIDeviceFree(pci);
> +    return ret;
> +}
> +
>   static int virHostdevManagerOnceInit(void)
>   {
>       if (!(virHostdevManagerClass = virClassNew(virClassForObject(),
> @@ -517,8 +553,10 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr hostdev_mgr,
>        */
>
>       for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
> +        void *opaque = hostdev_mgr;

This is unneeded variable. Just drop it and use hostdev_mgr instead.

>           virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i);
> -        virPCIDevicePtr other;
> +        virPCIDeviceAddress devAddr = { dev->domain, dev->bus,
> +                                        dev->slot, dev->function };
>           bool strict_acs_check = !!(flags & VIR_HOSTDEV_STRICT_ACS_CHECK);
>
>           if (!virPCIDeviceIsAssignable(dev, strict_acs_check)) {
> @@ -528,24 +566,17 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr hostdev_mgr,
>               goto cleanup;
>           }
>           /* The device is in use by other active domain if
> -         * the dev is in list activePCIHostdevs.
> +         * the dev is in list activePCIHostdevs. VFIO devices
> +         * belonging to same iommu group cant be shared
> +         * across guests.
>            */
> -        if ((other = virPCIDeviceListFind(hostdev_mgr->activePCIHostdevs, dev))) {
> -            const char *other_drvname;
> -            const char *other_domname;
> -
> -            virPCIDeviceGetUsedBy(other, &other_drvname, &other_domname);
> -            if (other_drvname && other_domname)
> -                virReportError(VIR_ERR_OPERATION_INVALID,
> -                               _("PCI device %s is in use by "
> -                                 "driver %s, domain %s"),
> -                               virPCIDeviceGetName(dev),
> -                               other_drvname, other_domname);
> -            else
> -                virReportError(VIR_ERR_OPERATION_INVALID,
> -                               _("PCI device %s is already in use"),
> -                               virPCIDeviceGetName(dev));
> -            goto cleanup;
> +        if (STREQ(virPCIDeviceGetStubDriver(dev), "vfio-pci")) {
> +           if (virPCIDeviceAddressIOMMUGroupIterate(&devAddr,
> +                                             virHostdevIsPCINodeDeviceUsed,
> +                                             opaque) < 0)
> +           goto cleanup;
> +        } else if (virHostdevIsPCINodeDeviceUsed(&devAddr, opaque)) {
> +           goto cleanup;
>           }
>       }
>
> @@ -1506,29 +1537,15 @@ int
>   virHostdevPCINodeDeviceReAttach(virHostdevManagerPtr hostdev_mgr,
>                                   virPCIDevicePtr pci)
>   {
> -    virPCIDevicePtr other;
> +    virPCIDeviceAddress devAddr = { pci->domain, pci->bus, pci->slot, pci->function };
>       int ret = -1;
> +    void *opaque = hostdev_mgr;
>
>       virObjectLock(hostdev_mgr->activePCIHostdevs);
>       virObjectLock(hostdev_mgr->inactivePCIHostdevs);
> -    other = virPCIDeviceListFind(hostdev_mgr->activePCIHostdevs, pci);
> -    if (other) {
> -        const char *other_drvname = NULL;
> -        const char *other_domname = NULL;
> -        virPCIDeviceGetUsedBy(other, &other_drvname, &other_domname);
>
> -        if (other_drvname && other_domname)
> -            virReportError(VIR_ERR_OPERATION_INVALID,
> -                           _("PCI device %s is still in use by "
> -                             "driver %s, domain %s"),
> -                           virPCIDeviceGetName(pci),
> -                           other_drvname, other_domname);
> -        else
> -            virReportError(VIR_ERR_OPERATION_INVALID,
> -                           _("PCI device %s is still in use"),
> -                           virPCIDeviceGetName(pci));
> +    if (virHostdevIsPCINodeDeviceUsed(&devAddr, opaque))
>           goto out;
> -    }
>
>       virPCIDeviceReattachInit(pci);
>

Okay, I like this patch. it's a weak ACK, but I can't push it without 
1/2 which implies slight rework of this patch too. Looking forward to v2.

Michal




More information about the libvir-list mailing list