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@xxxxxxxxxxxxxxxxxx>
---
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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list