[libvirt] [PATCHv3 00/10] Assign addresses to USB devices

v3: * drop change of commandline order of ccid controllers and USB hubs (that could possibly break compatibility) * assign addresses to redirdevs too * take custom port= setting of xhci controllers into account * hubs without addresses get one assigned only after we took a look on all the other devices Ján Tomko (10): Add USB addresses to qemuhotplug test cases Store USB port path as an array of integers Introduce virDomainUSBDeviceDefForeach Introduce virDomainUSBAddressSet Add functions for adding USB controllers to addrs Add functions for adding USB hubs to addrs Reserve existing USB addresses Assign addresses to USB devices Assign addresses on USB device hotplug Auto-add one hub if there are too many USB devices src/conf/device_conf.h | 2 +- src/conf/domain_addr.c | 491 +++++++++++++++++++++ src/conf/domain_addr.h | 58 +++ src/conf/domain_conf.c | 110 ++++- src/conf/domain_conf.h | 7 + src/libvirt_private.syms | 12 + src/qemu/qemu_command.c | 3 +- src/qemu/qemu_domain.h | 1 + src/qemu/qemu_domain_address.c | 155 ++++++- src/qemu/qemu_hotplug.c | 27 ++ ...otplug-console-compat-2-live+console-virtio.xml | 1 + .../qemuhotplug-hotplug-base-live+disk-usb.xml | 1 + .../qemuxml2argvdata/qemuxml2argv-bios-nvram.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-bios.args | 2 +- .../qemuxml2argv-console-compat-2-live.xml | 1 + .../qemuxml2argv-console-compat-2.xml | 4 +- .../qemuxml2argv-controller-order.args | 8 +- .../qemuxml2argv-disk-usb-device-removable.args | 3 +- .../qemuxml2argv-disk-usb-device.args | 2 +- .../qemuxml2argv-graphics-spice-timeout.args | 2 +- .../qemuxml2argv-graphics-spice-usb-redir.args | 2 +- ...muxml2argv-hostdev-usb-address-device-boot.args | 2 +- .../qemuxml2argv-hostdev-usb-address-device.args | 2 +- .../qemuxml2argv-hostdev-usb-address.args | 2 +- .../qemuxml2argv-hugepages-numa.args | 6 +- .../qemuxml2argv-input-usbmouse.args | 2 +- .../qemuxml2argv-input-usbtablet.args | 2 +- .../qemuxml2argv-pseries-usb-kbd.args | 2 +- .../qemuxml2argv-serial-spiceport.args | 2 +- .../qemuxml2argv-smartcard-controller.args | 2 +- .../qemuxml2argv-smartcard-host-certificates.args | 2 +- .../qemuxml2argv-smartcard-host.args | 2 +- ...emuxml2argv-smartcard-passthrough-spicevmc.args | 2 +- .../qemuxml2argv-smartcard-passthrough-tcp.args | 2 +- .../qemuxml2argv-sound-device.args | 2 +- .../qemuxml2argv-usb-hub-autoadd.args | 28 ++ .../qemuxml2argv-usb-hub-autoadd.xml | 23 + .../qemuxml2argv-usb-hub-conflict.args | 25 ++ .../qemuxml2argv-usb-hub-conflict.xml | 22 + .../qemuxml2argv-usb-port-autoassign.args | 28 ++ .../qemuxml2argv-usb-port-autoassign.xml | 27 ++ .../qemuxml2argv-usb-redir-boot.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-usb-redir.args | 2 +- tests/qemuxml2argvtest.c | 9 + 44 files changed, 1048 insertions(+), 44 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-hub-autoadd.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-hub-autoadd.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-hub-conflict.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-hub-conflict.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-port-autoassign.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-port-autoassign.xml -- 2.7.3

This test assumes the XML will be the same after formatting. Add USB addresses to it to keep it working when we autoassign them. --- .../qemuhotplug-console-compat-2-live+console-virtio.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-console-compat-2-live.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml | 4 +++- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2-live+console-virtio.xml b/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2-live+console-virtio.xml index 3495ee6..7ca36d5 100644 --- a/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2-live+console-virtio.xml +++ b/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2-live+console-virtio.xml @@ -100,6 +100,7 @@ </channel> <input type='tablet' bus='usb'> <alias name='input0'/> + <address type='usb' bus='0' port='1'/> </input> <input type='mouse' bus='ps2'> <alias name='input1'/> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-console-compat-2-live.xml b/tests/qemuxml2argvdata/qemuxml2argv-console-compat-2-live.xml index b36af27..f300940 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-console-compat-2-live.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-console-compat-2-live.xml @@ -95,6 +95,7 @@ </channel> <input type='tablet' bus='usb'> <alias name='input0'/> + <address type='usb' bus='0' port='1'/> </input> <input type='mouse' bus='ps2'> <alias name='input1'/> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml b/tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml index 2ae104e..7b35709 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml @@ -78,7 +78,9 @@ <target type='virtio' name='org.qemu.guest_agent.0'/> <address type='virtio-serial' controller='0' bus='0' port='1'/> </channel> - <input type='tablet' bus='usb'/> + <input type='tablet' bus='usb'> + <address type='usb' bus='0' port='1'/> + </input> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <graphics type='vnc' port='-1' autoport='yes' listen='0.0.0.0'> -- 2.7.3

On 23/06/16 09:45, Ján Tomko wrote:
This test assumes the XML will be the same after formatting. Add USB addresses to it to keep it working when we autoassign them. --- .../qemuhotplug-console-compat-2-live+console-virtio.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-console-compat-2-live.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml | 4 +++- 3 files changed, 5 insertions(+), 1 deletion(-)
... ACK Erik

In preparation to tracking which USB addresses are occupied. Introduce two helper functions for printing the port path as a string and appending it to a virBuffer. --- src/conf/device_conf.h | 2 +- src/conf/domain_addr.c | 26 ++++++++++++++++++++++++++ src/conf/domain_addr.h | 8 ++++++++ src/conf/domain_conf.c | 21 +++++++++------------ src/libvirt_private.syms | 2 ++ src/qemu/qemu_command.c | 3 ++- 6 files changed, 48 insertions(+), 14 deletions(-) diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h index 934afd4..478060a 100644 --- a/src/conf/device_conf.h +++ b/src/conf/device_conf.h @@ -122,7 +122,7 @@ typedef struct _virDomainDeviceCcidAddress { typedef struct _virDomainDeviceUSBAddress { unsigned int bus; - char *port; + unsigned int port[VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH]; } virDomainDeviceUSBAddress, *virDomainDeviceUSBAddressPtr; typedef struct _virDomainDeviceSpaprVioAddress { diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 794270d..1f5193c 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -1251,3 +1251,29 @@ virDomainVirtioSerialAddrRelease(virDomainVirtioSerialAddrSetPtr addrs, VIR_FREE(str); return ret; } + + +void +virDomainUSBAddressPortFormatBuf(virBufferPtr buf, + unsigned int *port) +{ + size_t i; + + for (i = 0; i < VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH; i++) { + if (port[i] == 0) + break; + virBufferAsprintf(buf, "%u.", port[i]); + } + virBufferTrim(buf, ".", -1); +} + + +char * +virDomainUSBAddressPortFormat(unsigned int *port) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + virDomainUSBAddressPortFormatBuf(&buf, port); + if (virBufferCheckError(&buf) < 0) + return NULL; + return virBufferContentAndReset(&buf); +} diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index f3eda89..8efd512 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -237,4 +237,12 @@ virDomainVirtioSerialAddrRelease(virDomainVirtioSerialAddrSetPtr addrs, virDomainDeviceInfoPtr info) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +void +virDomainUSBAddressPortFormatBuf(virBufferPtr buf, + unsigned int *port) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +char * +virDomainUSBAddressPortFormat(unsigned int *port) + ATTRIBUTE_NONNULL(1); + #endif /* __DOMAIN_ADDR_H__ */ diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9443281..780d705 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -32,6 +32,7 @@ #include "internal.h" #include "virerror.h" #include "datatypes.h" +#include "domain_addr.h" #include "domain_conf.h" #include "snapshot_conf.h" #include "viralloc.h" @@ -3321,8 +3322,6 @@ virDomainDeviceInfoCopy(virDomainDeviceInfoPtr dst, void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info) { VIR_FREE(info->alias); - if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) - VIR_FREE(info->addr.usb.port); memset(&info->addr, 0, sizeof(info->addr)); info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE; VIR_FREE(info->romfile); @@ -4867,9 +4866,10 @@ virDomainDeviceInfoFormat(virBufferPtr buf, break; case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB: - virBufferAsprintf(buf, " bus='%d' port='%s'", - info->addr.usb.bus, - info->addr.usb.port); + virBufferAsprintf(buf, " bus='%d' port='", + info->addr.usb.bus); + virDomainUSBAddressPortFormatBuf(buf, info->addr.usb.port); + virBufferAddLit(buf, "'"); break; case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO: @@ -5101,14 +5101,14 @@ virDomainDeviceCcidAddressParseXML(xmlNodePtr node, } static int -virDomainDeviceUSBAddressParsePort(char *port) +virDomainDeviceUSBAddressParsePort(virDomainDeviceUSBAddressPtr addr, + char *port) { - unsigned int p; char *tmp = port; size_t i; for (i = 0; i < VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH; i++) { - if (virStrToLong_uip(tmp, &tmp, 10, &p) < 0) + if (virStrToLong_uip(tmp, &tmp, 10, &addr->port[i]) < 0) break; if (*tmp == '\0') @@ -5135,12 +5135,9 @@ virDomainDeviceUSBAddressParseXML(xmlNodePtr node, port = virXMLPropString(node, "port"); bus = virXMLPropString(node, "bus"); - if (port && virDomainDeviceUSBAddressParsePort(port) < 0) + if (port && virDomainDeviceUSBAddressParsePort(addr, port) < 0) goto cleanup; - addr->port = port; - port = NULL; - if (bus && virStrToLong_uip(bus, NULL, 10, &addr->bus) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 501c23e..1fb06b1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -109,6 +109,8 @@ virDomainPCIAddressSetGrow; virDomainPCIAddressSlotInUse; virDomainPCIAddressValidate; virDomainPCIControllerModelToConnectType; +virDomainUSBAddressPortFormat; +virDomainUSBAddressPortFormatBuf; virDomainVirtioSerialAddrAssign; virDomainVirtioSerialAddrAutoAssign; virDomainVirtioSerialAddrIsComplete; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6944129..a4fcbe2 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -375,7 +375,8 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, VIR_DOMAIN_CONTROLLER_TYPE_USB, info->addr.usb.bus))) goto cleanup; - virBufferAsprintf(buf, ",bus=%s.0,port=%s", contAlias, info->addr.usb.port); + virBufferAsprintf(buf, ",bus=%s.0,port=", contAlias); + virDomainUSBAddressPortFormatBuf(buf, info->addr.usb.port); } else if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO) { if (info->addr.spaprvio.has_reg) virBufferAsprintf(buf, ",reg=0x%llx", info->addr.spaprvio.reg); -- 2.7.3

On 23/06/16 09:45, Ján Tomko wrote:
In preparation to tracking which USB addresses are occupied. Introduce two helper functions for printing the port path as a string and appending it to a virBuffer. --- src/conf/device_conf.h | 2 +- src/conf/domain_addr.c | 26 ++++++++++++++++++++++++++ src/conf/domain_addr.h | 8 ++++++++ src/conf/domain_conf.c | 21 +++++++++------------ src/libvirt_private.syms | 2 ++ src/qemu/qemu_command.c | 3 ++- 6 files changed, 48 insertions(+), 14 deletions(-)
diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h index 934afd4..478060a 100644 --- a/src/conf/device_conf.h +++ b/src/conf/device_conf.h @@ -122,7 +122,7 @@ typedef struct _virDomainDeviceCcidAddress {
typedef struct _virDomainDeviceUSBAddress { unsigned int bus; - char *port; + unsigned int port[VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH]; } virDomainDeviceUSBAddress, *virDomainDeviceUSBAddressPtr;
typedef struct _virDomainDeviceSpaprVioAddress { diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 794270d..1f5193c 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -1251,3 +1251,29 @@ virDomainVirtioSerialAddrRelease(virDomainVirtioSerialAddrSetPtr addrs, VIR_FREE(str); return ret; } + + +void +virDomainUSBAddressPortFormatBuf(virBufferPtr buf, + unsigned int *port) +{ + size_t i; + + for (i = 0; i < VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH; i++) { + if (port[i] == 0) + break; + virBufferAsprintf(buf, "%u.", port[i]); + } + virBufferTrim(buf, ".", -1); +} + + +char * +virDomainUSBAddressPortFormat(unsigned int *port) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + virDomainUSBAddressPortFormatBuf(&buf, port); + if (virBufferCheckError(&buf) < 0) + return NULL; + return virBufferContentAndReset(&buf); +} diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index f3eda89..8efd512 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -237,4 +237,12 @@ virDomainVirtioSerialAddrRelease(virDomainVirtioSerialAddrSetPtr addrs, virDomainDeviceInfoPtr info) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+void +virDomainUSBAddressPortFormatBuf(virBufferPtr buf, + unsigned int *port) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +char * +virDomainUSBAddressPortFormat(unsigned int *port) + ATTRIBUTE_NONNULL(1); + #endif /* __DOMAIN_ADDR_H__ */ diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9443281..780d705 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -32,6 +32,7 @@ #include "internal.h" #include "virerror.h" #include "datatypes.h" +#include "domain_addr.h" #include "domain_conf.h" #include "snapshot_conf.h" #include "viralloc.h" @@ -3321,8 +3322,6 @@ virDomainDeviceInfoCopy(virDomainDeviceInfoPtr dst, void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info) { VIR_FREE(info->alias); - if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) - VIR_FREE(info->addr.usb.port); memset(&info->addr, 0, sizeof(info->addr)); info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE; VIR_FREE(info->romfile); @@ -4867,9 +4866,10 @@ virDomainDeviceInfoFormat(virBufferPtr buf, break;
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB: - virBufferAsprintf(buf, " bus='%d' port='%s'", - info->addr.usb.bus, - info->addr.usb.port); + virBufferAsprintf(buf, " bus='%d' port='", + info->addr.usb.bus); + virDomainUSBAddressPortFormatBuf(buf, info->addr.usb.port); + virBufferAddLit(buf, "'"); break;
The empty string that could be formatted into the buffer if the port wasn't originally specified was the only thing that had caught my eye. I tried to define a domain with the port attribute omitted, since according to virDomainDeviceUSBAddressParseXML port is optional. I hit the RNG validation failure but once I bypassed it, the domain was defined just fine. However, once I started it, I got an error: "Cannot parse <address> 'port' attribute". I suspect it's due to virDomainObjSetDefTransient running the xml parser again, which then fails on the empty string because we formatted it back. I got the same behavior with 1.3.5 as well with the only difference being that we format the port back as '(null)', so imho we should not format the port back if it would lead to '' or '(null)'. Again, your patch did not break the functionality (as described above), it *was* already broken, so that issue should be fixed first. Erik

A helper that will execute a callback on every USB device in the domain definition. With an ability to skip USB hubs, since we will want to treat them differently in some cases. --- src/conf/domain_conf.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 7 ++++ src/libvirt_private.syms | 1 + 3 files changed, 97 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 780d705..396655a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -24054,6 +24054,95 @@ virDomainSmartcardDefForeach(virDomainDefPtr def, } +int +virDomainUSBDeviceDefForeach(virDomainDefPtr def, + virDomainUSBDeviceDefIterator iter, + void *opaque, + bool skipHubs) +{ + size_t i; + + /* usb-hub */ + if (!skipHubs) { + for (i = 0; i < def->nhubs; i++) { + virDomainHubDefPtr hub = def->hubs[i]; + if (hub->type == VIR_DOMAIN_HUB_TYPE_USB) { + if (iter(&hub->info, opaque) < 0) + return -1; + } + } + } + + /* usb-host */ + for (i = 0; i < def->nhostdevs; i++) { + virDomainHostdevDefPtr hostdev = def->hostdevs[i]; + if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) { + if (iter(hostdev->info, opaque) < 0) + return -1; + } + } + + /* usb-storage */ + for (i = 0; i < def->ndisks; i++) { + virDomainDiskDefPtr disk = def->disks[i]; + if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) { + if (iter(&disk->info, opaque) < 0) + return -1; + } + } + + /* TODO: add def->nets here when libvirt starts supporting usb-net */ + + /* usb-ccid */ + for (i = 0; i < def->ncontrollers; i++) { + virDomainControllerDefPtr cont = def->controllers[i]; + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_CCID) { + if (iter(&cont->info, opaque) < 0) + return -1; + } + } + + /* usb-kbd, usb-mouse, usb-tablet */ + for (i = 0; i < def->ninputs; i++) { + virDomainInputDefPtr input = def->inputs[i]; + + if (input->bus == VIR_DOMAIN_INPUT_BUS_USB) { + if (iter(&input->info, opaque) < 0) + return -1; + } + } + + /* usb-serial */ + for (i = 0; i < def->nserials; i++) { + virDomainChrDefPtr serial = def->serials[i]; + if (serial->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB) { + if (iter(&serial->info, opaque) < 0) + return -1; + } + } + + /* usb-audio model=usb */ + for (i = 0; i < def->nsounds; i++) { + virDomainSoundDefPtr sound = def->sounds[i]; + if (sound->model == VIR_DOMAIN_SOUND_MODEL_USB) { + if (iter(&sound->info, opaque) < 0) + return -1; + } + } + + /* usb-redir */ + for (i = 0; i < def->nredirdevs; i++) { + virDomainRedirdevDefPtr redirdev = def->redirdevs[i]; + if (redirdev->bus == VIR_DOMAIN_REDIRDEV_BUS_USB) { + if (iter(&redirdev->info, opaque) < 0) + return -1; + } + } + + return 0; +} + + /* Call iter(disk, name, depth, opaque) for each element of disk and * its backing chain in the pre-populated disk->src.backingStore. * ignoreOpenFailure determines whether to warn about a chain that diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 6e81e52..149b75f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2898,6 +2898,13 @@ typedef int (*virDomainDiskDefPathIterator)(virDomainDiskDefPtr disk, size_t depth, void *opaque); +typedef int (*virDomainUSBDeviceDefIterator)(virDomainDeviceInfoPtr info, + void *opaque); +int virDomainUSBDeviceDefForeach(virDomainDefPtr def, + virDomainUSBDeviceDefIterator iter, + void *opaque, + bool skipHubs); + int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk, bool ignoreOpenFailure, virDomainDiskDefPathIterator iter, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1fb06b1..414d990 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -476,6 +476,7 @@ virDomainTPMBackendTypeToString; virDomainTPMDefFree; virDomainTPMModelTypeFromString; virDomainTPMModelTypeToString; +virDomainUSBDeviceDefForeach; virDomainVideoDefaultRAM; virDomainVideoDefaultType; virDomainVideoDefFree; -- 2.7.3

On 23/06/16 09:45, Ján Tomko wrote:
A helper that will execute a callback on every USB device in the domain definition.
With an ability to skip USB hubs, since we will want to treat them differently in some cases. --- src/conf/domain_conf.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 7 ++++ src/libvirt_private.syms | 1 + 3 files changed, 97 insertions(+)
... ACK, this on is independent from the previous one. The change also looks pretty reasonable to me so it can go straight in. Erik

A new type to track USB addresses. Every <controller type='usb' index='i'/> is represented by as a virDomainUSBAddressHub at buses[i]. Each of these hubs has up to 'nports' ports. If a port is occupied, it has the corresponding bit set in the 'ports' bitmap, e.g. port 1 would have the 0th bit set. If there is a hub on this port, then hubs[i] will point to this hub. --- src/conf/domain_addr.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_addr.h | 22 ++++++++++++++++++++++ src/libvirt_private.syms | 2 ++ 3 files changed, 70 insertions(+) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 1f5193c..a43657b 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -1277,3 +1277,49 @@ virDomainUSBAddressPortFormat(unsigned int *port) return NULL; return virBufferContentAndReset(&buf); } + + +virDomainUSBAddressSetPtr +virDomainUSBAddressSetCreate(void) +{ + virDomainUSBAddressSetPtr addrs; + + if (VIR_ALLOC(addrs) < 0) + return NULL; + + return addrs; +} + + +static void +virDomainUSBAddressHubFree(virDomainUSBAddressHubPtr hub) +{ + size_t i; + + if (!hub) + return; + + for (i = 0; i < hub->nports; i++) { + if (hub->hubs[i]) + virDomainUSBAddressHubFree(hub->hubs[i]); + } + virBitmapFree(hub->ports); + VIR_FREE(hub); +} + + +void +virDomainUSBAddressSetFree(virDomainUSBAddressSetPtr addrs) +{ + size_t i; + + if (!addrs) + return; + + for (i = 0; i < addrs->nbuses; i++) { + if (addrs->buses[i]) + virDomainUSBAddressHubFree(addrs->buses[i]); + } + VIR_FREE(addrs->buses); + VIR_FREE(addrs); +} diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index 8efd512..51bbb61 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -245,4 +245,26 @@ char * virDomainUSBAddressPortFormat(unsigned int *port) ATTRIBUTE_NONNULL(1); +typedef struct _virDomainUSBAddressHub virDomainUSBAddressHub; +typedef virDomainUSBAddressHub *virDomainUSBAddressHubPtr; +struct _virDomainUSBAddressHub { + /* indexes are shifted by one: + * ports[0] represents port 1, because ports are numbered from 1 */ + virBitmapPtr ports; + size_t nports; + virDomainUSBAddressHubPtr *hubs; +}; + +struct _virDomainUSBAddressSet { + /* every <controller type='usb' index='i'> is represented + * as a hub at buses[i] */ + virDomainUSBAddressHubPtr *buses; + size_t nbuses; +}; +typedef struct _virDomainUSBAddressSet virDomainUSBAddressSet; +typedef virDomainUSBAddressSet *virDomainUSBAddressSetPtr; + +virDomainUSBAddressSetPtr virDomainUSBAddressSetCreate(void); +void virDomainUSBAddressSetFree(virDomainUSBAddressSetPtr addrs); + #endif /* __DOMAIN_ADDR_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 414d990..9e1bad7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -111,6 +111,8 @@ virDomainPCIAddressValidate; virDomainPCIControllerModelToConnectType; virDomainUSBAddressPortFormat; virDomainUSBAddressPortFormatBuf; +virDomainUSBAddressSetCreate; +virDomainUSBAddressSetFree; virDomainVirtioSerialAddrAssign; virDomainVirtioSerialAddrAutoAssign; virDomainVirtioSerialAddrIsComplete; -- 2.7.3

On 23/06/16 09:45, Ján Tomko wrote:
A new type to track USB addresses.
Every <controller type='usb' index='i'/> is represented by as a virDomainUSBAddressHub at buses[i].
"represented by as..", I'd say something like "is represented by an object of type virDomainUSBAddressHub where each of such objects can be located at buses[i]" or something similar...
Each of these hubs has up to 'nports' ports. If a port is occupied, it has the corresponding bit set in the 'ports' bitmap, e.g. port 1 would have the 0th bit set. If there is a hub on this port, then hubs[i] will point to this hub. --- src/conf/domain_addr.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_addr.h | 22 ++++++++++++++++++++++ src/libvirt_private.syms | 2 ++ 3 files changed, 70 insertions(+)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 1f5193c..a43657b 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -1277,3 +1277,49 @@ virDomainUSBAddressPortFormat(unsigned int *port) return NULL; return virBufferContentAndReset(&buf); } + + +virDomainUSBAddressSetPtr +virDomainUSBAddressSetCreate(void) +{ + virDomainUSBAddressSetPtr addrs; + + if (VIR_ALLOC(addrs) < 0) + return NULL; + + return addrs; +} + + +static void +virDomainUSBAddressHubFree(virDomainUSBAddressHubPtr hub) +{ + size_t i; + + if (!hub) + return; + + for (i = 0; i < hub->nports; i++) { + if (hub->hubs[i]) + virDomainUSBAddressHubFree(hub->hubs[i]); + } + virBitmapFree(hub->ports); + VIR_FREE(hub); +} + + +void +virDomainUSBAddressSetFree(virDomainUSBAddressSetPtr addrs) +{ + size_t i; + + if (!addrs) + return; + + for (i = 0; i < addrs->nbuses; i++) { + if (addrs->buses[i]) + virDomainUSBAddressHubFree(addrs->buses[i]); + } + VIR_FREE(addrs->buses); + VIR_FREE(addrs); +} diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index 8efd512..51bbb61 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -245,4 +245,26 @@ char * virDomainUSBAddressPortFormat(unsigned int *port) ATTRIBUTE_NONNULL(1);
+typedef struct _virDomainUSBAddressHub virDomainUSBAddressHub; +typedef virDomainUSBAddressHub *virDomainUSBAddressHubPtr; +struct _virDomainUSBAddressHub { + /* indexes are shifted by one: + * ports[0] represents port 1, because ports are numbered from 1 */ + virBitmapPtr ports; + size_t nports; + virDomainUSBAddressHubPtr *hubs; +}; + +struct _virDomainUSBAddressSet { + /* every <controller type='usb' index='i'> is represented + * as a hub at buses[i] */ + virDomainUSBAddressHubPtr *buses; + size_t nbuses; +};
At first, I wasn't really convinced by the data type name "virDomainUSBAddressSet", but unfortunately I couldn't come up with anything more descriptive, so I guess I'm okay with it.
+typedef struct _virDomainUSBAddressSet virDomainUSBAddressSet; +typedef virDomainUSBAddressSet *virDomainUSBAddressSetPtr; + +virDomainUSBAddressSetPtr virDomainUSBAddressSetCreate(void); +void virDomainUSBAddressSetFree(virDomainUSBAddressSetPtr addrs); + #endif /* __DOMAIN_ADDR_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 414d990..9e1bad7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -111,6 +111,8 @@ virDomainPCIAddressValidate; virDomainPCIControllerModelToConnectType; virDomainUSBAddressPortFormat; virDomainUSBAddressPortFormatBuf; +virDomainUSBAddressSetCreate; +virDomainUSBAddressSetFree; virDomainVirtioSerialAddrAssign; virDomainVirtioSerialAddrAutoAssign; virDomainVirtioSerialAddrIsComplete;
ACK Erik

Walk through all the usb controllers in the domain definition and create the corresponding structures in the virDomainUSBAddressSet. --- src/conf/domain_addr.c | 121 +++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_addr.h | 4 ++ src/libvirt_private.syms | 1 + 3 files changed, 126 insertions(+) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index a43657b..9ae9570 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -1323,3 +1323,124 @@ virDomainUSBAddressSetFree(virDomainUSBAddressSetPtr addrs) VIR_FREE(addrs->buses); VIR_FREE(addrs); } + + +static size_t +virDomainUSBAddressControllerModelToPorts(virDomainControllerDefPtr cont) +{ + int model = cont->model; + + if (model == -1) + model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI; + + switch ((virDomainControllerModelUSB) model) { + case VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI: + case VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX4_UHCI: + case VIR_DOMAIN_CONTROLLER_MODEL_USB_VT82C686B_UHCI: + return 2; + + case VIR_DOMAIN_CONTROLLER_MODEL_USB_EHCI: + case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_EHCI1: + return 6; + + case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI1: + case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI2: + case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI3: + /* These have two ports each and are used to provide USB1.1 + * ports while ICH9_EHCI1 provides 6 USB2.0 ports. + * Ignore these since we will add the EHCI1 too. */ + return 0; + + case VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI: + return 3; + + case VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI: + if (cont->opts.usbopts.ports != -1) + return cont->opts.usbopts.ports; + return 4; + + case VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE: + case VIR_DOMAIN_CONTROLLER_MODEL_USB_LAST: + /* yoda */ + break; + } + return 0; +} + + +static virDomainUSBAddressHubPtr +virDomainUSBAddressHubNew(size_t nports) +{ + virDomainUSBAddressHubPtr hub = NULL, ret = NULL; + + if (VIR_ALLOC(hub) < 0) + goto cleanup; + + if (!(hub->ports = virBitmapNew(nports))) + goto cleanup; + + if (VIR_ALLOC_N(hub->hubs, nports) < 0) + goto cleanup; + hub->nports = nports; + + ret = hub; + hub = NULL; + cleanup: + virDomainUSBAddressHubFree(hub); + return ret; +} + + +static int +virDomainUSBAddressSetAddController(virDomainUSBAddressSetPtr addrs, + virDomainControllerDefPtr cont) +{ + size_t nports = virDomainUSBAddressControllerModelToPorts(cont); + virDomainUSBAddressHubPtr hub = NULL; + int ret = -1; + + VIR_DEBUG("Adding a USB controller model=%s with %zu ports", + virDomainControllerModelUSBTypeToString(cont->model), + nports); + + /* Skip UHCI{1,2,3} companions; only add the EHCI1 */ + if (nports == 0) + return 0; + + if (addrs->nbuses <= cont->idx) { + if (VIR_EXPAND_N(addrs->buses, addrs->nbuses, cont->idx - addrs->nbuses + 1) < 0) + goto cleanup; + } else if (addrs->buses[cont->idx]) { + virReportError(VIR_ERR_XML_ERROR, + _("Duplicate USB controllers with index %u"), + cont->idx); + goto cleanup; + } + + if (!(hub = virDomainUSBAddressHubNew(nports))) + goto cleanup; + + addrs->buses[cont->idx] = hub; + hub = NULL; + + ret = 0; + cleanup: + virDomainUSBAddressHubFree(hub); + return ret; +} + + +int virDomainUSBAddressSetAddControllers(virDomainUSBAddressSetPtr addrs, + virDomainDefPtr def) +{ + size_t i; + + for (i = 0; i < def->ncontrollers; i++) { + virDomainControllerDefPtr cont = def->controllers[i]; + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB) { + if (virDomainUSBAddressSetAddController(addrs, cont) < 0) + return -1; + } + } + return 0; +} diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index 51bbb61..693c5ee 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -265,6 +265,10 @@ typedef struct _virDomainUSBAddressSet virDomainUSBAddressSet; typedef virDomainUSBAddressSet *virDomainUSBAddressSetPtr; virDomainUSBAddressSetPtr virDomainUSBAddressSetCreate(void); + +int virDomainUSBAddressSetAddControllers(virDomainUSBAddressSetPtr addrs, + virDomainDefPtr def) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); void virDomainUSBAddressSetFree(virDomainUSBAddressSetPtr addrs); #endif /* __DOMAIN_ADDR_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9e1bad7..9e99216 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -111,6 +111,7 @@ virDomainPCIAddressValidate; virDomainPCIControllerModelToConnectType; virDomainUSBAddressPortFormat; virDomainUSBAddressPortFormatBuf; +virDomainUSBAddressSetAddControllers; virDomainUSBAddressSetCreate; virDomainUSBAddressSetFree; virDomainVirtioSerialAddrAssign; -- 2.7.3

On 23/06/16 09:45, Ján Tomko wrote:
Walk through all the usb controllers in the domain definition and create the corresponding structures in the virDomainUSBAddressSet. --- src/conf/domain_addr.c | 121 +++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_addr.h | 4 ++ src/libvirt_private.syms | 1 + 3 files changed, 126 insertions(+)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index a43657b..9ae9570 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -1323,3 +1323,124 @@ virDomainUSBAddressSetFree(virDomainUSBAddressSetPtr addrs) VIR_FREE(addrs->buses); VIR_FREE(addrs); } + + +static size_t +virDomainUSBAddressControllerModelToPorts(virDomainControllerDefPtr cont) +{ + int model = cont->model; + + if (model == -1) + model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI; + + switch ((virDomainControllerModelUSB) model) { + case VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI: + case VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX4_UHCI: + case VIR_DOMAIN_CONTROLLER_MODEL_USB_VT82C686B_UHCI: + return 2; + + case VIR_DOMAIN_CONTROLLER_MODEL_USB_EHCI: + case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_EHCI1: + return 6; + + case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI1: + case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI2: + case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI3: + /* These have two ports each and are used to provide USB1.1 + * ports while ICH9_EHCI1 provides 6 USB2.0 ports. + * Ignore these since we will add the EHCI1 too. */ + return 0; + + case VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI: + return 3; + + case VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI: + if (cont->opts.usbopts.ports != -1) + return cont->opts.usbopts.ports;
Since you incorporated Gerd's comment on making xHCI controller's port range configurable, I didn't find anything else (besides the line below), so I'm tempted to say ACK, but maybe someone who knows more about usb controllers might want to put their two cents in. Erik
+ return 4; + + case VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE: + case VIR_DOMAIN_CONTROLLER_MODEL_USB_LAST: + /* yoda */
please remove this ^^ line
+ break; + } + return 0; +} +

Walk through all the usb hubs in the domain definition that have a USB address specified, create the corresponding structures in the virDomainUSBAddressSet and mark the port it occupies as used. --- src/conf/domain_addr.c | 114 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 9ae9570..343ae0c 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -1430,6 +1430,109 @@ virDomainUSBAddressSetAddController(virDomainUSBAddressSetPtr addrs, } +static ssize_t +virDomainUSBAddressGetLastIdx(virDomainDeviceInfoPtr info) +{ + ssize_t i; + for (i = VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH - 1; i > 0; i--) { + if (info->addr.usb.port[i] != 0) + break; + } + return i; +} + + +/* Find the USBAddressHub structure representing the hub/controller + * that corresponds to the bus/port path specified by info. + * Returns the index of the requested port in targetIdx. + */ +static virDomainUSBAddressHubPtr +virDomainUSBAddressFindPort(virDomainUSBAddressSetPtr addrs, + virDomainDeviceInfoPtr info, + int *targetIdx, + const char *portStr) +{ + virDomainUSBAddressHubPtr hub = NULL; + ssize_t i, lastIdx; + + if (info->addr.usb.bus >= addrs->nbuses || + !addrs->buses[info->addr.usb.bus]) { + virReportError(VIR_ERR_XML_ERROR, _("Missing USB bus %u"), + info->addr.usb.bus); + return NULL; + } + hub = addrs->buses[info->addr.usb.bus]; + + lastIdx = virDomainUSBAddressGetLastIdx(info); + + for (i = 0; i < lastIdx; i++) { + /* ports are numbered from 1 */ + int portIdx = info->addr.usb.port[i] - 1; + + if (hub->nports <= portIdx) { + virReportError(VIR_ERR_XML_ERROR, + _("port %u out of range in USB address bus: %u port: %s"), + info->addr.usb.port[i], + info->addr.usb.bus, + portStr); + return NULL; + } + hub = hub->hubs[portIdx]; + } + + *targetIdx = info->addr.usb.port[lastIdx] - 1; + return hub; +} + + +#define VIR_DOMAIN_USB_HUB_PORTS 8 + +static int +virDomainUSBAddressSetAddHub(virDomainUSBAddressSetPtr addrs, + virDomainHubDefPtr hub) +{ + virDomainUSBAddressHubPtr targetHub = NULL, newHub = NULL; + int ret = -1; + int targetPort; + char *portStr = NULL; + + if (hub->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Wrong address type for USB hub")); + goto cleanup; + } + + if (!(portStr = virDomainUSBAddressPortFormat(hub->info.addr.usb.port))) + goto cleanup; + + VIR_DEBUG("Adding a USB hub with 8 ports on bus=%u port=%s", + hub->info.addr.usb.bus, portStr); + + if (!(newHub = virDomainUSBAddressHubNew(VIR_DOMAIN_USB_HUB_PORTS))) + goto cleanup; + + if (!(targetHub = virDomainUSBAddressFindPort(addrs, &(hub->info), &targetPort, + portStr))) + goto cleanup; + + if (targetHub->hubs[targetPort]) { + virReportError(VIR_ERR_XML_ERROR, + _("Dupicate USB hub on bus %u port %s"), + hub->info.addr.usb.bus, portStr); + goto cleanup; + } + ignore_value(virBitmapSetBit(targetHub->ports, targetPort)); + targetHub->hubs[targetPort] = newHub; + newHub = NULL; + + ret = 0; + cleanup: + virDomainUSBAddressHubFree(newHub); + VIR_FREE(portStr); + return ret; +} + + int virDomainUSBAddressSetAddControllers(virDomainUSBAddressSetPtr addrs, virDomainDefPtr def) { @@ -1442,5 +1545,16 @@ int virDomainUSBAddressSetAddControllers(virDomainUSBAddressSetPtr addrs, return -1; } } + + for (i = 0; i < def->nhubs; i++) { + virDomainHubDefPtr hub = def->hubs[i]; + if (hub->type == VIR_DOMAIN_HUB_TYPE_USB && + hub->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) { + /* USB hubs that do not yet have an USB address have to be + * dealt with later */ + if (virDomainUSBAddressSetAddHub(addrs, hub) < 0) + return -1; + } + } return 0; } -- 2.7.3

Check if they fit on the USB controllers the domain has, and error out if two devices try to use the same address. --- src/conf/domain_addr.c | 39 ++++++++++++++++++++++ src/conf/domain_addr.h | 4 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_domain.h | 1 + src/qemu/qemu_domain_address.c | 38 ++++++++++++++++++++- .../qemuxml2argv-usb-hub-conflict.xml | 22 ++++++++++++ tests/qemuxml2argvtest.c | 3 ++ 7 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-hub-conflict.xml diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 343ae0c..853f4ce 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -1558,3 +1558,42 @@ int virDomainUSBAddressSetAddControllers(virDomainUSBAddressSetPtr addrs, } return 0; } + + +int +virDomainUSBAddressReserve(virDomainDeviceInfoPtr info, + void *data) +{ + virDomainUSBAddressSetPtr addrs = data; + virDomainUSBAddressHubPtr targetHub = NULL; + char *portStr = NULL; + int ret = -1; + int targetPort; + + if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) + return 0; + + portStr = virDomainUSBAddressPortFormat(info->addr.usb.port); + if (!portStr) + goto cleanup; + VIR_DEBUG("Reserving USB address bus=%u port=%s", info->addr.usb.bus, portStr); + + if (!(targetHub = virDomainUSBAddressFindPort(addrs, info, &targetPort, + portStr))) + goto cleanup; + + if (virBitmapIsBitSet(targetHub->ports, targetPort)) { + virReportError(VIR_ERR_XML_ERROR, + _("Duplicate USB address bus %u port %s"), + info->addr.usb.bus, portStr); + goto cleanup; + } + + ignore_value(virBitmapSetBit(targetHub->ports, targetPort)); + + ret = 0; + + cleanup: + VIR_FREE(portStr); + return ret; +} diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index 693c5ee..0308dd4 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -271,4 +271,8 @@ int virDomainUSBAddressSetAddControllers(virDomainUSBAddressSetPtr addrs, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); void virDomainUSBAddressSetFree(virDomainUSBAddressSetPtr addrs); +int +virDomainUSBAddressReserve(virDomainDeviceInfoPtr info, + void *data) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); #endif /* __DOMAIN_ADDR_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9e99216..72a1048 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -111,6 +111,7 @@ virDomainPCIAddressValidate; virDomainPCIControllerModelToConnectType; virDomainUSBAddressPortFormat; virDomainUSBAddressPortFormatBuf; +virDomainUSBAddressReserve; virDomainUSBAddressSetAddControllers; virDomainUSBAddressSetCreate; virDomainUSBAddressSetFree; diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 2443e97..e2141ed 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -190,6 +190,7 @@ struct _qemuDomainObjPrivate { virDomainPCIAddressSetPtr pciaddrs; virDomainCCWAddressSetPtr ccwaddrs; virDomainVirtioSerialAddrSetPtr vioserialaddrs; + virDomainUSBAddressSetPtr usbaddrs; virQEMUCapsPtr qemuCaps; char *lockState; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index d9d71e7..4af902b 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -1625,11 +1625,44 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, } +static int +qemuDomainAssignUSBAddresses(virDomainDefPtr def, + virDomainObjPtr obj) +{ + int ret = -1; + virDomainUSBAddressSetPtr addrs = NULL; + qemuDomainObjPrivatePtr priv = NULL; + + if (!(addrs = virDomainUSBAddressSetCreate())) + goto cleanup; + + if (virDomainUSBAddressSetAddControllers(addrs, def) < 0) + goto cleanup; + + if (virDomainUSBDeviceDefForeach(def, virDomainUSBAddressReserve, addrs, + true) < 0) + goto cleanup; + + VIR_DEBUG("Existing USB addresses have been reserved"); + + if (obj && obj->privateData) { + priv = obj->privateData; + priv->usbaddrs = addrs; + addrs = NULL; + } + ret = 0; + + cleanup: + virDomainUSBAddressSetFree(addrs); + return ret; +} + + int qemuDomainAssignAddresses(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, virDomainObjPtr obj, - bool newDomain ATTRIBUTE_UNUSED) + bool newDomain) { if (qemuDomainAssignVirtioSerialAddresses(def, obj) < 0) return -1; @@ -1645,6 +1678,9 @@ qemuDomainAssignAddresses(virDomainDefPtr def, if (qemuDomainAssignPCIAddresses(def, qemuCaps, obj) < 0) return -1; + if (newDomain && qemuDomainAssignUSBAddresses(def, obj) < 0) + return -1; + return 0; } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-hub-conflict.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb-hub-conflict.xml new file mode 100644 index 0000000..9a48ba0 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-hub-conflict.xml @@ -0,0 +1,22 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <devices> + <emulator>/usr/bin/qemu</emulator> + <controller type='usb' index='0'/> + <memballoon model='virtio'/> + <hub type='usb'> + <address type='usb' bus='0' port='1'/> + </hub> + <input type='mouse' bus='usb'> + <address type='usb' bus='0' port='1'/> + </input> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index a4b8bf4..52ca5ff 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1159,6 +1159,9 @@ mymain(void) DO_TEST("usb-hub", QEMU_CAPS_CHARDEV, QEMU_CAPS_USB_HUB, QEMU_CAPS_NODEFCONFIG); + DO_TEST_PARSE_ERROR("usb-hub-conflict", + QEMU_CAPS_CHARDEV, QEMU_CAPS_USB_HUB, + QEMU_CAPS_NODEFCONFIG); DO_TEST("usb-ports", QEMU_CAPS_CHARDEV, QEMU_CAPS_USB_HUB, QEMU_CAPS_NODEFCONFIG); -- 2.7.3

On 23/06/16 09:45, Ján Tomko wrote:
Check if they fit on the USB controllers the domain has, and error out if two devices try to use the same address. --- src/conf/domain_addr.c | 39 ++++++++++++++++++++++ src/conf/domain_addr.h | 4 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_domain.h | 1 + src/qemu/qemu_domain_address.c | 38 ++++++++++++++++++++- .../qemuxml2argv-usb-hub-conflict.xml | 22 ++++++++++++ tests/qemuxml2argvtest.c | 3 ++ 7 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-hub-conflict.xml
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 343ae0c..853f4ce 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -1558,3 +1558,42 @@ int virDomainUSBAddressSetAddControllers(virDomainUSBAddressSetPtr addrs, } return 0; } + + +int +virDomainUSBAddressReserve(virDomainDeviceInfoPtr info, + void *data) +{ + virDomainUSBAddressSetPtr addrs = data; + virDomainUSBAddressHubPtr targetHub = NULL; + char *portStr = NULL; + int ret = -1; + int targetPort; + + if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) + return 0; + + portStr = virDomainUSBAddressPortFormat(info->addr.usb.port);
So when you leave out the *port* attribute, and attempt to define such domain would result in an unknown error, yet defining such a domain worked previously just fine (yes, there were/are other issues with that). But you already realized this, since you posted a patch (https://www.redhat.com/archives/libvir-list/2016-June/msg01973.html) to make the port attribute mandatory (See my comment there). Compared to the behaviour of PCI address assignment, one would assume that if you omit an attribute that the docs does not specify as mandatory, everything will turn out just fine for the USB addresses, as it is the case with PCI addresses. Erik

Automatically assign addresses to USB devices. Just like reserving, this is only done for newly defined domains. https://bugzilla.redhat.com/show_bug.cgi?id=1215968 --- src/conf/domain_addr.c | 98 +++++++++++++++++++++- src/conf/domain_addr.h | 14 ++++ src/libvirt_private.syms | 3 + src/qemu/qemu_domain_address.c | 64 ++++++++++++++ .../qemuxml2argvdata/qemuxml2argv-bios-nvram.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-bios.args | 2 +- .../qemuxml2argv-controller-order.args | 8 +- .../qemuxml2argv-disk-usb-device-removable.args | 3 +- .../qemuxml2argv-disk-usb-device.args | 2 +- .../qemuxml2argv-graphics-spice-timeout.args | 2 +- .../qemuxml2argv-graphics-spice-usb-redir.args | 2 +- ...muxml2argv-hostdev-usb-address-device-boot.args | 2 +- .../qemuxml2argv-hostdev-usb-address-device.args | 2 +- .../qemuxml2argv-hostdev-usb-address.args | 2 +- .../qemuxml2argv-hugepages-numa.args | 6 +- .../qemuxml2argv-input-usbmouse.args | 2 +- .../qemuxml2argv-input-usbtablet.args | 2 +- .../qemuxml2argv-pseries-usb-kbd.args | 2 +- .../qemuxml2argv-serial-spiceport.args | 2 +- .../qemuxml2argv-smartcard-controller.args | 2 +- .../qemuxml2argv-smartcard-host-certificates.args | 2 +- .../qemuxml2argv-smartcard-host.args | 2 +- ...emuxml2argv-smartcard-passthrough-spicevmc.args | 2 +- .../qemuxml2argv-smartcard-passthrough-tcp.args | 2 +- .../qemuxml2argv-sound-device.args | 2 +- .../qemuxml2argv-usb-hub-conflict.args | 25 ++++++ .../qemuxml2argv-usb-port-autoassign.args | 28 +++++++ .../qemuxml2argv-usb-port-autoassign.xml | 27 ++++++ .../qemuxml2argv-usb-redir-boot.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-usb-redir.args | 2 +- tests/qemuxml2argvtest.c | 3 + 31 files changed, 290 insertions(+), 29 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-hub-conflict.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-port-autoassign.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-port-autoassign.xml diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 853f4ce..74bbbb3 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -1487,7 +1487,7 @@ virDomainUSBAddressFindPort(virDomainUSBAddressSetPtr addrs, #define VIR_DOMAIN_USB_HUB_PORTS 8 -static int +int virDomainUSBAddressSetAddHub(virDomainUSBAddressSetPtr addrs, virDomainHubDefPtr hub) { @@ -1560,6 +1560,86 @@ int virDomainUSBAddressSetAddControllers(virDomainUSBAddressSetPtr addrs, } +static int +virDomainUSBAddressFindFreePort(virDomainUSBAddressHubPtr hub, + unsigned int *portpath, + unsigned int level) +{ + unsigned int port; + ssize_t portIdx; + size_t i; + + /* Look for free ports on the current hub */ + if ((portIdx = virBitmapNextClearBit(hub->ports, -1)) >= 0) { + port = portIdx + 1; + VIR_DEBUG("Found a free port %u at level %u", port, level); + portpath[level] = port; + return 0; + } + + VIR_DEBUG("No ports found on hub %p, trying the hubs on it", hub); + + if (level >= VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH - 1) + return -1; + + /* Recursively search through the ports that contain another hub */ + for (i = 0; i < hub->nports; i++) { + if (!hub->hubs[i]) + continue; + + port = i + 1; + VIR_DEBUG("Looking at USB hub at level: %u port: %u", level, port); + if (virDomainUSBAddressFindFreePort(hub->hubs[i], portpath, + level + 1) < 0) + continue; + + portpath[level] = port; + return 0; + } + return -1; +} + + +int +virDomainUSBAddressAssign(virDomainUSBAddressSetPtr addrs, + virDomainDeviceInfoPtr info) +{ + unsigned int portpath[VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH] = { 0 }; + char *portStr = NULL; + size_t i; + int ret = -1; + + for (i = 0; i < addrs->nbuses; i++) { + virDomainUSBAddressHubPtr hub = addrs->buses[i]; + if (!hub) + continue; + + if (virDomainUSBAddressFindFreePort(hub, portpath, 0) < 0) + continue; + + /* we found a free port */ + if (!(portStr = virDomainUSBAddressPortFormat(portpath))) + goto cleanup; + + info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB; + info->addr.usb.bus = i; + memcpy(info->addr.usb.port, portpath, sizeof(portpath)); + VIR_DEBUG("Assigning USB addr bus=%u port=%s", + info->addr.usb.bus, portStr); + VIR_FREE(portStr); + if (virDomainUSBAddressReserve(info, addrs) < 0) + goto cleanup; + + return 0; + } + + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("No free USB ports")); + cleanup: + VIR_FREE(portStr); + return ret; +} + + int virDomainUSBAddressReserve(virDomainDeviceInfoPtr info, void *data) @@ -1597,3 +1677,19 @@ virDomainUSBAddressReserve(virDomainDeviceInfoPtr info, VIR_FREE(portStr); return ret; } + + +int +virDomainUSBAddressEnsure(virDomainUSBAddressSetPtr addrs, + virDomainDeviceInfoPtr info) +{ + if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + if (virDomainUSBAddressAssign(addrs, info) < 0) + return -1; + } else if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) { + if (virDomainUSBAddressReserve(info, addrs) < 0) + return -1; + } + + return 0; +} diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index 0308dd4..cd8501f 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -269,10 +269,24 @@ virDomainUSBAddressSetPtr virDomainUSBAddressSetCreate(void); int virDomainUSBAddressSetAddControllers(virDomainUSBAddressSetPtr addrs, virDomainDefPtr def) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +int +virDomainUSBAddressSetAddHub(virDomainUSBAddressSetPtr addrs, + virDomainHubDefPtr hub) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); void virDomainUSBAddressSetFree(virDomainUSBAddressSetPtr addrs); int virDomainUSBAddressReserve(virDomainDeviceInfoPtr info, void *data) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + +int +virDomainUSBAddressAssign(virDomainUSBAddressSetPtr addrs, + virDomainDeviceInfoPtr info) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +int +virDomainUSBAddressEnsure(virDomainUSBAddressSetPtr addrs, + virDomainDeviceInfoPtr info) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); #endif /* __DOMAIN_ADDR_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 72a1048..f8776d6 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -109,10 +109,13 @@ virDomainPCIAddressSetGrow; virDomainPCIAddressSlotInUse; virDomainPCIAddressValidate; virDomainPCIControllerModelToConnectType; +virDomainUSBAddressAssign; +virDomainUSBAddressEnsure; virDomainUSBAddressPortFormat; virDomainUSBAddressPortFormatBuf; virDomainUSBAddressReserve; virDomainUSBAddressSetAddControllers; +virDomainUSBAddressSetAddHub; virDomainUSBAddressSetCreate; virDomainUSBAddressSetFree; virDomainVirtioSerialAddrAssign; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 4af902b..2406edf 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -1625,6 +1625,62 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, } +struct qemuAssignUSBIteratorInfo { + virDomainUSBAddressSetPtr addrs; + size_t count; +}; + + +static int +qemuDomainAssignUSBPortsIterator(virDomainDeviceInfoPtr info, + void *opaque) +{ + struct qemuAssignUSBIteratorInfo *data = opaque; + + if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) + return 0; + + return virDomainUSBAddressAssign(data->addrs, info); +} + + +static int +qemuDomainAssignUSBHubs(virDomainUSBAddressSetPtr addrs, + virDomainDefPtr def) +{ + size_t i; + + for (i = 0; i < def->nhubs; i++) { + virDomainHubDefPtr hub = def->hubs[i]; + if (hub->type != VIR_DOMAIN_HUB_TYPE_USB) + continue; + + if (hub->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) + continue; + if (virDomainUSBAddressAssign(addrs, &hub->info) < 0) + return -1; + + if (virDomainUSBAddressSetAddHub(addrs, hub) < 0) + return -1; + } + + return 0; +} + + +static int +qemuDomainAssignUSBPorts(virDomainUSBAddressSetPtr addrs, + virDomainDefPtr def) +{ + struct qemuAssignUSBIteratorInfo data = { .addrs = addrs }; + + return virDomainUSBDeviceDefForeach(def, + qemuDomainAssignUSBPortsIterator, + &data, + true); +} + + static int qemuDomainAssignUSBAddresses(virDomainDefPtr def, virDomainObjPtr obj) @@ -1645,6 +1701,14 @@ qemuDomainAssignUSBAddresses(virDomainDefPtr def, VIR_DEBUG("Existing USB addresses have been reserved"); + if (qemuDomainAssignUSBHubs(addrs, def) < 0) + goto cleanup; + + if (qemuDomainAssignUSBPorts(addrs, def) < 0) + goto cleanup; + + VIR_DEBUG("Finished assigning USB ports"); + if (obj && obj->privateData) { priv = obj->privateData; priv->usbaddrs = addrs; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.args b/tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.args index fe4e419..848a029 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.args @@ -21,5 +21,5 @@ QEMU_AUDIO_DRV=none \ -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ -serial pty \ --device usb-tablet,id=input0 \ +-device usb-tablet,id=input0,bus=usb.0,port=1 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-bios.args b/tests/qemuxml2argvdata/qemuxml2argv-bios.args index 012af85..604b871 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-bios.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-bios.args @@ -22,5 +22,5 @@ QEMU_AUDIO_DRV=none \ -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ -serial pty \ --device usb-tablet,id=input0 \ +-device usb-tablet,id=input0,bus=usb.0,port=1 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-controller-order.args b/tests/qemuxml2argvdata/qemuxml2argv-controller-order.args index 70f3fdb..7b98beb 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-controller-order.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-controller-order.args @@ -19,8 +19,8 @@ nowait \ -boot order=cna,menu=off \ -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x7 \ --device usb-ccid,id=ccid0 \ --device usb-hub,id=hub0 \ +-device usb-ccid,id=ccid0,bus=usb.0,port=1.1 \ +-device usb-hub,id=hub0,bus=usb.0,port=1 \ -drive file=/tmp/fdr.img,format=raw,if=none,id=drive-virtio-disk0,cache=none,\ aio=native \ -device virtio-blk-pci,bus=pci.0,addr=0x5,drive=drive-virtio-disk0,\ @@ -37,10 +37,10 @@ media=cdrom,id=drive-ide0-1-0,readonly=on \ -chardev spicevmc,id=charchannel0,name=vdagent \ -device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,\ id=channel0,name=com.redhat.spice.0 \ --device usb-tablet,id=input0 \ +-device usb-tablet,id=input0,bus=usb.0,port=1.2 \ -spice port=5901,tls-port=5902,addr=0.0.0.0,x509-dir=/etc/pki/libvirt-spice \ -vga cirrus \ -device intel-hda,id=sound0,bus=pci.0,addr=0x4 \ -device hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 \ --device usb-host,hostbus=14,hostaddr=6,id=hostdev0 \ +-device usb-host,hostbus=14,hostaddr=6,id=hostdev0,bus=usb.0,port=2 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.args index 63e2bb2..7cda592 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.args @@ -21,5 +21,6 @@ QEMU_AUDIO_DRV=none \ -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ -drive file=/tmp/usbdisk.img,format=raw,if=none,id=drive-usb-disk0 \ --device usb-storage,drive=drive-usb-disk0,id=usb-disk0,removable=on \ +-device usb-storage,bus=usb.0,port=1,drive=drive-usb-disk0,id=usb-disk0,\ +removable=on \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device.args index 5d1ea98..03ef44f 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device.args @@ -21,5 +21,5 @@ QEMU_AUDIO_DRV=none \ -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ -drive file=/tmp/usbdisk.img,format=raw,if=none,id=drive-usb-disk0 \ --device usb-storage,drive=drive-usb-disk0,id=usb-disk0 \ +-device usb-storage,bus=usb.0,port=1,drive=drive-usb-disk0,id=usb-disk0 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.args index c0be4ee..cead7d6 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.args @@ -28,7 +28,7 @@ media=cdrom,id=drive-ide0-1-0,readonly=on \ -device rtl8139,vlan=0,id=net0,mac=52:54:00:71:70:89,bus=pci.0,addr=0x7 \ -net tap,fd=3,vlan=0,name=hostnet0 \ -serial pty \ --device usb-tablet,id=input0 \ +-device usb-tablet,id=input0,bus=usb.0,port=1 \ -spice port=5900,addr=127.0.0.1 \ -vga std \ -device AC97,id=sound0,bus=pci.0,addr=0x3 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.args index fa248b3..3f00da4 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.args @@ -30,7 +30,7 @@ zlib-glz-wan-compression=auto,playback-compression=on,streaming-video=filter,\ disable-copy-paste \ -vga cirrus \ -chardev socket,id=charredir0,host=localhost,port=4000 \ --device usb-redir,chardev=charredir0,id=redir0 \ +-device usb-redir,chardev=charredir0,id=redir0,bus=usb.0,port=1 \ -chardev spicevmc,id=charredir1,name=usbredir \ -device usb-redir,chardev=charredir1,id=redir1,bus=usb.0,port=4 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address-device-boot.args b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address-device-boot.args index 8c00055..86f394b 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address-device-boot.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address-device-boot.args @@ -19,5 +19,5 @@ QEMU_AUDIO_DRV=none \ -usb \ -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ --device usb-host,hostbus=14,hostaddr=6,id=hostdev0,bootindex=1 \ +-device usb-host,hostbus=14,hostaddr=6,id=hostdev0,bootindex=1,bus=usb.0,port=1 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address-device.args b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address-device.args index b5e6834..7883c61 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address-device.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address-device.args @@ -20,5 +20,5 @@ QEMU_AUDIO_DRV=none \ -usb \ -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ --device usb-host,hostbus=14,hostaddr=6,id=hostdev0 \ +-device usb-host,hostbus=14,hostaddr=6,id=hostdev0,bus=usb.0,port=1 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address.args b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address.args index bb5d55a..d1c3e8f 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address.args @@ -19,4 +19,4 @@ QEMU_AUDIO_DRV=none \ -usb \ -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ --device usb-host,hostbus=14,hostaddr=6,id=hostdev0 +-device usb-host,hostbus=14,hostaddr=6,id=hostdev0,bus=usb.0,port=1 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.args index c5a9e53..5c356ef 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.args @@ -46,7 +46,7 @@ id=channel0,name=org.qemu.guest_agent.0 \ -chardev spicevmc,id=charchannel1,name=vdagent \ -device virtserialport,bus=virtio-serial0.0,nr=2,chardev=charchannel1,\ id=channel1,name=com.redhat.spice.0 \ --device usb-tablet,id=input0 \ +-device usb-tablet,id=input0,bus=usb.0,port=1 \ -spice port=5901,tls-port=5902,addr=127.0.0.1,x509-dir=/etc/pki/libvirt-spice \ -vga qxl \ -global qxl-vga.ram_size=67108864 \ @@ -54,7 +54,7 @@ id=channel1,name=com.redhat.spice.0 \ -device intel-hda,id=sound0,bus=pci.0,addr=0x4 \ -device hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 \ -chardev spicevmc,id=charredir0,name=usbredir \ --device usb-redir,chardev=charredir0,id=redir0 \ +-device usb-redir,chardev=charredir0,id=redir0,bus=usb.0,port=2 \ -chardev spicevmc,id=charredir1,name=usbredir \ --device usb-redir,chardev=charredir1,id=redir1 \ +-device usb-redir,chardev=charredir1,id=redir1,bus=usb.0,port=3 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x8 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse.args b/tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse.args index bd0e5c6..df96e6a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse.args @@ -19,4 +19,4 @@ QEMU_AUDIO_DRV=none \ -usb \ -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ --device usb-mouse,id=input0 +-device usb-mouse,id=input0,bus=usb.0,port=1 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-input-usbtablet.args b/tests/qemuxml2argvdata/qemuxml2argv-input-usbtablet.args index 294515f..faf21d5 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-input-usbtablet.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-input-usbtablet.args @@ -19,4 +19,4 @@ QEMU_AUDIO_DRV=none \ -usb \ -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ --device usb-tablet,id=input0 +-device usb-tablet,id=input0,bus=usb.0,port=1 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-kbd.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-kbd.args index 25c16cb..5887616 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-kbd.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-kbd.args @@ -22,4 +22,4 @@ server,nowait \ -device pci-ohci,id=usb,bus=pci,addr=0x1 \ -chardev pty,id=charserial0 \ -device spapr-vty,chardev=charserial0,reg=0x30000000 \ --device usb-kbd,id=input0 +-device usb-kbd,id=input0,bus=usb.0,port=1 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.args b/tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.args index 246e854..f05c3f2 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.args @@ -23,7 +23,7 @@ server,nowait \ -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ -chardev spiceport,id=charserial0,name=org.qemu.console.serial.0 \ -device isa-serial,chardev=charserial0,id=serial0 \ --device usb-tablet,id=input0 \ +-device usb-tablet,id=input0,bus=usb.0,port=1 \ -spice port=5903,tls-port=5904,addr=127.0.0.1,x509-dir=/etc/pki/libvirt-spice \ -device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,bus=pci.0,\ addr=0x2 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-controller.args b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-controller.args index d3135c2..beb2935 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-controller.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-controller.args @@ -19,7 +19,7 @@ server,nowait \ -mon chardev=charmonitor,id=monitor,mode=readline \ -no-acpi \ -boot c \ --device usb-ccid,id=ccid0 \ +-device usb-ccid,id=ccid0,bus=usb.0,port=1 \ -usb \ -device ccid-card-emulated,backend=nss-emulated,id=smartcard0,bus=ccid0.0 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host-certificates.args b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host-certificates.args index 09ef26c..72cf24b 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host-certificates.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host-certificates.args @@ -19,7 +19,7 @@ server,nowait \ -mon chardev=charmonitor,id=monitor,mode=readline \ -no-acpi \ -boot c \ --device usb-ccid,id=ccid0 \ +-device usb-ccid,id=ccid0,bus=usb.0,port=1 \ -usb \ -device ccid-card-emulated,backend=certificates,cert1=cert1,cert2=cert2,\ cert3=cert3,db=/etc/pki/nssdb,id=smartcard0,bus=ccid0.0 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host.args b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host.args index d3135c2..beb2935 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host.args @@ -19,7 +19,7 @@ server,nowait \ -mon chardev=charmonitor,id=monitor,mode=readline \ -no-acpi \ -boot c \ --device usb-ccid,id=ccid0 \ +-device usb-ccid,id=ccid0,bus=usb.0,port=1 \ -usb \ -device ccid-card-emulated,backend=nss-emulated,id=smartcard0,bus=ccid0.0 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-spicevmc.args b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-spicevmc.args index b618507..cdca4c4 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-spicevmc.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-spicevmc.args @@ -19,7 +19,7 @@ server,nowait \ -mon chardev=charmonitor,id=monitor,mode=readline \ -no-acpi \ -boot c \ --device usb-ccid,id=ccid0 \ +-device usb-ccid,id=ccid0,bus=usb.0,port=1 \ -usb \ -chardev spicevmc,id=charsmartcard0,name=smartcard \ -device ccid-card-passthru,chardev=charsmartcard0,id=smartcard0,bus=ccid0.0 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-tcp.args b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-tcp.args index e0fcb49..0c526c8 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-tcp.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-tcp.args @@ -19,7 +19,7 @@ server,nowait \ -mon chardev=charmonitor,id=monitor,mode=readline \ -no-acpi \ -boot c \ --device usb-ccid,id=ccid0 \ +-device usb-ccid,id=ccid0,bus=usb.0,port=1 \ -usb \ -chardev socket,id=charsmartcard0,host=127.0.0.1,port=2001,server,nowait \ -device ccid-card-passthru,chardev=charsmartcard0,id=smartcard0,bus=ccid0.0 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-sound-device.args b/tests/qemuxml2argvdata/qemuxml2argv-sound-device.args index 8d846a0..b084f4e 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-sound-device.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-sound-device.args @@ -34,5 +34,5 @@ QEMU_AUDIO_DRV=none \ -device ich9-intel-hda,id=sound7,bus=pci.0,addr=0x8 \ -device hda-micro,id=sound7-codec0,bus=sound7.0,cad=0 \ -device hda-duplex,id=sound7-codec1,bus=sound7.0,cad=1 \ --device usb-audio,id=sound8 \ +-device usb-audio,id=sound8,bus=usb.0,port=1 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x9 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-hub-conflict.args b/tests/qemuxml2argvdata/qemuxml2argv-usb-hub-conflict.args new file mode 100644 index 0000000..6ec2b85 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-hub-conflict.args @@ -0,0 +1,25 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 214 \ +-smp 1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefconfig \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-usb \ +-device usb-hub,id=hub0,bus=usb.0,port=1 \ +-device usb-mouse,id=input0,bus=usb.0,port=1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-port-autoassign.args b/tests/qemuxml2argvdata/qemuxml2argv-usb-port-autoassign.args new file mode 100644 index 0000000..ac5cfdd --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-port-autoassign.args @@ -0,0 +1,28 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 214 \ +-smp 1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefconfig \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-usb \ +-device usb-hub,id=hub0,bus=usb.0,port=1 \ +-device usb-hub,id=hub1,bus=usb.0,port=2 \ +-device usb-mouse,id=input0,bus=usb.0,port=1.1 \ +-device usb-mouse,id=input1,bus=usb.0,port=1.2 \ +-device usb-mouse,id=input2,bus=usb.0,port=1.3 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-port-autoassign.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb-port-autoassign.xml new file mode 100644 index 0000000..a2fe34e --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-port-autoassign.xml @@ -0,0 +1,27 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <devices> + <emulator>/usr/bin/qemu</emulator> + <controller type='usb' index='0'/> + <memballoon model='virtio'/> + <hub type='usb'> + <address type='usb' bus='0' port='1'/> + </hub> + <input type='mouse' bus='usb'> + </input> + <hub type='usb'> + </hub> + <input type='mouse' bus='usb'> + </input> + <input type='mouse' bus='usb'> + </input> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-redir-boot.args b/tests/qemuxml2argvdata/qemuxml2argv-usb-redir-boot.args index 53b9040..bc47963 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-usb-redir-boot.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-redir-boot.args @@ -24,7 +24,7 @@ addr=0x4 \ -device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x4.0x1 \ -device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x4.0x2 \ -chardev socket,id=charredir0,host=localhost,port=4000 \ --device usb-redir,chardev=charredir0,id=redir0,bootindex=1 \ +-device usb-redir,chardev=charredir0,id=redir0,bootindex=1,bus=usb.0,port=1 \ -chardev spicevmc,id=charredir1,name=usbredir \ -device usb-redir,chardev=charredir1,id=redir1,bootindex=2,bus=usb.0,port=4 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-redir.args b/tests/qemuxml2argvdata/qemuxml2argv-usb-redir.args index 08e8f3e..0999c97 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-usb-redir.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-redir.args @@ -25,7 +25,7 @@ addr=0x4 \ -device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x4.0x1 \ -device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x4.0x2 \ -chardev socket,id=charredir0,host=localhost,port=4000 \ --device usb-redir,chardev=charredir0,id=redir0 \ +-device usb-redir,chardev=charredir0,id=redir0,bus=usb.0,port=1 \ -chardev spicevmc,id=charredir1,name=usbredir \ -device usb-redir,chardev=charredir1,id=redir1,bus=usb.0,port=4 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 52ca5ff..6ed0a36 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1165,6 +1165,9 @@ mymain(void) DO_TEST("usb-ports", QEMU_CAPS_CHARDEV, QEMU_CAPS_USB_HUB, QEMU_CAPS_NODEFCONFIG); + DO_TEST("usb-port-autoassign", + QEMU_CAPS_CHARDEV, QEMU_CAPS_USB_HUB, + QEMU_CAPS_NODEFCONFIG); DO_TEST("usb-redir", QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_USB_HUB, -- 2.7.3

USB disks, redirected devices, host devices and serial devices are supported. --- src/conf/domain_addr.c | 29 ++++++++++++++++++++++ src/conf/domain_addr.h | 4 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_domain_address.c | 5 ++++ src/qemu/qemu_hotplug.c | 27 ++++++++++++++++++++ .../qemuhotplug-hotplug-base-live+disk-usb.xml | 1 + 6 files changed, 67 insertions(+) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 74bbbb3..a754474 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -1693,3 +1693,32 @@ virDomainUSBAddressEnsure(virDomainUSBAddressSetPtr addrs, return 0; } + + +int +virDomainUSBAddressRelease(virDomainUSBAddressSetPtr addrs, + virDomainDeviceInfoPtr info) +{ + virDomainUSBAddressHubPtr targetHub = NULL; + char *portStr = NULL; + int targetPort; + int ret = -1; + + if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) + return 0; + + portStr = virDomainUSBAddressPortFormat(info->addr.usb.port); + VIR_DEBUG("Releasing USB addr bus=%u port=%s", info->addr.usb.bus, portStr); + + if (!(targetHub = virDomainUSBAddressFindPort(addrs, info, &targetPort, + portStr))) + goto cleanup; + + ignore_value(virBitmapClearBit(targetHub->ports, targetPort)); + + ret = 0; + + cleanup: + VIR_FREE(portStr); + return ret; +} diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index cd8501f..4d9cbe1 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -289,4 +289,8 @@ int virDomainUSBAddressEnsure(virDomainUSBAddressSetPtr addrs, virDomainDeviceInfoPtr info) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +int +virDomainUSBAddressRelease(virDomainUSBAddressSetPtr addrs, + virDomainDeviceInfoPtr info) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); #endif /* __DOMAIN_ADDR_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f8776d6..351cc4e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -113,6 +113,7 @@ virDomainUSBAddressAssign; virDomainUSBAddressEnsure; virDomainUSBAddressPortFormat; virDomainUSBAddressPortFormatBuf; +virDomainUSBAddressRelease; virDomainUSBAddressReserve; virDomainUSBAddressSetAddControllers; virDomainUSBAddressSetAddHub; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 2406edf..ced7853 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -1774,4 +1774,9 @@ qemuDomainReleaseDeviceAddress(virDomainObjPtr vm, virDomainVirtioSerialAddrRelease(priv->vioserialaddrs, info) < 0) VIR_WARN("Unable to release virtio-serial address on %s", NULLSTR(devstr)); + if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB && + priv->usbaddrs && + virDomainUSBAddressRelease(priv->usbaddrs, info) < 0) + VIR_WARN("Unable to release usb address on %s", + NULLSTR(devstr)); } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f695903..10b9e30 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -627,6 +627,13 @@ qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr driver, char *devstr = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); const char *src = virDomainDiskGetSource(disk); + bool releaseaddr; + + if (priv->usbaddrs) { + if (virDomainUSBAddressEnsure(priv->usbaddrs, &disk->info) < 0) + goto cleanup; + releaseaddr = true; + } if (qemuDomainPrepareDisk(driver, vm, disk, NULL, false) < 0) goto cleanup; @@ -672,6 +679,8 @@ qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr driver, virDomainDiskInsertPreAlloced(vm->def, disk); cleanup: + if (ret < 0 && releaseaddr) + virDomainUSBAddressRelease(priv->usbaddrs, &disk->info); VIR_FREE(devstr); VIR_FREE(drivestr); virObjectUnref(cfg); @@ -1473,6 +1482,12 @@ qemuDomainAttachChrDeviceAssignAddr(qemuDomainObjPrivatePtr priv, return -1; return 1; + } else if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && + chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB) { + if (virDomainUSBAddressEnsure(priv->usbaddrs, &chr->info) < 0) + return -1; + return 1; + } else if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL && chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO) { if (virDomainVirtioSerialAddrAutoAssign(NULL, priv->vioserialaddrs, @@ -1791,11 +1806,18 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver, { qemuDomainObjPrivatePtr priv = vm->privateData; char *devstr = NULL; + bool releaseaddr = false; bool added = false; bool teardowncgroup = false; bool teardownlabel = false; int ret = -1; + if (priv->usbaddrs) { + if (virDomainUSBAddressEnsure(priv->usbaddrs, hostdev->info) < 0) + goto cleanup; + releaseaddr = true; + } + if (qemuHostdevPrepareUSBDevices(driver, vm->def->name, &hostdev, 1, 0) < 0) goto cleanup; @@ -1841,6 +1863,8 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver, VIR_WARN("Unable to restore host device labelling on hotplug fail"); if (added) qemuHostdevReAttachUSBDevices(driver, vm->def->name, &hostdev, 1); + if (releaseaddr) + virDomainUSBAddressRelease(priv->usbaddrs, hostdev->info); } VIR_FREE(devstr); return ret; @@ -2834,6 +2858,8 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, dev.type = VIR_DOMAIN_DEVICE_DISK; dev.data.disk = disk; ignore_value(qemuRemoveSharedDevice(driver, &dev, vm->def->name)); + if (priv->usbaddrs) + virDomainUSBAddressRelease(priv->usbaddrs, &disk->info); virDomainDiskDefFree(disk); return 0; @@ -2930,6 +2956,7 @@ qemuDomainRemoveUSBHostDevice(virQEMUDriverPtr driver, virDomainHostdevDefPtr hostdev) { qemuHostdevReAttachUSBDevices(driver, vm->def->name, &hostdev, 1); + qemuDomainReleaseDeviceAddress(vm, hostdev->info, NULL); } static void diff --git a/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base-live+disk-usb.xml b/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base-live+disk-usb.xml index 41039a4..cd686e6 100644 --- a/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base-live+disk-usb.xml +++ b/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base-live+disk-usb.xml @@ -27,6 +27,7 @@ <readonly/> <shareable/> <alias name='usb-disk16'/> + <address type='usb' bus='0' port='1'/> </disk> <controller type='usb' index='0'> <alias name='usb'/> -- 2.7.3

When parsing a command line with USB devices that have no address specified, QEMU automatically adds a USB hub if the device would fill up all the available USB ports. To help most of the users, add one hub if there are more USB devices than available ports. For wilder configurations, expect the user to provide us with more hubs and/or controllers. --- src/conf/domain_addr.c | 20 +++++++++ src/conf/domain_addr.h | 2 + src/libvirt_private.syms | 1 + src/qemu/qemu_domain_address.c | 48 ++++++++++++++++++++++ .../qemuxml2argv-usb-hub-autoadd.args | 28 +++++++++++++ .../qemuxml2argv-usb-hub-autoadd.xml | 23 +++++++++++ tests/qemuxml2argvtest.c | 3 ++ 7 files changed, 125 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-hub-autoadd.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-hub-autoadd.xml diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index a754474..155d720 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -1600,6 +1600,26 @@ virDomainUSBAddressFindFreePort(virDomainUSBAddressHubPtr hub, } +size_t +virDomainUSBAddressCountAvailablePorts(virDomainDefPtr def) +{ + size_t i, ret = 0; + + for (i = 0; i < def->ncontrollers; i++) { + virDomainControllerDefPtr cont = def->controllers[i]; + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB) + ret += virDomainUSBAddressControllerModelToPorts(cont); + } + + for (i = 0; i < def->nhubs; i++) { + virDomainHubDefPtr hub = def->hubs[i]; + if (hub->type == VIR_DOMAIN_HUB_TYPE_USB) + ret += VIR_DOMAIN_USB_HUB_PORTS; + } + return ret; +} + + int virDomainUSBAddressAssign(virDomainUSBAddressSetPtr addrs, virDomainDeviceInfoPtr info) diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index 4d9cbe1..599400d 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -273,6 +273,8 @@ int virDomainUSBAddressSetAddHub(virDomainUSBAddressSetPtr addrs, virDomainHubDefPtr hub) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +size_t +virDomainUSBAddressCountAvailablePorts(virDomainDefPtr def); void virDomainUSBAddressSetFree(virDomainUSBAddressSetPtr addrs); int diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 351cc4e..325a70d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -110,6 +110,7 @@ virDomainPCIAddressSlotInUse; virDomainPCIAddressValidate; virDomainPCIControllerModelToConnectType; virDomainUSBAddressAssign; +virDomainUSBAddressCountAvailablePorts; virDomainUSBAddressEnsure; virDomainUSBAddressPortFormat; virDomainUSBAddressPortFormatBuf; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index ced7853..8368a35 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -1682,6 +1682,51 @@ qemuDomainAssignUSBPorts(virDomainUSBAddressSetPtr addrs, static int +qemuDomainAssignUSBPortsCounter(virDomainDeviceInfoPtr info ATTRIBUTE_UNUSED, + void *opaque) +{ + struct qemuAssignUSBIteratorInfo *data = opaque; + + data->count++; + return 0; +} + + +static int +qemuDomainUSBAddressAddHubs(virDomainDefPtr def) +{ + struct qemuAssignUSBIteratorInfo data = { .count = 0 }; + virDomainHubDefPtr hub = NULL; + size_t available_ports; + int ret = -1; + + available_ports = virDomainUSBAddressCountAvailablePorts(def); + ignore_value(virDomainUSBDeviceDefForeach(def, + qemuDomainAssignUSBPortsCounter, + &data, + false)); + VIR_DEBUG("Found %zu USB devices and %zu provided USB ports", + data.count, available_ports); + + /* Add one hub if there are more devices than ports + * otherwise it's up to the user to specify more hubs/controllers */ + if (data.count > available_ports) { + if (VIR_ALLOC(hub) < 0) + return -1; + hub->type = VIR_DOMAIN_HUB_TYPE_USB; + + if (VIR_APPEND_ELEMENT(def->hubs, def->nhubs, hub) < 0) + goto cleanup; + } + + ret = 0; + cleanup: + VIR_FREE(hub); + return ret; +} + + +static int qemuDomainAssignUSBAddresses(virDomainDefPtr def, virDomainObjPtr obj) { @@ -1692,6 +1737,9 @@ qemuDomainAssignUSBAddresses(virDomainDefPtr def, if (!(addrs = virDomainUSBAddressSetCreate())) goto cleanup; + if (qemuDomainUSBAddressAddHubs(def) < 0) + goto cleanup; + if (virDomainUSBAddressSetAddControllers(addrs, def) < 0) goto cleanup; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-hub-autoadd.args b/tests/qemuxml2argvdata/qemuxml2argv-usb-hub-autoadd.args new file mode 100644 index 0000000..12c9691 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-hub-autoadd.args @@ -0,0 +1,28 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 214 \ +-smp 1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefconfig \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-usb \ +-device usb-hub,id=hub0,bus=usb.0,port=1 \ +-device usb-mouse,id=input0,bus=usb.0,port=2 \ +-device usb-mouse,id=input1,bus=usb.0,port=1.1 \ +-device usb-mouse,id=input2,bus=usb.0,port=1.2 \ +-device usb-tablet,id=input3,bus=usb.0,port=1.3 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-hub-autoadd.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb-hub-autoadd.xml new file mode 100644 index 0000000..43e0f1f --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-hub-autoadd.xml @@ -0,0 +1,23 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <devices> + <emulator>/usr/bin/qemu</emulator> + <controller type='usb' index='0'/> + <memballoon model='virtio'/> + <input type='mouse' bus='usb'> + </input> + <input type='mouse' bus='usb'> + </input> + <input type='mouse' bus='usb'> + </input> + <input type='tablet' bus='usb'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 6ed0a36..a202bed 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1159,6 +1159,9 @@ mymain(void) DO_TEST("usb-hub", QEMU_CAPS_CHARDEV, QEMU_CAPS_USB_HUB, QEMU_CAPS_NODEFCONFIG); + DO_TEST("usb-hub-autoadd", + QEMU_CAPS_CHARDEV, QEMU_CAPS_USB_HUB, + QEMU_CAPS_NODEFCONFIG); DO_TEST_PARSE_ERROR("usb-hub-conflict", QEMU_CAPS_CHARDEV, QEMU_CAPS_USB_HUB, QEMU_CAPS_NODEFCONFIG); -- 2.7.3
participants (2)
-
Erik Skultety
-
Ján Tomko