[libvirt] [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.
---
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