[libvirt] [PATCH 2/2] qemu: put usb cgroup setup in common function

[Laine Stump]

The USB-specific cgroup setup had been inserted inline in
qemuDomainAttachHostUsbDevice and qemuSetupCgroup, but now there is a
common cgroup setup function called for all hostdevs, so it makes sens
to put the usb-specific setup there and just rely on that function
being called.

The one thing I'm uncertain of here (and a reason for not pushing
until after release) is that previously hostdev->missing was checked
only when starting a domain (and cgroup setup for the device skipped
if missing was true), but with this consolidation, it is now checked
in the case of hotplug as well. I don't know if this will have any
practical effect (does it make sense to hotplug a "missing" usb
device?)
---
 src/qemu/qemu_cgroup.c  | 61 ++++++++++++++++++++++++++-----------------------
 src/qemu/qemu_cgroup.h  |  3 ---
 src/qemu/qemu_hotplug.c | 16 -------------
 3 files changed, 32 insertions(+), 48 deletions(-)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 92c53d9..7a7824d 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -191,9 +191,10 @@ qemuSetupTPMCgroup(virDomainDefPtr def,
 }
 
 
-int qemuSetupHostUsbDeviceCgroup(virUSBDevicePtr dev ATTRIBUTE_UNUSED,
-                                 const char *path,
-                                 void *opaque)
+static int
+qemuSetupHostUsbDeviceCgroup(virUSBDevicePtr dev ATTRIBUTE_UNUSED,
+                             const char *path,
+                             void *opaque)
 {
     virDomainObjPtr vm = opaque;
     qemuDomainObjPrivatePtr priv = vm->privateData;
@@ -221,6 +222,7 @@ qemuSetupHostdevCGroup(virDomainObjPtr vm,
     int ret = -1;
     qemuDomainObjPrivatePtr priv = vm->privateData;
     virPCIDevicePtr pci = NULL;
+    virUSBDevicePtr usb = NULL;
     char *path = NULL;
 
     /* currently this only does something for PCI devices using vfio
@@ -263,6 +265,28 @@ qemuSetupHostdevCGroup(virDomainObjPtr vm,
                 }
             }
             break;
+
+        case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
+            /* NB: hostdev->missing wasn't previously checked in the
+             * case of hotplug, only when starting a domain. Now it is
+             * always checked, and the cgroup setup skipped if true.
+             */
+            if (dev->missing)
+                break;
+            if ((usb = virUSBDeviceNew(dev->source.subsys.u.usb.bus,
+                                       dev->source.subsys.u.usb.device,
+                                       NULL)) == NULL) {
+                goto cleanup;
+            }
+
+            /* oddly, qemuSetupHostUsbDeviceCgroup doesn't ever
+             * reference the usb object we just created
+             */
+            if (virUSBDeviceFileIterate(usb, qemuSetupHostUsbDeviceCgroup,
+                                        vm) < 0) {
+                goto cleanup;
+            }
+            break;
         default:
             break;
         }
@@ -271,6 +295,7 @@ qemuSetupHostdevCGroup(virDomainObjPtr vm,
     ret = 0;
 cleanup:
     virPCIDeviceFree(pci);
+    virUSBDeviceFree(usb);
     VIR_FREE(path);
     return ret;
 }
@@ -326,6 +351,9 @@ qemuTeardownHostdevCgroup(virDomainObjPtr vm,
                 }
             }
             break;
+        case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
+            /* nothing to tear down for USB */
+            break;
         default:
             break;
         }
@@ -545,33 +573,8 @@ int qemuSetupCgroup(virQEMUDriverPtr driver,
             goto cleanup;
 
         for (i = 0; i < vm->def->nhostdevs; i++) {
-            virDomainHostdevDefPtr hostdev = vm->def->hostdevs[i];
-            virUSBDevicePtr usb;
-
-            if (qemuSetupHostdevCGroup(vm, hostdev) < 0)
+            if (qemuSetupHostdevCGroup(vm, vm->def->hostdevs[i]) < 0)
                 goto cleanup;
-
-            /* NB: the code below here should be moved into
-             * qemuSetupHostdevCGroup()
-             */
-            if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
-                continue;
-            if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB)
-                continue;
-            if (hostdev->missing)
-                continue;
-
-            if ((usb = virUSBDeviceNew(hostdev->source.subsys.u.usb.bus,
-                                       hostdev->source.subsys.u.usb.device,
-                                       NULL)) == NULL)
-                goto cleanup;
-
-            if (virUSBDeviceFileIterate(usb, qemuSetupHostUsbDeviceCgroup,
-                                        vm) < 0) {
-                virUSBDeviceFree(usb);
-                goto cleanup;
-            }
-            virUSBDeviceFree(usb);
         }
     }
 
diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h
index 64d71a5..048c2fa 100644
--- a/src/qemu/qemu_cgroup.h
+++ b/src/qemu/qemu_cgroup.h
@@ -33,9 +33,6 @@ int qemuSetupDiskCgroup(virDomainObjPtr vm,
                         virDomainDiskDefPtr disk);
 int qemuTeardownDiskCgroup(virDomainObjPtr vm,
                            virDomainDiskDefPtr disk);
-int qemuSetupHostUsbDeviceCgroup(virUSBDevicePtr dev,
-                                 const char *path,
-                                 void *opaque);
 int qemuSetupHostdevCGroup(virDomainObjPtr vm,
                            virDomainHostdevDefPtr dev) ATTRIBUTE_RETURN_CHECK;
 int qemuTeardownHostdevCgroup(virDomainObjPtr vm,
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index eeee507..ad46a3d 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1151,22 +1151,6 @@ int qemuDomainAttachHostUsbDevice(virQEMUDriverPtr driver,
         goto error;
     }
 
-    if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) {
-        virUSBDevicePtr usb;
-
-        if ((usb = virUSBDeviceNew(hostdev->source.subsys.u.usb.bus,
-                                hostdev->source.subsys.u.usb.device,
-                                NULL)) == NULL)
-            goto error;
-
-        if (virUSBDeviceFileIterate(usb, qemuSetupHostUsbDeviceCgroup,
-                                    vm) < 0) {
-            virUSBDeviceFree(usb);
-            goto error;
-        }
-        virUSBDeviceFree(usb);
-    }
-
     qemuDomainObjEnterMonitor(driver, vm);
     if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE))
         ret = qemuMonitorAddDevice(priv->mon, devstr);
-- 
1.7.11.7