[PATCH] qemu: Rollback on used USB devices

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

One of our latest USB device handling patches
05abd1507d66aabb6cad12eeafeb4c4d1911c585 introduced a regression.
That is, we first create a temporary list of all USB devices that
are to be used by domain just starting up. Then we iterate over and
check if a device from the list is in the global list of currently
assigned devices (activeUsbHostdevs). If not, we add it there and
continue with next iteration then. But if a device from temporary
list is either taken already or adding to the activeUsbHostdevs fails,
we remove all devices in temp list from the activeUsbHostdevs list.
Therefore, if a device is already taken we remove it from
activeUsbHostdevs even if we should not. Thus, next time we allow
the device to be assigned to another domain.
---
 src/qemu/qemu_hostdev.c |   28 ++++++++++++----------------
 1 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index b649ae0..ff13305 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -567,7 +567,7 @@ qemuPrepareHostdevUSBDevices(struct qemud_driver *driver,
                              const char *name,
                              usbDeviceList *list)
 {
-    int i;
+    int i, j;
     unsigned int count;
     usbDevice *tmp;
 
@@ -586,7 +586,7 @@ qemuPrepareHostdevUSBDevices(struct qemud_driver *driver,
                 qemuReportError(VIR_ERR_OPERATION_INVALID,
                                 _("USB device %s is already in use"),
                                 usbDeviceGetName(tmp));
-            return -1;
+            goto error;
         }
 
         usbDeviceSetUsedBy(usb, name);
@@ -598,9 +598,16 @@ qemuPrepareHostdevUSBDevices(struct qemud_driver *driver,
          * perform rollback on failure.
          */
         if (usbDeviceListAdd(driver->activeUsbHostdevs, usb) < 0)
-            return -1;
+            goto error;
     }
     return 0;
+
+error:
+    for (j = 0; j < i; j++) {
+        tmp = usbDeviceListGet(list, i);
+        usbDeviceListSteal(driver->activeUsbHostdevs, tmp);
+    }
+    return -1;
 }
 
 static int
@@ -621,8 +628,7 @@ qemuPrepareHostUSBDevices(struct qemud_driver *driver,
     if (!(list = usbDeviceListNew()))
         goto cleanup;
 
-    /* Loop 1: build temporary list and validate no usb device
-     * is already taken
+    /* Loop 1: build temporary list
      */
     for (i = 0 ; i < nhostdevs ; i++) {
         virDomainHostdevDefPtr hostdev = hostdevs[i];
@@ -678,7 +684,7 @@ qemuPrepareHostUSBDevices(struct qemud_driver *driver,
      * wrong, perform rollback.
      */
     if (qemuPrepareHostdevUSBDevices(driver, def->name, list) < 0)
-        goto inactivedevs;
+        goto cleanup;
 
     /* Loop 2: Temporary list was successfully merged with
      * driver list, so steal all items to avoid freeing them
@@ -690,16 +696,6 @@ qemuPrepareHostUSBDevices(struct qemud_driver *driver,
     }
 
     ret = 0;
-    goto cleanup;
-
-inactivedevs:
-    /* Steal devices from driver->activeUsbHostdevs.
-     * We will free them later.
-     */
-    for (i = 0; i < usbDeviceListCount(list); i++) {
-        tmp = usbDeviceListGet(list, i);
-        usbDeviceListSteal(driver->activeUsbHostdevs, tmp);
-    }
 
 cleanup:
     usbDeviceListFree(list);
-- 
1.7.8.5

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