On 2011年12月23日 01:20, Michal Privoznik wrote:
In order to avoid situation where a USB device is
in use by two domains, we must keep a list of already
attached devices like we do for PCI.
---
src/qemu/qemu_conf.h | 2 +
src/qemu/qemu_driver.c | 4 +
src/qemu/qemu_hostdev.c | 92 ++++++++++++++++++++++++++++--
src/qemu/qemu_hostdev.h | 4 +
src/qemu/qemu_hotplug.c | 17 +++++-
src/util/hostusb.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++
src/util/hostusb.h | 18 ++++++
7 files changed, 269 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index f5a0f60..d8d7915 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -36,6 +36,7 @@
# include "security/security_manager.h"
# include "cgroup.h"
# include "pci.h"
+# include "hostusb.h"
# include "cpu_conf.h"
# include "driver.h"
# include "bitmap.h"
@@ -125,6 +126,7 @@ struct qemud_driver {
bool autoStartBypassCache;
pciDeviceList *activePciHostdevs;
+ usbDeviceList *activeUsbHostdevs;
virBitmapPtr reservedVNCPorts;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c908135..eeeb935 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -583,6 +583,9 @@ qemudStartup(int privileged) {
if ((qemu_driver->activePciHostdevs = pciDeviceListNew()) == NULL)
goto error;
+ if ((qemu_driver->activeUsbHostdevs = usbDeviceListNew()) == NULL)
+ goto error;
+
if (privileged) {
if (chown(qemu_driver->libDir, qemu_driver->user,
qemu_driver->group)< 0) {
virReportSystemError(errno,
@@ -773,6 +776,7 @@ qemudShutdown(void) {
qemuDriverLock(qemu_driver);
pciDeviceListFree(qemu_driver->activePciHostdevs);
+ usbDeviceListFree(qemu_driver->activeUsbHostdevs);
virCapabilitiesFree(qemu_driver->caps);
virDomainObjListDeinit(&qemu_driver->domains);
diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index 60401f0..c7adb1d 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -314,13 +314,30 @@ qemuPrepareHostPCIDevices(struct qemud_driver *driver,
}
-static int
-qemuPrepareHostUSBDevices(struct qemud_driver *driver ATTRIBUTE_UNUSED,
- virDomainDefPtr def)
+int
+qemuPrepareHostdevUSBDevices(struct qemud_driver *driver,
+ const char *name,
+ virDomainHostdevDefPtr *hostdevs,
+ int nhostdevs)
{
+ int ret = -1;
int i;
- for (i = 0 ; i< def->nhostdevs ; i++) {
- virDomainHostdevDefPtr hostdev = def->hostdevs[i];
+ usbDeviceList *list;
+ usbDevice *tmp;
+
+ /* To prevent situation where USB device is assigned to two domains
+ * we need to keep a list of currently assigned USB devices.
+ * This is done in several loops which cannot be joined into one big
+ * loop. See qemuPrepareHostdevPCIDevices()
+ */
+ if (!(list = usbDeviceListNew()))
+ goto cleanup;
+
+ /* Loop 1: build temporary list and validate no usb device
+ * is already taken
+ */
+ for (i = 0 ; i< nhostdevs ; i++) {
+ virDomainHostdevDefPtr hostdev = hostdevs[i];
if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
continue;
@@ -339,13 +356,74 @@ qemuPrepareHostUSBDevices(struct qemud_driver *driver
ATTRIBUTE_UNUSED,
hostdev->source.subsys.u.usb.bus = usbDeviceGetBus(usb);
hostdev->source.subsys.u.usb.device = usbDeviceGetDevno(usb);
- usbFreeDevice(usb);
+ if ((tmp = usbDeviceListFind(driver->activeUsbHostdevs, usb))) {
+ const char *other_name = usbDeviceGetUsedBy(tmp);
+
+ if (other_name)
+ qemuReportError(VIR_ERR_OPERATION_INVALID,
+ _("USB device %s is in use by domain
%s"),
+ usbDeviceGetName(tmp), other_name);
+ else
+ qemuReportError(VIR_ERR_OPERATION_INVALID,
+ _("USB device %s is already in use"),
+ usbDeviceGetName(tmp));
+ usbFreeDevice(usb);
+ goto cleanup;
+ }
+
+ if (usbDeviceListAdd(list, usb)< 0) {
+ usbFreeDevice(usb);
+ goto cleanup;
+ }
+
}
}
- return 0;
+ /* Loop 2: Mark devices in temporary list as used by @name
+ * and add them do driver list. However, if something goes
+ * wrong, perform rollback.
+ */
+ for (i = 0; i< usbDeviceListCount(list); i++) {
+ tmp = usbDeviceListGet(list, i);
+ usbDeviceSetUsedBy(tmp, name);
+ if (usbDeviceListAdd(driver->activeUsbHostdevs, tmp)< 0) {
+ usbFreeDevice(tmp);
+ goto inactivedevs;
+ }
+ }
+
+ /* Loop 3: Temporary list was successfully merged with
+ * driver list, so steal all items to avoid freeing them
+ * in cleanup label.
+ */
+ while (usbDeviceListCount(list)> 0) {
+ tmp = usbDeviceListGet(list, 0);
+ usbDeviceListSteal(list, tmp);
+ }
+
+ 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);
+ return ret;
}
+static int
+qemuPrepareHostUSBDevices(struct qemud_driver *driver,
+ virDomainDefPtr def)
+{
+ return qemuPrepareHostdevUSBDevices(driver, def->name, def->hostdevs,
def->nhostdevs);
+}
int qemuPrepareHostDevices(struct qemud_driver *driver,
virDomainDefPtr def)
diff --git a/src/qemu/qemu_hostdev.h b/src/qemu/qemu_hostdev.h
index 07d7de2..d852f5b 100644
--- a/src/qemu/qemu_hostdev.h
+++ b/src/qemu/qemu_hostdev.h
@@ -33,6 +33,10 @@ int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver,
const char *name,
virDomainHostdevDefPtr *hostdevs,
int nhostdevs);
+int qemuPrepareHostdevUSBDevices(struct qemud_driver *driver,
+ const char *name,
+ virDomainHostdevDefPtr *hostdevs,
+ int nhostdevs);
int qemuPrepareHostDevices(struct qemud_driver *driver,
virDomainDefPtr def);
void qemuReattachPciDevice(pciDevice *dev, struct qemud_driver *driver);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 4067bb0..f3597a1 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1097,6 +1097,9 @@ int qemuDomainAttachHostDevice(struct qemud_driver *driver,
/* Resolve USB product/vendor to bus/device */
if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB&&
hostdev->source.subsys.u.usb.vendor) {
+ if (qemuPrepareHostdevUSBDevices(driver, vm->def->name,&hostdev,
1)< 0)
+ goto error;
+
usbDevice *usb
= usbFindDevice(hostdev->source.subsys.u.usb.vendor,
hostdev->source.subsys.u.usb.product);
@@ -2068,6 +2071,7 @@ qemuDomainDetachHostUsbDevice(struct qemud_driver *driver,
{
virDomainHostdevDefPtr detach = NULL;
qemuDomainObjPrivatePtr priv = vm->privateData;
+ usbDevice *usb;
int i, ret;
for (i = 0 ; i< vm->def->nhostdevs ; i++) {
@@ -2123,6 +2127,17 @@ qemuDomainDetachHostUsbDevice(struct qemud_driver *driver,
if (ret< 0)
return -1;
+ usb = usbGetDevice(detach->source.subsys.u.usb.bus,
+ detach->source.subsys.u.usb.device);
+ if (usb) {
+ usbDeviceListDel(driver->activeUsbHostdevs, usb);
+ usbFreeDevice(usb);
+ } else {
+ VIR_WARN("Unable to find device %03d.%03d in list of used USB
devices",
+ detach->source.subsys.u.usb.bus,
+ detach->source.subsys.u.usb.device);
+ }
+
if (vm->def->nhostdevs> 1) {
memmove(vm->def->hostdevs + i,
vm->def->hostdevs + i + 1,
@@ -2162,7 +2177,7 @@ int qemuDomainDetachHostDevice(struct qemud_driver *driver,
switch (hostdev->source.subsys.type) {
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
ret = qemuDomainDetachHostPciDevice(driver, vm, dev,&detach);
- break;
+ break;
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
ret = qemuDomainDetachHostUsbDevice(driver, vm, dev,&detach);
break;
diff --git a/src/util/hostusb.c b/src/util/hostusb.c
index 1669e2f..92f52a2 100644
--- a/src/util/hostusb.c
+++ b/src/util/hostusb.c
@@ -49,6 +49,12 @@ struct _usbDevice {
char name[USB_ADDR_LEN]; /* domain:bus:slot.function */
char id[USB_ID_LEN]; /* product vendor */
char *path;
+ const char *used_by; /* name of the domain using this dev */
+};
+
+struct _usbDeviceList {
+ unsigned int count;
+ usbDevice **devs;
};
/* For virReportOOMError() and virReportSystemError() */
@@ -225,6 +231,22 @@ usbFreeDevice(usbDevice *dev)
}
+void usbDeviceSetUsedBy(usbDevice *dev,
+ const char *name)
+{
+ dev->used_by = name;
+}
+
+const char * usbDeviceGetUsedBy(usbDevice *dev)
+{
+ return dev->used_by;
+}
+
+const char *usbDeviceGetName(usbDevice *dev)
+{
+ return dev->name;
+}
+
unsigned usbDeviceGetBus(usbDevice *dev)
{
return dev->bus;
@@ -243,3 +265,121 @@ int usbDeviceFileIterate(usbDevice *dev,
{
return (actor)(dev, dev->path, opaque);
}
+
+usbDeviceList *
+usbDeviceListNew(void)
+{
+ usbDeviceList *list;
+
+ if (VIR_ALLOC(list)< 0) {
+ virReportOOMError();
+ return NULL;
+ }
+
+ return list;
+}
+
+void
+usbDeviceListFree(usbDeviceList *list)
+{
+ int i;
+
+ if (!list)
+ return;
+
+ for (i = 0; i< list->count; i++)
+ usbFreeDevice(list->devs[i]);
+
+ VIR_FREE(list->devs);
+ VIR_FREE(list);
+}
+
+int
+usbDeviceListAdd(usbDeviceList *list,
+ usbDevice *dev)
+{
+ if (usbDeviceListFind(list, dev)) {
+ usbReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Device %s is already in use"),
+ dev->name);
+ return -1;
+ }
+
+ if (VIR_REALLOC_N(list->devs, list->count+1)< 0) {
+ virReportOOMError();
+ return -1;
+ }
+
+ list->devs[list->count++] = dev;
+
+ return 0;
+}
+
+usbDevice *
+usbDeviceListGet(usbDeviceList *list,
+ int idx)
+{
+ if (idx>= list->count ||
+ idx< 0)
+ return NULL;
+
+ return list->devs[idx];
+}
+
+int
+usbDeviceListCount(usbDeviceList *list)
+{
+ return list->count;
+}
+
+usbDevice *
+usbDeviceListSteal(usbDeviceList *list,
+ usbDevice *dev)
+{
+ usbDevice *ret = NULL;
+ int i;
+
+ for (i = 0; i< list->count; i++) {
+ if (list->devs[i]->bus != dev->bus ||
+ list->devs[i]->dev != dev->dev)
+ continue;
+
+ ret = list->devs[i];
+
+ if (i != list->count--)
+ memmove(&list->devs[i],
+&list->devs[i+1],
+ sizeof(*list->devs) * (list->count - i));
+
+ if (VIR_REALLOC_N(list->devs, list->count)< 0) {
+ ; /* not fatal */
+ }
+
+ break;
+ }
+ return ret;
+}
+
+void
+usbDeviceListDel(usbDeviceList *list,
+ usbDevice *dev)
+{
+ usbDevice *ret = usbDeviceListSteal(list, dev);
+ if (ret)
+ usbFreeDevice(ret);
+}
+
+usbDevice *
+usbDeviceListFind(usbDeviceList *list,
+ usbDevice *dev)
+{
+ int i;
+
+ for (i = 0; i< list->count; i++) {
+ if (list->devs[i]->bus == dev->bus&&
+ list->devs[i]->dev == dev->dev)
+ return list->devs[i];
+ }
+
+ return NULL;
+}
diff --git a/src/util/hostusb.h b/src/util/hostusb.h
index f4a13ca..afaa32f 100644
--- a/src/util/hostusb.h
+++ b/src/util/hostusb.h
@@ -17,6 +17,7 @@
*
* Authors:
* Daniel P. Berrange<berrange(a)redhat.com>
+ * Michal Privoznik<mprivozn(a)redhat.com>
*/
#ifndef __VIR_USB_H__
@@ -25,12 +26,16 @@
# include "internal.h"
typedef struct _usbDevice usbDevice;
+typedef struct _usbDeviceList usbDeviceList;
usbDevice *usbGetDevice(unsigned bus,
unsigned devno);
usbDevice *usbFindDevice(unsigned vendor,
unsigned product);
void usbFreeDevice (usbDevice *dev);
+void usbDeviceSetUsedBy(usbDevice *dev, const char *name);
+const char *usbDeviceGetUsedBy(usbDevice *dev);
+const char *usbDeviceGetName(usbDevice *dev);
unsigned usbDeviceGetBus(usbDevice *dev);
unsigned usbDeviceGetDevno(usbDevice *dev);
@@ -49,5 +54,18 @@ int usbDeviceFileIterate(usbDevice *dev,
usbDeviceFileActor actor,
void *opaque);
+usbDeviceList *usbDeviceListNew(void);
+void usbDeviceListFree(usbDeviceList *list);
+int usbDeviceListAdd(usbDeviceList *list,
+ usbDevice *dev);
+usbDevice * usbDeviceListGet(usbDeviceList *list,
+ int idx);
+int usbDeviceListCount(usbDeviceList *list);
+usbDevice * usbDeviceListSteal(usbDeviceList *list,
+ usbDevice *dev);
+void usbDeviceListDel(usbDeviceList *list,
+ usbDevice *dev);
+usbDevice * usbDeviceListFind(usbDeviceList *list,
+ usbDevice *dev);
#endif /* __VIR_USB_H__ */
Everything looks fine, except missing to add the new internal functions
into libvirt_private.syms. ACK with the nit fixed.
Regards,
Osier