From 5b66c62d47c15464c0d3db2484a0f4e7762ebfa5 Mon Sep 17 00:00:00 2001
From: Michal Privoznik <mprivozn@redhat.com>
Date: Wed, 16 May 2012 16:42:02 +0200
Subject: [PATCH] qemu: Rollback on used USB devices

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.
(cherry picked from commit 2f5fdc886ec7ed8b871ebd0576271f8ee5be1f71)
---
 src/qemu/qemu_hostdev.c | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index b649ae0872..ff133057a3 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);