[PATCH 08/10] qemu: Refactor helpers for USB device attachment

[Osier Yang <jyang@xxxxxxxxxx>]

It's better to put the usb related codes into qemuDomainAttachHostUsbDevice
instead of qemuDomainAttachHostDevice.

And in the old qemuDomainAttachHostDevice, just stealing the "usb" from
driver->activeUsbHostdevs leaks the memory.

Though the "usb" can be reused for virUSBDeviceFileIterate to set up the
cgroup settings, because it will be used as readonly, let's not increase
the confusion, use a "tmp_usb" instead.

---
 src/qemu/qemu_hotplug.c | 94 +++++++++++++++++++++----------------------------
 1 file changed, 41 insertions(+), 53 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 238d0d7..eb3ac52 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1120,36 +1120,52 @@ int qemuDomainAttachHostUsbDevice(virQEMUDriverPtr driver,
                                   virDomainObjPtr vm,
                                   virDomainHostdevDefPtr hostdev)
 {
-    int ret;
     qemuDomainObjPrivatePtr priv = vm->privateData;
+    virUSBDeviceList *list = NULL;
+    virUSBDevicePtr usb = NULL;
     char *devstr = NULL;
+    int ret = -1;
+    bool added = false;
+
+    if (qemuFindHostdevUSBDevice(hostdev, true, &usb) < 0)
+        return -1;
+
+    if (!(list = virUSBDeviceListNew()))
+        goto cleanup;
+
+    if (virUSBDeviceListAdd(list, usb) < 0)
+        goto cleanup;
+
+    if (qemuPrepareHostdevUSBDevices(driver, vm->def->name, list) < 0)
+        goto cleanup;
+
+    added = true;
+    virUSBDeviceListSteal(list, usb);
 
     if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
         if (qemuAssignDeviceHostdevAlias(vm->def, hostdev, -1) < 0)
-            goto error;
+            goto cleanup;
         if (!(devstr = qemuBuildUSBHostdevDevStr(hostdev, priv->qemuCaps)))
-            goto error;
+            goto cleanup;
     }
 
-    if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs+1) < 0) {
-        virReportOOMError();
-        goto error;
-    }
+    if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs+1) < 0)
+        goto cleanup;
 
     if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) {
-        virUSBDevicePtr usb;
+        virUSBDevicePtr tmp_usb;
 
         if ((usb = virUSBDeviceNew(hostdev->source.subsys.u.usb.bus,
-                                hostdev->source.subsys.u.usb.device,
-                                NULL)) == NULL)
-            goto error;
+                                   hostdev->source.subsys.u.usb.device,
+                                   NULL)) == NULL)
+            goto cleanup;
 
         if (virUSBDeviceFileIterate(usb, qemuSetupHostUsbDeviceCgroup,
                                     vm) < 0) {
-            virUSBDeviceFree(usb);
-            goto error;
+            virUSBDeviceFree(tmp_usb);
+            goto cleanup;
         }
-        virUSBDeviceFree(usb);
+        virUSBDeviceFree(tmp_usb);
     }
 
     qemuDomainObjEnterMonitor(driver, vm);
@@ -1160,28 +1176,27 @@ int qemuDomainAttachHostUsbDevice(virQEMUDriverPtr driver,
                                            hostdev->source.subsys.u.usb.bus,
                                            hostdev->source.subsys.u.usb.device);
     qemuDomainObjExitMonitor(driver, vm);
+
     virDomainAuditHostdev(vm, hostdev, "attach", ret == 0);
+
     if (ret < 0)
-        goto error;
+        goto cleanup;
 
     vm->def->hostdevs[vm->def->nhostdevs++] = hostdev;
-
-    VIR_FREE(devstr);
-
-    return 0;
-
-error:
+    ret = 0;
+cleanup:
+    if (added)
+        virUSBDeviceListSteal(driver->activeUsbHostdevs, usb);
+    virUSBDeviceFree(usb);
+    virObjectUnref(list);
     VIR_FREE(devstr);
-    return -1;
+    return ret;
 }
 
 int qemuDomainAttachHostDevice(virQEMUDriverPtr driver,
                                virDomainObjPtr vm,
                                virDomainHostdevDefPtr hostdev)
 {
-    virUSBDeviceList *list;
-    virUSBDevicePtr usb = NULL;
-
     if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                        _("hostdev mode '%s' not supported"),
@@ -1189,30 +1204,9 @@ int qemuDomainAttachHostDevice(virQEMUDriverPtr driver,
         return -1;
     }
 
-    if (!(list = virUSBDeviceListNew()))
-        goto cleanup;
-
-    if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) {
-        if (qemuFindHostdevUSBDevice(hostdev, true, &usb) < 0)
-            goto cleanup;
-
-        if (virUSBDeviceListAdd(list, usb) < 0) {
-            virUSBDeviceFree(usb);
-            usb = NULL;
-            goto cleanup;
-        }
-
-        if (qemuPrepareHostdevUSBDevices(driver, vm->def->name, list) < 0) {
-            usb = NULL;
-            goto cleanup;
-        }
-
-        virUSBDeviceListSteal(list, usb);
-    }
-
     if (virSecurityManagerSetHostdevLabel(driver->securityManager,
                                           vm->def, hostdev, NULL) < 0)
-        goto cleanup;
+        return -1;
 
     switch (hostdev->source.subsys.type) {
     case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
@@ -1234,18 +1228,12 @@ int qemuDomainAttachHostDevice(virQEMUDriverPtr driver,
         goto error;
     }
 
-    virObjectUnref(list);
     return 0;
 
 error:
     if (virSecurityManagerRestoreHostdevLabel(driver->securityManager,
                                               vm->def, hostdev, NULL) < 0)
         VIR_WARN("Unable to restore host device labelling on hotplug fail");
-
-cleanup:
-    virObjectUnref(list);
-    if (usb)
-        virUSBDeviceListSteal(driver->activeUsbHostdevs, usb);
     return -1;
 }
 
-- 
1.8.1.4

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list