[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