qemuDomainGetHostdevPath: Report /dev/vfio/vfio less frequently

So far, qemuDomainGetHostdevPath has no knowledge of the reasong
it is called and thus reports /dev/vfio/vfio for every VFIO
backed device. This is suboptimal, as we want it to:

a) report /dev/vfio/vfio on every addition or domain startup
b) report /dev/vfio/vfio only on last VFIO device being unplugged

If a domain is being stopped then namespace and CGroup die with
it so no need to worry about that. I mean, even when a domain
that's exiting has more than one VFIO devices assigned to it,
this function does not clean /dev/vfio/vfio in CGroup nor in the
namespace. But that doesn't matter.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NMarc-André Lureau <marcandre.lureau@redhat.com>

src/qemu/qemu_cgroup.c

@@ -52,7 +52,6 @@ const char *const defaultDeviceACL[] = {
 #define DEVICE_PTY_MAJOR 136
 #define DEVICE_SND_MAJOR 116
 
-#define DEV_VFIO "/dev/vfio/vfio"
 
 static int
 qemuSetupImagePathCgroup(virDomainObjPtr vm,
@@ -271,7 +270,7 @@ qemuSetupHostdevCgroup(virDomainObjPtr vm,
     size_t i, npaths = 0;
     int rv, ret = -1;
 
-    if (qemuDomainGetHostdevPath(dev, &npaths, &path, &perms) < 0)
+    if (qemuDomainGetHostdevPath(NULL, dev, false, &npaths, &path, &perms) < 0)
         goto cleanup;
 
     for (i = 0; i < npaths; i++) {
@@ -298,11 +297,10 @@ int
 qemuTeardownHostdevCgroup(virDomainObjPtr vm,
                        virDomainHostdevDefPtr dev)
 {
-    int ret = -1;
     qemuDomainObjPrivatePtr priv = vm->privateData;
-    virDomainHostdevSubsysPCIPtr pcisrc = &dev->source.subsys.u.pci;
-    virPCIDevicePtr pci = NULL;
-    char *path = NULL;
+    char **path = NULL;
+    size_t i, npaths = 0;
+    int rv, ret = -1;
 
     /* currently this only does something for PCI devices using vfio
      * for device assignment, but it is called for *all* hostdev
@@ -312,70 +310,27 @@ qemuTeardownHostdevCgroup(virDomainObjPtr vm,
     if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES))
         return 0;
 
-    if (dev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
-
-        switch ((virDomainHostdevSubsysType) dev->source.subsys.type) {
-        case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
-            if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) {
-                int rv;
-                size_t i, vfios = 0;
-
-                pci = virPCIDeviceNew(pcisrc->addr.domain,
-                                      pcisrc->addr.bus,
-                                      pcisrc->addr.slot,
-                                      pcisrc->addr.function);
-                if (!pci)
-                    goto cleanup;
-
-                if (!(path = virPCIDeviceGetIOMMUGroupDev(pci)))
-                    goto cleanup;
-
-                VIR_DEBUG("Cgroup deny %s for PCI device assignment", path);
-                rv = virCgroupDenyDevicePath(priv->cgroup, path,
-                                             VIR_CGROUP_DEVICE_RWM, false);
-                virDomainAuditCgroupPath(vm, priv->cgroup,
-                                         "deny", path, "rwm", rv == 0);
-                if (rv < 0)
-                    goto cleanup;
-
-                /* If this is the last hostdev with VFIO backend deny
-                 * /dev/vfio/vfio too. */
-                for (i = 0; i < vm->def->nhostdevs; i++) {
-                    virDomainHostdevDefPtr tmp = vm->def->hostdevs[i];
-                    if (tmp->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
-                        tmp->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI &&
-                        tmp->source.subsys.u.pci.backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO)
-                        vfios++;
-                }
-
-                if (vfios == 0) {
-                    VIR_DEBUG("Cgroup deny " DEV_VFIO " for PCI device assignment");
-                    rv = virCgroupDenyDevicePath(priv->cgroup, DEV_VFIO,
-                                                 VIR_CGROUP_DEVICE_RWM, false);
-                    virDomainAuditCgroupPath(vm, priv->cgroup,
-                                             "deny", DEV_VFIO, "rwm", rv == 0);
-                    if (rv < 0)
-                        goto cleanup;
-                }
-            }
-            break;
-        case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
-            /* nothing to tear down for USB */
-            break;
-        case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
-            /* nothing to tear down for SCSI */
-            break;
-        case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST:
-            /* nothing to tear down for scsi_host */
-            break;
-        case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
-            break;
-        }
+    if (dev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
+        dev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI &&
+        dev->source.subsys.u.pci.backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO &&
+        qemuDomainGetHostdevPath(vm->def, dev, true,
+                                 &npaths, &path, NULL) < 0)
+        goto cleanup;
+
+    for (i = 0; i < npaths; i++) {
+        VIR_DEBUG("Cgroup deny %s", path[i]);
+        rv = virCgroupDenyDevicePath(priv->cgroup, path[i],
+                                     VIR_CGROUP_DEVICE_RWM, false);
+        virDomainAuditCgroupPath(vm, priv->cgroup,
+                                 "deny", path[i], "rwm", rv == 0);
+        if (rv < 0)
+            goto cleanup;
     }
 
     ret = 0;
  cleanup:
-    virPCIDeviceFree(pci);
+    for (i = 0; i < npaths; i++)
+        VIR_FREE(path[i]);
     VIR_FREE(path);
     return ret;
 }

src/qemu/qemu_domain.c

@@ -6846,7 +6846,9 @@ qemuDomainSupportsVideoVga(virDomainVideoDefPtr video,
 
 /**
  * qemuDomainGetHostdevPath:
+ * @def: domain definition
  * @dev: host device definition
+ * @teardown: true if device will be removed
  * @npaths: number of items in @path and @perms arrays
  * @path: resulting path to @dev
  * @perms: Optional pointer to VIR_CGROUP_DEVICE_* perms
@@ -6861,7 +6863,9 @@ qemuDomainSupportsVideoVga(virDomainVideoDefPtr video,
  * Returns 0 on success, -1 otherwise.
  */
 int
-qemuDomainGetHostdevPath(virDomainHostdevDefPtr dev,
+qemuDomainGetHostdevPath(virDomainDefPtr def,
+                         virDomainHostdevDefPtr dev,
+                         bool teardown,
                          size_t *npaths,
                          char ***path,
                          int **perms)
@@ -6902,7 +6906,21 @@ qemuDomainGetHostdevPath(virDomainHostdevDefPtr dev,
                 freeTmpPath = true;
 
                 perm = VIR_CGROUP_DEVICE_RW;
-                includeVFIO = true;
+                if (teardown) {
+                    size_t nvfios = 0;
+                    for (i = 0; i < def->nhostdevs; i++) {
+                        virDomainHostdevDefPtr tmp = def->hostdevs[i];
+                        if (tmp->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
+                            tmp->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI &&
+                            tmp->source.subsys.u.pci.backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO)
+                            nvfios++;
+                    }
+
+                    if (nvfios == 0)
+                        includeVFIO = true;
+                } else {
+                    includeVFIO = true;
+                }
             }
             break;
 
@@ -7411,7 +7429,7 @@ qemuDomainSetupHostdev(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
     char **path = NULL;
     size_t i, npaths = 0;
 
-    if (qemuDomainGetHostdevPath(dev, &npaths, &path, NULL) < 0)
+    if (qemuDomainGetHostdevPath(NULL, dev, false, &npaths, &path, NULL) < 0)
         goto cleanup;
 
     for (i = 0; i < npaths; i++) {
@@ -8062,7 +8080,7 @@ qemuDomainNamespaceSetupHostdev(virQEMUDriverPtr driver,
     if (!qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT))
         return 0;
 
-    if (qemuDomainGetHostdevPath(hostdev, &npaths, &path, NULL) < 0)
+    if (qemuDomainGetHostdevPath(NULL, hostdev, false, &npaths, &path, NULL) < 0)
         goto cleanup;
 
     for (i = 0; i < npaths; i++) {
@@ -8093,14 +8111,14 @@ qemuDomainNamespaceTeardownHostdev(virQEMUDriverPtr driver,
     if (!qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT))
         return 0;
 
-    if (qemuDomainGetHostdevPath(hostdev, &npaths, &path, NULL) < 0)
+    if (qemuDomainGetHostdevPath(vm->def, hostdev, true,
+                                 &npaths, &path, NULL) < 0)
         goto cleanup;
 
-    /* Don't remove other paths than for the @hostdev itself.
-     * They might be still in use by other devices. */
-    if (npaths > 0 &&
-        qemuDomainDetachDeviceUnlink(driver, vm, path[0]) < 0)
-        goto cleanup;
+    for (i = 0; i < npaths; i++) {
+        if (qemuDomainDetachDeviceUnlink(driver, vm, path[i]) < 0)
+            goto cleanup;
+    }
 
     ret = 0;
  cleanup:

src/qemu/qemu_domain.h

@@ -802,7 +802,9 @@ int qemuDomainCheckMonitor(virQEMUDriverPtr driver,
 bool qemuDomainSupportsVideoVga(virDomainVideoDefPtr video,
                                 virQEMUCapsPtr qemuCaps);
 
-int qemuDomainGetHostdevPath(virDomainHostdevDefPtr dev,
+int qemuDomainGetHostdevPath(virDomainDefPtr def,
+                             virDomainHostdevDefPtr dev,
+                             bool teardown,
                              size_t *npaths,
                              char ***path,
                              int **perms);