[libvirt] [PATCH] qemu: Keep list of USB devices attached to domains
Osier Yang
jyang at redhat.com
Fri Dec 23 03:48:32 UTC 2011
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 at redhat.com>
> + * Michal Privoznik<mprivozn at 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
More information about the libvir-list
mailing list