9:52 a.m.
For https://bugzilla.redhat.com/show_bug.cgi?id=1215968
For domains started without the addresses specified on the command line,
QEMU makes up the addresses and adds USB hubs if the device is added
to the second-to-last USB port.
A domain started with these exact ports and hubs specified on the command line
is not compatible with this domain.
This series only generates the addresses for new domains, to avoid breaking
migrating already existing domains.
Ján Tomko (9):
Remove dead code from qemuDomainAttachControllerDevice
Remove unused virDomainVirtioSerialAddrSetRemoveController
Store USB port path as an array of integers
Add newDomain parameter to qemuDomainAssignAddresses
Introduce virDomainUSBAddressSet
Add functions for adding usb controllers to addrs
Reserve existing USB addresses
Assign addresses to USB devices
Assign addresses on USB device hotplug
src/conf/domain_addr.c | 406 +++++++++++++++++++--
src/conf/domain_addr.h | 53 ++-
src/conf/domain_conf.c | 29 +-
src/conf/domain_conf.h | 4 +-
src/libvirt_private.syms | 10 +-
src/qemu/qemu_command.c | 235 +++++++++++-
src/qemu/qemu_command.h | 3 +-
src/qemu/qemu_domain.h | 1 +
src/qemu/qemu_driver.c | 22 +-
src/qemu/qemu_hotplug.c | 57 ++-
src/qemu/qemu_process.c | 6 +-
tests/qemuhotplugtest.c | 2 +-
...qemuhotplug-console-compat-2+console-virtio.xml | 4 +-
.../qemuhotplug-hotplug-base+disk-usb.xml | 1 +
.../qemuxml2argvdata/qemuxml2argv-bios-nvram.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-bios.args | 2 +-
.../qemuxml2argv-console-compat-2.xml | 4 +-
.../qemuxml2argv-controller-order.args | 7 +-
.../qemuxml2argv-controller-order.xml | 2 +
.../qemuxml2argv-disk-usb-device-removable.args | 3 +-
.../qemuxml2argv-disk-usb-device.args | 3 +-
.../qemuxml2argv-graphics-spice-timeout.args | 2 +-
...muxml2argv-hostdev-usb-address-device-boot.args | 2 +-
.../qemuxml2argv-hostdev-usb-address-device.args | 3 +-
.../qemuxml2argv-hugepages-numa.args | 2 +-
.../qemuxml2argv-input-usbmouse-addr.args | 2 +-
.../qemuxml2argv-input-usbmouse-addr.xml | 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 | 3 +-
.../qemuxml2argv-smartcard-passthrough-tcp.args | 2 +-
.../qemuxml2argv-sound-device.args | 2 +-
.../qemuxml2argv-usb-hub-conflict.xml | 22 ++
.../qemuxml2argv-usb-port-autoassign.args | 15 +
.../qemuxml2argv-usb-port-autoassign.xml | 27 ++
tests/qemuxml2argvtest.c | 11 +-
tests/qemuxmlnstest.c | 2 +-
40 files changed, 854 insertions(+), 109 deletions(-)
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.4.6
9:52 a.m.
New subject: [libvirt] [PATCH 1/9] Remove dead code from qemuDomainAttachControllerDevice
We only support hotplugging SCSI controllers,
USB and virtio-serial related code is useless here.
---
src/qemu/qemu_hotplug.c | 18 ------------------
1 file changed, 18 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index aabdb78..8e38153 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -432,7 +432,6 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver,
char *devstr = NULL;
qemuDomainObjPrivatePtr priv = vm->privateData;
bool releaseaddr = false;
- bool addedToAddrSet = false;
if (virDomainControllerFind(vm->def, controller->type, controller->idx)
>= 0) {
virReportError(VIR_ERR_OPERATION_FAILED,
@@ -463,20 +462,6 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver,
if (qemuAssignDeviceControllerAlias(vm->def, priv->qemuCaps, controller)
< 0)
goto cleanup;
- if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
- controller->model == -1 &&
- !virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_PIIX3_USB_UHCI)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("USB controller hotplug unsupported in this QEMU
binary"));
- goto cleanup;
- }
-
- if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL &&
- virDomainVirtioSerialAddrSetAddController(priv->vioserialaddrs,
- controller) < 0)
- goto cleanup;
- addedToAddrSet = true;
-
if (!(devstr = qemuBuildControllerDevStr(vm->def, controller,
priv->qemuCaps, NULL)))
goto cleanup;
}
@@ -505,9 +490,6 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver,
}
cleanup:
- if (ret != 0 && addedToAddrSet)
- virDomainVirtioSerialAddrSetRemoveController(priv->vioserialaddrs,
- controller);
if (ret != 0 && releaseaddr)
qemuDomainReleaseDeviceAddress(vm, &controller->info, NULL);
--
2.4.6
12:39 p.m.
New subject: [libvirt] [PATCH 1/9] Remove dead code from qemuDomainAttachControllerDevice
On 08/12/2015 10:52 AM, Ján Tomko wrote:
We only support hotplugging SCSI controllers,
USB and virtio-serial related code is useless here.
---
src/qemu/qemu_hotplug.c | 18 ------------------
1 file changed, 18 deletions(-)
It's true that we only call this from two places currently:
1. qemuDomainAttachDeviceControllerLive (static)
-> Only when cont->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI
2. qemuDomainFindOrCreateSCSIDiskController (static)
-> Only via qemuDomainAttachSCSIDisk which only gets called when
(disk->bus == VIR_DOMAIN_DISK_BUS_SCSI
However, this is a global (ok somewhat global) function and thus could
erroneously be called by something else not realizing it's only
supported for SCSI.
So it seems to me perhaps the check and message from
qemuDomainAttachDeviceControllerLive could be moved to the top of this
function...
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index aabdb78..8e38153 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -432,7 +432,6 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver,
char *devstr = NULL;
qemuDomainObjPrivatePtr priv = vm->privateData;
bool releaseaddr = false;
- bool addedToAddrSet = false;
e.g. right here:
if (cont->type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
_("'%s' controller cannot be hot plugged."),
virDomainControllerTypeToString(cont->type));
return -1;
}
Then the other code just becomes:
return qemuDomainAttachControllerDevice(driver, vm, cont);
With the rest of this as is.
John
if (virDomainControllerFind(vm->def, controller->type,
controller->idx) >= 0) {
virReportError(VIR_ERR_OPERATION_FAILED,
@@ -463,20 +462,6 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver,
if (qemuAssignDeviceControllerAlias(vm->def, priv->qemuCaps, controller)
< 0)
goto cleanup;
- if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
- controller->model == -1 &&
- !virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_PIIX3_USB_UHCI)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("USB controller hotplug unsupported in this QEMU
binary"));
- goto cleanup;
- }
-
- if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL &&
- virDomainVirtioSerialAddrSetAddController(priv->vioserialaddrs,
- controller) < 0)
- goto cleanup;
- addedToAddrSet = true;
-
if (!(devstr = qemuBuildControllerDevStr(vm->def, controller,
priv->qemuCaps, NULL)))
goto cleanup;
}
@@ -505,9 +490,6 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver,
}
cleanup:
- if (ret != 0 && addedToAddrSet)
- virDomainVirtioSerialAddrSetRemoveController(priv->vioserialaddrs,
- controller);
if (ret != 0 && releaseaddr)
qemuDomainReleaseDeviceAddress(vm, &controller->info, NULL);
9:52 a.m.
New subject: [libvirt] [PATCH 2/9] Remove unused virDomainVirtioSerialAddrSetRemoveController
This function was never used.
Also mark virDomainVirtioSerialAddrSetAddController as static.
---
src/conf/domain_addr.c | 23 +----------------------
src/conf/domain_addr.h | 8 --------
src/libvirt_private.syms | 2 --
3 files changed, 1 insertion(+), 32 deletions(-)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index ca5803e..9883c4f 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -825,7 +825,7 @@
virDomainVirtioSerialAddrFindController(virDomainVirtioSerialAddrSetPtr addrs,
* Adds virtio serial ports of the existing controller
* to the address set.
*/
-int
+static int
virDomainVirtioSerialAddrSetAddController(virDomainVirtioSerialAddrSetPtr addrs,
virDomainControllerDefPtr cont)
{
@@ -884,27 +884,6 @@
virDomainVirtioSerialAddrSetAddControllers(virDomainVirtioSerialAddrSetPtr addrs
return 0;
}
-/* virDomainVirtioSerialAddrSetRemoveController
- *
- * Removes a virtio serial controller from the address set.
- */
-void
-virDomainVirtioSerialAddrSetRemoveController(virDomainVirtioSerialAddrSetPtr addrs,
- virDomainControllerDefPtr cont)
-{
- ssize_t pos;
-
- if (cont->type != VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL)
- return;
-
- VIR_DEBUG("Removing virtio serial controller index %u "
- "from the address set", cont->idx);
-
- pos = virDomainVirtioSerialAddrFindController(addrs, cont->idx);
-
- if (pos >= 0)
- VIR_DELETE_ELEMENT(addrs->controllers, pos, addrs->ncontrollers);
-}
void
virDomainVirtioSerialAddrSetFree(virDomainVirtioSerialAddrSetPtr addrs)
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 2220a79..208635c 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -206,14 +206,6 @@ typedef virDomainVirtioSerialAddrSet
*virDomainVirtioSerialAddrSetPtr;
virDomainVirtioSerialAddrSetPtr
virDomainVirtioSerialAddrSetCreate(void);
int
-virDomainVirtioSerialAddrSetAddController(virDomainVirtioSerialAddrSetPtr addrs,
- virDomainControllerDefPtr cont)
- ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
-void
-virDomainVirtioSerialAddrSetRemoveController(virDomainVirtioSerialAddrSetPtr addrs,
- virDomainControllerDefPtr cont)
- ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
-int
virDomainVirtioSerialAddrSetAddControllers(virDomainVirtioSerialAddrSetPtr addrs,
virDomainDefPtr def)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 45f42f5..f1e5f48 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -111,11 +111,9 @@ virDomainVirtioSerialAddrAutoAssign;
virDomainVirtioSerialAddrIsComplete;
virDomainVirtioSerialAddrRelease;
virDomainVirtioSerialAddrReserve;
-virDomainVirtioSerialAddrSetAddController;
virDomainVirtioSerialAddrSetAddControllers;
virDomainVirtioSerialAddrSetCreate;
virDomainVirtioSerialAddrSetFree;
-virDomainVirtioSerialAddrSetRemoveController;
# conf/domain_audit.h
--
2.4.6
12:46 p.m.
New subject: [libvirt] [PATCH 2/9] Remove unused virDomainVirtioSerialAddrSetRemoveController
On 08/12/2015 10:52 AM, Ján Tomko wrote:
This function was never used.
I think that portion could have been removed in 1/9 since that's when
you removed the unreachable 'external' call to it.
Also mark virDomainVirtioSerialAddrSetAddController as static.
And this just becomes 2/9
---
src/conf/domain_addr.c | 23 +----------------------
src/conf/domain_addr.h | 8 --------
src/libvirt_private.syms | 2 --
3 files changed, 1 insertion(+), 32 deletions(-)
But I understand why it's easier to do it one patch...
ACK,
John
9:52 a.m.
New subject: [libvirt] [PATCH 3/9] Store USB port path as an array of integers
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/domain_addr.c | 25 +++++++++++++++++++++++++
src/conf/domain_addr.h | 8 ++++++++
src/conf/domain_conf.c | 29 +++++++++++++++--------------
src/conf/domain_conf.h | 4 +++-
src/libvirt_private.syms | 2 ++
src/qemu/qemu_command.c | 3 ++-
6 files changed, 55 insertions(+), 16 deletions(-)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 9883c4f..a5d142d 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1173,3 +1173,28 @@ virDomainVirtioSerialAddrRelease(virDomainVirtioSerialAddrSetPtr
addrs,
VIR_FREE(str);
return ret;
}
+
+
+void
+virDomainUSBAddressGetPortBuf(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 *
+virDomainUSBAddressGetPortString(unsigned int *port)
+{
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ virDomainUSBAddressGetPortBuf(&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 208635c..c6d8da7 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -240,4 +240,12 @@ virDomainVirtioSerialAddrRelease(virDomainVirtioSerialAddrSetPtr
addrs,
virDomainDeviceInfoPtr info)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+void
+virDomainUSBAddressGetPortBuf(virBufferPtr buf,
+ unsigned int *port)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+char *
+virDomainUSBAddressGetPortString(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 f1e02e3..0526aee 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -33,6 +33,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"
@@ -3345,8 +3346,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);
@@ -4329,9 +4328,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);
+ virDomainUSBAddressGetPortBuf(buf, info->addr.usb.port);
+ virBufferAddLit(buf, "'");
break;
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO:
@@ -4567,27 +4567,28 @@ virDomainDeviceUSBAddressParseXML(xmlNodePtr node,
virDomainDeviceUSBAddressPtr addr)
{
char *port, *bus, *tmp;
- unsigned int p;
int ret = -1;
+ size_t i;
memset(addr, 0, sizeof(*addr));
port = virXMLPropString(node, "port");
bus = virXMLPropString(node, "bus");
- if (port &&
- ((virStrToLong_uip(port, &tmp, 10, &p) < 0 || (*tmp != '\0'
&& *tmp != '.')) ||
- (*tmp == '.' && (virStrToLong_ui(tmp + 1, &tmp, 10, &p)
< 0 || (*tmp != '\0' && *tmp != '.'))) ||
- (*tmp == '.' && (virStrToLong_ui(tmp + 1, &tmp, 10, &p)
< 0 || (*tmp != '\0' && *tmp != '.'))) ||
- (*tmp == '.' && (virStrToLong_ui(tmp + 1, &tmp, 10, &p)
< 0 || (*tmp != '\0'))))) {
+ for (i = 0, tmp = port;
+ tmp && *tmp != '\0' && i <
VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH;
+ i++) {
+ if (virStrToLong_uip(tmp, &tmp, 10, &addr->port[i]) < 0)
+ break;
+ if (*tmp == '.')
+ tmp++;
+ }
+ if (tmp && *tmp != '\0') {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'port'
attribute"));
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/conf/domain_conf.h b/src/conf/domain_conf.h
index 9762c4f..abd2cdb 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -294,11 +294,13 @@ struct _virDomainDeviceCcidAddress {
unsigned int slot;
};
+# define VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH 7
+
typedef struct _virDomainDeviceUSBAddress virDomainDeviceUSBAddress;
typedef virDomainDeviceUSBAddress *virDomainDeviceUSBAddressPtr;
struct _virDomainDeviceUSBAddress {
unsigned int bus;
- char *port;
+ unsigned int port[VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH];
};
typedef struct _virDomainDeviceSpaprVioAddress virDomainDeviceSpaprVioAddress;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f1e5f48..5168230 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -106,6 +106,8 @@ virDomainPCIAddressSetFree;
virDomainPCIAddressSetGrow;
virDomainPCIAddressSlotInUse;
virDomainPCIAddressValidate;
+virDomainUSBAddressGetPortBuf;
+virDomainUSBAddressGetPortString;
virDomainVirtioSerialAddrAssign;
virDomainVirtioSerialAddrAutoAssign;
virDomainVirtioSerialAddrIsComplete;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 84cbfe1..4e77279 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2796,7 +2796,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);
+ virDomainUSBAddressGetPortBuf(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.4.6
2:29 p.m.
New subject: [libvirt] [PATCH 3/9] Store USB port path as an array of integers
On 08/12/2015 10:52 AM, 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/domain_addr.c | 25 +++++++++++++++++++++++++
src/conf/domain_addr.h | 8 ++++++++
src/conf/domain_conf.c | 29 +++++++++++++++--------------
src/conf/domain_conf.h | 4 +++-
src/libvirt_private.syms | 2 ++
src/qemu/qemu_command.c | 3 ++-
6 files changed, 55 insertions(+), 16 deletions(-)
Hmm.. interesting we don't have any tests using dotted notation port
values. Although I did try and find it was successful on the xml parse
and print, it may be "useful" to create a test that uses 2, 3, and 4 dot
octets... and 5 for a test failure.
In order to test I modified
tests/qemuxml2argvdata/qemuxml2argv-usb-redir-filter.xml and
tests/qemuxml2xmloutdata/qemuxml2xmlout-usb-redir-filter.xml to replace
a port address with a 4 dot octet.
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 9883c4f..a5d142d 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1173,3 +1173,28 @@ virDomainVirtioSerialAddrRelease(virDomainVirtioSerialAddrSetPtr
addrs,
VIR_FREE(str);
return ret;
}
+
+
+void
+virDomainUSBAddressGetPortBuf(virBufferPtr buf,
+ unsigned int *port)
Perhaps virDomainUSBAddressPrintPortBuf ??
+{
+ 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 *
+virDomainUSBAddressGetPortString(unsigned int *port)
+{
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ virDomainUSBAddressGetPortBuf(&buf, port);
+ if (virBufferCheckError(&buf) < 0)
+ return NULL;
Peeking ahead to callers (none here, but future patches) - there needs
to be either NULLSTR(portstr) for all or a decision to fail if NULL is
returned.
+ return virBufferContentAndReset(&buf);
+}
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 208635c..c6d8da7 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -240,4 +240,12 @@ virDomainVirtioSerialAddrRelease(virDomainVirtioSerialAddrSetPtr
addrs,
virDomainDeviceInfoPtr info)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+void
+virDomainUSBAddressGetPortBuf(virBufferPtr buf,
+ unsigned int *port)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+char *
+virDomainUSBAddressGetPortString(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 f1e02e3..0526aee 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -33,6 +33,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"
@@ -3345,8 +3346,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);
@@ -4329,9 +4328,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);
+ virDomainUSBAddressGetPortBuf(buf, info->addr.usb.port);
+ virBufferAddLit(buf, "'");
break;
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO:
@@ -4567,27 +4567,28 @@ virDomainDeviceUSBAddressParseXML(xmlNodePtr node,
virDomainDeviceUSBAddressPtr addr)
{
char *port, *bus, *tmp;
- unsigned int p;
int ret = -1;
+ size_t i;
memset(addr, 0, sizeof(*addr));
port = virXMLPropString(node, "port");
bus = virXMLPropString(node, "bus");
- if (port &&
- ((virStrToLong_uip(port, &tmp, 10, &p) < 0 || (*tmp != '\0'
&& *tmp != '.')) ||
- (*tmp == '.' && (virStrToLong_ui(tmp + 1, &tmp, 10, &p)
< 0 || (*tmp != '\0' && *tmp != '.'))) ||
- (*tmp == '.' && (virStrToLong_ui(tmp + 1, &tmp, 10, &p)
< 0 || (*tmp != '\0' && *tmp != '.'))) ||
- (*tmp == '.' && (virStrToLong_ui(tmp + 1, &tmp, 10, &p)
< 0 || (*tmp != '\0'))))) {
+ for (i = 0, tmp = port;
+ tmp && *tmp != '\0' && i <
VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH;
+ i++) {
+ if (virStrToLong_uip(tmp, &tmp, 10, &addr->port[i]) < 0)
+ break;
+ if (*tmp == '.')
+ tmp++;
+ }
My eyes hurt (from both algorithms), but I can see from testing this
works, although I think VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH is wrong
since 1.2.3.4.5 parsed successfully.
+ if (tmp && *tmp != '\0') {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse <address> 'port'
attribute"));
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/conf/domain_conf.h b/src/conf/domain_conf.h
index 9762c4f..abd2cdb 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -294,11 +294,13 @@ struct _virDomainDeviceCcidAddress {
unsigned int slot;
};
+# define VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH 7
+
7? This is for the 4 octet usb port addr value, correct?
that is (from our docs)
type='usb'
USB addresses have the following additional attributes: bus (a hex
value between 0 and 0xfff, inclusive), and port (a dotted notation of up
to four octets, such as 1.2 or 2.1.3.1).
so shouldn't this be 4? Or is that changing too?
With a few things cleaned up this is ACK-able it seems - would be nice
to have a test with the multi-octet port address even though it's not
'required'...
John
typedef struct _virDomainDeviceUSBAddress
virDomainDeviceUSBAddress;
typedef virDomainDeviceUSBAddress *virDomainDeviceUSBAddressPtr;
struct _virDomainDeviceUSBAddress {
unsigned int bus;
- char *port;
+ unsigned int port[VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH];
};
typedef struct _virDomainDeviceSpaprVioAddress virDomainDeviceSpaprVioAddress;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f1e5f48..5168230 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -106,6 +106,8 @@ virDomainPCIAddressSetFree;
virDomainPCIAddressSetGrow;
virDomainPCIAddressSlotInUse;
virDomainPCIAddressValidate;
+virDomainUSBAddressGetPortBuf;
+virDomainUSBAddressGetPortString;
virDomainVirtioSerialAddrAssign;
virDomainVirtioSerialAddrAutoAssign;
virDomainVirtioSerialAddrIsComplete;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 84cbfe1..4e77279 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2796,7 +2796,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);
+ virDomainUSBAddressGetPortBuf(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);
9:52 a.m.
New subject: [libvirt] [PATCH 4/9] Add newDomain parameter to qemuDomainAssignAddresses
To differentiate a new domain from an existing one.
---
src/qemu/qemu_command.c | 5 ++++-
src/qemu/qemu_command.h | 3 ++-
src/qemu/qemu_driver.c | 22 +++++++++++-----------
src/qemu/qemu_process.c | 6 +++---
tests/qemuhotplugtest.c | 2 +-
tests/qemuxml2argvtest.c | 2 +-
tests/qemuxmlnstest.c | 2 +-
7 files changed, 23 insertions(+), 19 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4e77279..4b520d7 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1773,10 +1773,13 @@ qemuDomainPCIBusFullyReserved(virDomainPCIAddressBusPtr bus)
int qemuDomainAssignAddresses(virDomainDefPtr def,
virQEMUCapsPtr qemuCaps,
- virDomainObjPtr obj)
+ virDomainObjPtr obj,
+ bool newDomain ATTRIBUTE_UNUSED)
{
int rc;
+ VIR_DEBUG("def=%p obj=%p newDomain=%d", def, obj, newDomain);
+
rc = qemuDomainAssignVirtioSerialAddresses(def, obj);
if (rc)
return rc;
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index e356f5b..da9ea14 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -266,7 +266,8 @@ virDomainDefPtr qemuParseCommandLinePid(virCapsPtr qemuCaps,
int qemuDomainAssignAddresses(virDomainDefPtr def,
virQEMUCapsPtr qemuCaps,
- virDomainObjPtr obj)
+ virDomainObjPtr obj,
+ bool newDomain)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def,
virQEMUCapsPtr qemuCaps);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2e44500..02faf3e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1737,7 +1737,7 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn,
if (qemuCanonicalizeMachine(def, qemuCaps) < 0)
goto cleanup;
- if (qemuDomainAssignAddresses(def, qemuCaps, NULL) < 0)
+ if (qemuDomainAssignAddresses(def, qemuCaps, NULL, true) < 0)
goto cleanup;
if (!(vm = virDomainObjListAdd(driver->domains, def,
@@ -7246,7 +7246,7 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn,
if (qemuAssignDeviceAliases(def, qemuCaps) < 0)
goto cleanup;
- if (qemuDomainAssignAddresses(def, qemuCaps, NULL) < 0)
+ if (qemuDomainAssignAddresses(def, qemuCaps, NULL, true) < 0)
goto cleanup;
/* do fake auto-alloc of graphics ports, if such config is used */
@@ -7486,7 +7486,7 @@ static virDomainPtr qemuDomainDefineXMLFlags(virConnectPtr conn,
const char *xml
if (qemuCanonicalizeMachine(def, qemuCaps) < 0)
goto cleanup;
- if (qemuDomainAssignAddresses(def, qemuCaps, NULL) < 0)
+ if (qemuDomainAssignAddresses(def, qemuCaps, NULL, true) < 0)
goto cleanup;
if (!(vm = virDomainObjListAdd(driver->domains, def,
@@ -8036,7 +8036,7 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO)
if (virDomainDefAddImplicitControllers(vmdef) < 0)
return -1;
- if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0)
+ if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL, false) < 0)
return -1;
break;
@@ -8045,7 +8045,7 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
if (virDomainNetInsert(vmdef, net))
return -1;
dev->data.net = NULL;
- if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0)
+ if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL, false) < 0)
return -1;
break;
@@ -8061,7 +8061,7 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
dev->data.hostdev = NULL;
if (virDomainDefAddImplicitControllers(vmdef) < 0)
return -1;
- if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0)
+ if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL, false) < 0)
return -1;
break;
@@ -8093,7 +8093,7 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
return -1;
dev->data.controller = NULL;
- if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0)
+ if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL, false) < 0)
return -1;
break;
@@ -8103,7 +8103,7 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
dev->data.chr = NULL;
if (virDomainDefAddImplicitControllers(vmdef) < 0)
return -1;
- if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0)
+ if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL, false) < 0)
return -1;
break;
@@ -8132,7 +8132,7 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
return -1;
dev->data.rng = NULL;
- if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0)
+ if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL, false) < 0)
return -1;
break;
@@ -8371,7 +8371,7 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps,
vmdef->nets[pos] = net;
dev->data.net = NULL;
- if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0)
+ if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL, false) < 0)
return -1;
break;
@@ -15859,7 +15859,7 @@ static virDomainPtr qemuDomainQemuAttach(virConnectPtr conn,
if (qemuAssignDeviceAliases(def, qemuCaps) < 0)
goto cleanup;
- if (qemuDomainAssignAddresses(def, qemuCaps, NULL) < 0)
+ if (qemuDomainAssignAddresses(def, qemuCaps, NULL, false) < 0)
goto cleanup;
if (!(vm = virDomainObjListAdd(driver->domains, def,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 505778e..5349021 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3840,7 +3840,7 @@ qemuProcessReconnect(void *opaque)
}
if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE))
- if ((qemuDomainAssignAddresses(obj->def, priv->qemuCaps, obj)) < 0)
+ if ((qemuDomainAssignAddresses(obj->def, priv->qemuCaps, obj, false)) <
0)
goto error;
/* if domain requests security driver we haven't loaded, report error, but
@@ -4711,7 +4711,7 @@ int qemuProcessStart(virConnectPtr conn,
*/
if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
VIR_DEBUG("Assigning domain PCI addresses");
- if ((qemuDomainAssignAddresses(vm->def, priv->qemuCaps, vm)) < 0)
+ if ((qemuDomainAssignAddresses(vm->def, priv->qemuCaps, vm, false)) <
0)
goto cleanup;
}
@@ -5561,7 +5561,7 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
*/
if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
VIR_DEBUG("Assigning domain PCI addresses");
- if ((qemuDomainAssignAddresses(vm->def, priv->qemuCaps, vm)) < 0)
+ if ((qemuDomainAssignAddresses(vm->def, priv->qemuCaps, vm, false)) <
0)
goto error;
}
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 368a5e7..ea37a6c 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -85,7 +85,7 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt,
if (event)
virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_DEVICE_DEL_EVENT);
- if (qemuDomainAssignAddresses((*vm)->def, priv->qemuCaps, *vm) < 0)
+ if (qemuDomainAssignAddresses((*vm)->def, priv->qemuCaps, *vm, true) < 0)
goto cleanup;
if (qemuAssignDeviceAliases((*vm)->def, priv->qemuCaps) < 0)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index c2482e6..8815087 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -307,7 +307,7 @@ static int testCompareXMLToArgvFiles(const char *xml,
virQEMUCapsFilterByMachineType(extraFlags, vmdef->os.machine);
if (virQEMUCapsGet(extraFlags, QEMU_CAPS_DEVICE)) {
- if (qemuDomainAssignAddresses(vmdef, extraFlags, NULL)) {
+ if (qemuDomainAssignAddresses(vmdef, extraFlags, NULL, true)) {
if (flags & FLAG_EXPECT_ERROR)
goto ok;
goto out;
diff --git a/tests/qemuxmlnstest.c b/tests/qemuxmlnstest.c
index 8eaab8a..ed11eb8 100644
--- a/tests/qemuxmlnstest.c
+++ b/tests/qemuxmlnstest.c
@@ -89,7 +89,7 @@ static int testCompareXMLToArgvFiles(const char *xml,
QEMU_CAPS_LAST);
if (virQEMUCapsGet(extraFlags, QEMU_CAPS_DEVICE))
- qemuDomainAssignAddresses(vmdef, extraFlags, NULL);
+ qemuDomainAssignAddresses(vmdef, extraFlags, NULL, true);
log = virtTestLogContentAndReset();
VIR_FREE(log);
--
2.4.6
6:28 p.m.
New subject: [libvirt] [PATCH 4/9] Add newDomain parameter to qemuDomainAssignAddresses
On 08/12/2015 10:52 AM, Ján Tomko wrote:
To differentiate a new domain from an existing one.
---
src/qemu/qemu_command.c | 5 ++++-
src/qemu/qemu_command.h | 3 ++-
src/qemu/qemu_driver.c | 22 +++++++++++-----------
src/qemu/qemu_process.c | 6 +++---
tests/qemuhotplugtest.c | 2 +-
tests/qemuxml2argvtest.c | 2 +-
tests/qemuxmlnstest.c | 2 +-
7 files changed, 23 insertions(+), 19 deletions(-)
Could have used a slightly beefier commit message...
I've looked ahead and see how it's used and it seems OK - although it's
too bad we couldn't have some way to know in the domain def to know this
domain has never been defined/created *and* saved in some manner. This
certainly works, just was thinking of a way to avoid another parameter
and perhaps a way that could be extensible in the future
ACK to what's here - unless this causes someone to think of something
different.
John
9:52 a.m.
New subject: [libvirt] [PATCH 5/9] Introduce virDomainUSBAddressSet
A new type to track USB addresses.
The buses in virDomainUSBAddressSet correspond to USB controllers.
They are represented by the virDomainUSBAddressHub type, having nports
USB ports. These can contain nested hubs (nports != 0), or they can be
occupied by other USB devices (with nports = 0).
---
src/conf/domain_addr.c | 41 +++++++++++++++++++++++++++++++++++++++++
src/conf/domain_addr.h | 17 +++++++++++++++++
src/libvirt_private.syms | 2 ++
3 files changed, 60 insertions(+)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index a5d142d..3962357 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1198,3 +1198,44 @@ virDomainUSBAddressGetPortString(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;
+
+ for (i = 0; i < hub->nports; i++) {
+ if (hub->ports[i])
+ virDomainUSBAddressHubFree(hub->ports[i]);
+ }
+ VIR_FREE(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 c6d8da7..dcf86d4 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -248,4 +248,21 @@ char *
virDomainUSBAddressGetPortString(unsigned int *port)
ATTRIBUTE_NONNULL(1);
+typedef struct _virDomainUSBAddressHub virDomainUSBAddressHub;
+typedef virDomainUSBAddressHub *virDomainUSBAddressHubPtr;
+struct _virDomainUSBAddressHub {
+ virDomainUSBAddressHubPtr *ports;
+ size_t nports;
+};
+
+struct _virDomainUSBAddressSet {
+ 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 5168230..a628d00 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -108,6 +108,8 @@ virDomainPCIAddressSlotInUse;
virDomainPCIAddressValidate;
virDomainUSBAddressGetPortBuf;
virDomainUSBAddressGetPortString;
+virDomainUSBAddressSetCreate;
+virDomainUSBAddressSetFree;
virDomainVirtioSerialAddrAssign;
virDomainVirtioSerialAddrAutoAssign;
virDomainVirtioSerialAddrIsComplete;
--
2.4.6
9:26 a.m.
New subject: [libvirt] [PATCH 5/9] Introduce virDomainUSBAddressSet
Picking up where I left off (more or less) before KVM Forum... I think
I need a virtual whiteboard... I tried reviewing patches 5 -> 7 together
- going back and forth. Hopefully I haven't left (m)any disjoint
thoughts... I have left some thoughts on the way through - some just to
make sure I'm following what's being done... others because I found
something not quite right.
On 08/12/2015 10:52 AM, Ján Tomko wrote:
A new type to track USB addresses.
The buses in virDomainUSBAddressSet correspond to USB controllers.
They are represented by the virDomainUSBAddressHub type, having nports
USB ports. These can contain nested hubs (nports != 0), or they can be
occupied by other USB devices (with nports = 0).
Recalling earlier reviews - nested ports are the "n.n", "n.n.n", and
"n.n.n.n" dotted octet port numbers... What's still not clear in my mind
is whether it's possible to have "port='1'" and
"port='1.1'" in
hardware. Based on the algorithm - I assume not, but without digging in
it wasn't obvious. Maybe this is something we can document to make it
clearer.
Based on your description and the recursive virDomainUSBAddressHubFree,
pictorally this is what I envision:
AddressSet
+---------+ AddressHub
| buses[] |----> +--------+
| nbuses | | ports[]|----> +---------+ AddressHub
+---------+ | nports | | ports[0]|----> +--------+
+--------+ | ports[1]| | ports + ...
nports != 0 +---------+ | nports |
+--------+
nports != 0
After reading further, I think AddressSet may need to define which
controller index is being used rather than allocate an array of buses
based on the controller index found.
It also took me a while to find where nports == 0 (the code wasn't self
commenting ;-))
It seems that AddressSet is an array of buses (numbered 0...0xfff
according to formatdomain.html.in - something that could be checked) and
that AddressHub is either another hub (eg, an array of 'nports') or a
device. An array entry can contain either another array or a device.
There can be up to 4 nested octets.
---
src/conf/domain_addr.c | 41 +++++++++++++++++++++++++++++++++++++++++
src/conf/domain_addr.h | 17 +++++++++++++++++
src/libvirt_private.syms | 2 ++
3 files changed, 60 insertions(+)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index a5d142d..3962357 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1198,3 +1198,44 @@ virDomainUSBAddressGetPortString(unsigned int *port)
return NULL;
return virBufferContentAndReset(&buf);
}
+
+
+virDomainUSBAddressSetPtr
+virDomainUSBAddressSetCreate(void)
+{
+ virDomainUSBAddressSetPtr addrs;
+
+ if (VIR_ALLOC(addrs) < 0)
+ return NULL;
+
I think we may need an 'addrs->ctlr_idx = -1;'
+ return addrs;
+}
+
+
+static void
+virDomainUSBAddressHubFree(virDomainUSBAddressHubPtr hub)
+{
+ size_t i;
+
+ for (i = 0; i < hub->nports; i++) {
+ if (hub->ports[i])
+ virDomainUSBAddressHubFree(hub->ports[i]);
+ }
+ VIR_FREE(hub->ports);
+ VIR_FREE(hub);
+}
+
Need extra LF (others had 2 this one had 1)
+void virDomainUSBAddressSetFree(virDomainUSBAddressSetPtr addrs)
void
virDomain*
+{
+ 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 c6d8da7..dcf86d4 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -248,4 +248,21 @@ char *
virDomainUSBAddressGetPortString(unsigned int *port)
ATTRIBUTE_NONNULL(1);
+typedef struct _virDomainUSBAddressHub virDomainUSBAddressHub;
+typedef virDomainUSBAddressHub *virDomainUSBAddressHubPtr;
+struct _virDomainUSBAddressHub {
+ virDomainUSBAddressHubPtr *ports;
+ size_t nports;
+};
+
+struct _virDomainUSBAddressSet {
+ virDomainUSBAddressHubPtr *buses;
+ size_t nbuses;
This may need an:
int ctlr_idx;
John
+};
+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 5168230..a628d00 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -108,6 +108,8 @@ virDomainPCIAddressSlotInUse;
virDomainPCIAddressValidate;
virDomainUSBAddressGetPortBuf;
virDomainUSBAddressGetPortString;
+virDomainUSBAddressSetCreate;
+virDomainUSBAddressSetFree;
virDomainVirtioSerialAddrAssign;
virDomainVirtioSerialAddrAutoAssign;
virDomainVirtioSerialAddrIsComplete;
9:52 a.m.
New subject: [libvirt] [PATCH 6/9] Add functions for adding usb controllers to addrs
---
src/conf/domain_addr.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++++
src/conf/domain_addr.h | 4 +++
src/libvirt_private.syms | 1 +
3 files changed, 93 insertions(+)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 3962357..024d47b 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1239,3 +1239,91 @@ void virDomainUSBAddressSetFree(virDomainUSBAddressSetPtr addrs)
VIR_FREE(addrs->buses);
VIR_FREE(addrs);
}
+
+
+static int virDomainUSBAddressSetAddController(virDomainUSBAddressSetPtr addrs,
+ virDomainControllerDefPtr cont)
+{
+ virDomainUSBAddressHubPtr hub = NULL;
+ size_t ports = 0;
+ int ret = -1;
+ int model = cont->model;
+
+ VIR_DEBUG("addrs=%p controller type=%d model=%d",
+ addrs, cont->type, cont->model);
+
+ if (VIR_ALLOC(hub) < 0)
+ goto cleanup;
+
+ 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:
+ ports = 2;
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_EHCI:
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_EHCI1:
+ ports = 6;
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI1:
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI2:
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI3:
+ return 0;
+ /* FIXME * check the companions? */
+ ports = 2;
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI:
+ ports = 3;
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI:
+ ports = 15;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE:
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_LAST:
+ ret = 0;
+ goto cleanup;
+ }
+
+ if (cont->idx >= addrs->nbuses) {
+ if (VIR_EXPAND_N(addrs->buses, addrs->nbuses, cont->idx -
addrs->nbuses + 1) < 0)
+ goto cleanup;
+ }
+
+ if (VIR_ALLOC_N(hub->ports, ports + 1) < 0)
+ goto cleanup;
+ hub->nports = ports + 1;
+
+ VIR_DEBUG("Added %zu ports on hub %p", hub->nports - 1, hub);
+
+ /* FIXME: is there a bus already? */
+ addrs->buses[cont->idx] = hub;
+ hub = NULL;
+
+ ret = 0;
+ cleanup:
+ VIR_FREE(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 dcf86d4..64d35e9 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -263,6 +263,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 a628d00..7db4839 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -108,6 +108,7 @@ virDomainPCIAddressSlotInUse;
virDomainPCIAddressValidate;
virDomainUSBAddressGetPortBuf;
virDomainUSBAddressGetPortString;
+virDomainUSBAddressSetAddControllers;
virDomainUSBAddressSetCreate;
virDomainUSBAddressSetFree;
virDomainVirtioSerialAddrAssign;
--
2.4.6
5:10 p.m.
New subject: [libvirt] [PATCH 6/9] Add functions for adding usb controllers to addrs
On 08/12/2015 10:52 AM, Ján Tomko wrote:
---
src/conf/domain_addr.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++++
src/conf/domain_addr.h | 4 +++
src/libvirt_private.syms | 1 +
3 files changed, 93 insertions(+)
Ran the series through Coverity as a first pass... Figured I'd report
what was spewed first before digging in deeper...
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 3962357..024d47b 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1239,3 +1239,91 @@ void virDomainUSBAddressSetFree(virDomainUSBAddressSetPtr addrs)
VIR_FREE(addrs->buses);
VIR_FREE(addrs);
}
+
+
+static int virDomainUSBAddressSetAddController(virDomainUSBAddressSetPtr addrs,
+ virDomainControllerDefPtr cont)
+{
+ virDomainUSBAddressHubPtr hub = NULL;
+ size_t ports = 0;
+ int ret = -1;
+ int model = cont->model;
+
+ VIR_DEBUG("addrs=%p controller type=%d model=%d",
+ addrs, cont->type, cont->model);
+
+ if (VIR_ALLOC(hub) < 0)
+ goto cleanup;
+
+ 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:
+ ports = 2;
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_EHCI:
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_EHCI1:
+ ports = 6;
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI1:
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI2:
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI3:
+ return 0;
+ /* FIXME * check the companions? */
+ ports = 2;
+ break;
I'm sure you realize Coverity would choke here... between 'hub' not
being freed and ports=2 not reachable.
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI:
+ ports = 3;
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI:
+ ports = 15;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE:
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_LAST:
+ ret = 0;
+ goto cleanup;
+ }
+
+ if (cont->idx >= addrs->nbuses) {
+ if (VIR_EXPAND_N(addrs->buses, addrs->nbuses, cont->idx -
addrs->nbuses + 1) < 0)
+ goto cleanup;
+ }
+
+ if (VIR_ALLOC_N(hub->ports, ports + 1) < 0)
+ goto cleanup;
+ hub->nports = ports + 1;
+
+ VIR_DEBUG("Added %zu ports on hub %p", hub->nports - 1, hub);
+
+ /* FIXME: is there a bus already? */
+ addrs->buses[cont->idx] = hub;
+ hub = NULL;
+
+ ret = 0;
+ cleanup:
+ VIR_FREE(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 dcf86d4..64d35e9 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -263,6 +263,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 a628d00..7db4839 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -108,6 +108,7 @@ virDomainPCIAddressSlotInUse;
virDomainPCIAddressValidate;
virDomainUSBAddressGetPortBuf;
virDomainUSBAddressGetPortString;
+virDomainUSBAddressSetAddControllers;
virDomainUSBAddressSetCreate;
virDomainUSBAddressSetFree;
virDomainVirtioSerialAddrAssign;
9:32 a.m.
New subject: [libvirt] [PATCH 6/9] Add functions for adding usb controllers to addrs
Perhaps needs a bit more meat/description for the commit message...
On 08/12/2015 10:52 AM, Ján Tomko wrote:
---
src/conf/domain_addr.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++++
src/conf/domain_addr.h | 4 +++
src/libvirt_private.syms | 1 +
3 files changed, 93 insertions(+)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 3962357..024d47b 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1239,3 +1239,91 @@ void virDomainUSBAddressSetFree(virDomainUSBAddressSetPtr addrs)
VIR_FREE(addrs->buses);
VIR_FREE(addrs);
}
+
+
+static int virDomainUSBAddressSetAddController(virDomainUSBAddressSetPtr addrs,
+ virDomainControllerDefPtr cont)
static int
virDomain*
+{
+ virDomainUSBAddressHubPtr hub = NULL;
+ size_t ports = 0;
Perhaps easier if use 'nports'
+ int ret = -1;
+ int model = cont->model;
+
+ VIR_DEBUG("addrs=%p controller type=%d model=%d",
+ addrs, cont->type, cont->model);
+
+ if (VIR_ALLOC(hub) < 0)
+ goto cleanup;
This should be paired this with the ports allocation and have a separate
API to also be used by next patch
+
+ 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:
+ ports = 2;
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_EHCI:
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_EHCI1:
+ ports = 6;
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI1:
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI2:
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI3:
+ return 0;
+ /* FIXME * check the companions? */
+ ports = 2;
+ break;
This needs to be fixed/resolved.
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI:
+ ports = 3;
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI:
+ ports = 15;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE:
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_LAST:
+ ret = 0;
+ goto cleanup;
+ }
+
+ if (cont->idx >= addrs->nbuses) {
+ if (VIR_EXPAND_N(addrs->buses, addrs->nbuses, cont->idx -
addrs->nbuses + 1) < 0)
+ goto cleanup;
+ }
So if I read this correctly, someone defining a single controller in
their XML with an index of 5 (or 10 or 100 or anything up to 0xfff -
which is supposed to be the bus# max) will be creating that many
AddressSets. There's no requirement that one numbers their controllers
starting at '0' - just that the index is used as the order to be loaded.
Perhaps not what is desired. Perhaps need some sort of number of
controllers of type 'model' found API rather than a straight 'idx' to
'nbuses' comparison.
Once it's figured out which controller index is being used, that could
be set in 'addrs->ctlr_idx'. Could also create a companion search API
or two that could take the 'ctlr_idx' and "find" the controller knowing
the eventual USB device type.
Since there's no guarantee of having cont->idx start at 0 and run
through some maximum, should this use a VIR_INSERT_ELEMENT type
algorithm to keep things ordered? Using ctlr_idx for ordering (if it
even matters).
+
+ if (VIR_ALLOC_N(hub->ports, ports + 1) < 0)
+ goto cleanup;
+ hub->nports = ports + 1;
+
Mental note #1 - why are we allocating an extra port? Answered later on
- using port#0 is not valid (nor is using 0 in dotted notation - eg
1.0.1 would equate to just 1). Since this isn't obvious - it might be
beneficial to add a note as to why we're adding 1.
Mental note #2 - this is what I'll refer to later as the root usb hub...
The VIR_ALLOC(hub) and this VIR_ALLOC_N should have a separate API to be
shared in next patch as well. It can take a parameter of 'nports'.
What's not fully clear (and something I note in the next patch) is
whether a root hub port can contain a device or if it has to contain
another hub. I would think physically the answer would be a device
could live on the root hub, but dealing with the physical world is not
something I know all that well..
+ VIR_DEBUG("Added %zu ports on hub %p", hub->nports
- 1, hub);
+
+ /* FIXME: is there a bus already? */
+ addrs->buses[cont->idx] = hub;
Wouldn't this be solved with VIR_INSERT_ELEMENT? If buses[cont->idx] !=
NULL before this line what does that say - something's wrong with the
algorithm. At the very least you'll leak memory and lose at least 1
device and possibly more.
+ hub = NULL;
+
+ ret = 0;
+ cleanup:
+ VIR_FREE(hub);
+ return ret;
+}
+
+
+int virDomainUSBAddressSetAddControllers(virDomainUSBAddressSetPtr addrs,
+ virDomainDefPtr def)
int
virDomain*
John
+{
+ 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 dcf86d4..64d35e9 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -263,6 +263,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 a628d00..7db4839 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -108,6 +108,7 @@ virDomainPCIAddressSlotInUse;
virDomainPCIAddressValidate;
virDomainUSBAddressGetPortBuf;
virDomainUSBAddressGetPortString;
+virDomainUSBAddressSetAddControllers;
virDomainUSBAddressSetCreate;
virDomainUSBAddressSetFree;
virDomainVirtioSerialAddrAssign;
9:52 a.m.
New subject: [libvirt] [PATCH 7/9] Reserve existing USB addresses
If USB addresses have been provided for all USB devices,
or we are defining a new domain, reserve the addresses.
Check if they fit on the USB controllers the domain has,
and error out if two devices try to use the same address.
Do not error out on missing hubs.
The input-usbmouse test used port=4 even though the implicit
USB controller only has two.
---
src/conf/domain_addr.c | 109 ++++++++++++++++
src/conf/domain_addr.h | 6 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c | 137 ++++++++++++++++++++-
src/qemu/qemu_domain.h | 1 +
.../qemuxml2argv-input-usbmouse-addr.args | 2 +-
.../qemuxml2argv-input-usbmouse-addr.xml | 2 +-
.../qemuxml2argv-usb-hub-conflict.xml | 22 ++++
tests/qemuxml2argvtest.c | 3 +
9 files changed, 280 insertions(+), 3 deletions(-)
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 024d47b..cda6e08 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1313,6 +1313,69 @@ static int
virDomainUSBAddressSetAddController(virDomainUSBAddressSetPtr addrs,
}
+/* Finds the port specified by the port path in the device info.
+ * The USB bus is expected to exist.
+ * The USB hubs will be auto-created.
+ */
+static int
+virDomainUSBAddressFindPort(virDomainUSBAddressHubPtr **port,
+ virDomainUSBAddressSetPtr addrs,
+ virDomainDeviceInfoPtr info)
+{
+ ssize_t i;
+ virDomainUSBAddressHubPtr *hub = NULL;
+ char *portstr = NULL;
+ int ret = -1;
+
+ portstr = virDomainUSBAddressGetPortString(info->addr.usb.port);
+
+ if (addrs->nbuses <= info->addr.usb.bus ||
+ !addrs->buses[info->addr.usb.bus]) {
+ virReportError(VIR_ERR_XML_ERROR, _("Missing USB bus %u"),
+ info->addr.usb.bus);
+ goto cleanup;
+ }
+ hub = &(addrs->buses[info->addr.usb.bus]);
+
+ for (i = 0; i < VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH; i++) {
+ int portnum = info->addr.usb.port[i];
+ if (!portnum)
+ break;
+
+ if (!(*hub)) {
+ /* FIXME: Add the hub to the domain definition */
+ if (VIR_ALLOC(*hub) < 0)
+ goto cleanup;
+ if (VIR_ALLOC_N((*hub)->ports, 8+1) < 0) {
+ VIR_FREE((*hub));
+ goto cleanup;
+ }
+ (*hub)->nports = 8+1;
+ }
+ if ((*hub)->nports < portnum) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("port %u out of range in port path %s"),
+ portnum, portstr);
+ goto cleanup;
+ }
+ hub = &((*hub)->ports[info->addr.usb.port[i]]);
+ if ((*hub) && (*hub)->nports == 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Port %d is already occupied in port path %s"),
+ portnum, portstr);
+ goto cleanup;
+ }
+ }
+
+ ret = 0;
+ *port = hub;
+
+ cleanup:
+ VIR_FREE(portstr);
+ return ret;
+}
+
+
int virDomainUSBAddressSetAddControllers(virDomainUSBAddressSetPtr addrs,
virDomainDefPtr def)
{
@@ -1327,3 +1390,49 @@ int virDomainUSBAddressSetAddControllers(virDomainUSBAddressSetPtr
addrs,
}
return 0;
}
+
+
+int
+virDomainUSBAddressReserve(virDomainDefPtr def ATTRIBUTE_UNUSED,
+ virDomainDeviceDefPtr dev ATTRIBUTE_UNUSED,
+ virDomainDeviceInfoPtr info,
+ void *data)
+{
+ virDomainUSBAddressSetPtr addrs = data;
+ virDomainUSBAddressHubPtr *port = NULL;
+ char *portstr = NULL;
+ int ret = -1;
+
+ if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB)
+ return 0;
+
+ portstr = virDomainUSBAddressGetPortString(info->addr.usb.port);
+ VIR_DEBUG("Reserving USB addr bus=%u port=%s", info->addr.usb.bus,
portstr);
+
+ if (virDomainUSBAddressFindPort(&port, addrs, info) < 0)
+ goto cleanup;
+
+ if (dev &&
+ dev->type == VIR_DOMAIN_DEVICE_HUB &&
+ dev->data.hub->type == VIR_DOMAIN_HUB_TYPE_USB) {
+ if (!*port) {
+ if (VIR_ALLOC(*port) < 0)
+ goto cleanup;
+
+ if (VIR_ALLOC_N((*port)->ports, 8 + 1) < 0) {
+ VIR_FREE(*port);
+ goto cleanup;
+ }
+ (*port)->nports = 8 + 1;
+ }
+ } else {
+ if (VIR_ALLOC(*port) < 0)
+ goto cleanup;
+ }
+
+ ret = 0;
+
+ cleanup:
+ VIR_FREE(portstr);
+ return ret;
+}
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 64d35e9..8f866ae 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -269,4 +269,10 @@ int virDomainUSBAddressSetAddControllers(virDomainUSBAddressSetPtr
addrs,
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
void virDomainUSBAddressSetFree(virDomainUSBAddressSetPtr addrs);
+int
+virDomainUSBAddressReserve(virDomainDefPtr def,
+ virDomainDeviceDefPtr dev,
+ virDomainDeviceInfoPtr info,
+ void *data)
+ ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
#endif /* __DOMAIN_ADDR_H__ */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 7db4839..1420b19 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -108,6 +108,7 @@ virDomainPCIAddressSlotInUse;
virDomainPCIAddressValidate;
virDomainUSBAddressGetPortBuf;
virDomainUSBAddressGetPortString;
+virDomainUSBAddressReserve;
virDomainUSBAddressSetAddControllers;
virDomainUSBAddressSetCreate;
virDomainUSBAddressSetFree;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4b520d7..a6a3438 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1563,6 +1563,137 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def,
}
+static void
+qemuDomainCountUSBAddresses(virDomainDefPtr def,
+ size_t *specified,
+ size_t *total)
+{
+ size_t i;
+ size_t spec = 0, tot = 0;
+
+ /* usb-hub */
+ for (i = 0; i < def->nhubs; i++) {
+ virDomainHubDefPtr hub = def->hubs[i];
+ if (hub->type == VIR_DOMAIN_HUB_TYPE_USB) {
+ tot++;
+ if (hub->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB)
+ spec++;
+ }
+ }
+
+ /* 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) {
+ tot++;
+ if (hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB)
+ spec++;
+ }
+ }
+
+ /* usb-storage */
+ for (i = 0; i < def->ndisks; i++) {
+ virDomainDiskDefPtr disk = def->disks[i];
+ if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) {
+ tot++;
+ if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB)
+ spec++;
+ }
+ }
+
+ /* TODO: usb-net */
+
+ /* usb-ccid */
+ for (i = 0; i < def->ncontrollers; i++) {
+ virDomainControllerDefPtr cont = def->controllers[i];
+ if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_CCID) {
+ tot++;
+ if (cont->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB)
+ spec++;
+ }
+ }
+
+ /* 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) {
+ tot++;
+ if (input->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB)
+ spec++;
+ }
+ }
+
+ /* usb-serial */
+ for (i = 0; i < def->nserials; i++) {
+ virDomainChrDefPtr serial = def->serials[i];
+ if (serial->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB) {
+ tot++;
+ if (serial->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB)
+ spec++;
+ }
+ }
+
+ /* usb-audio model=usb */
+ for (i = 0; i < def->nsounds; i++) {
+ virDomainSoundDefPtr sound = def->sounds[i];
+ if (sound->model == VIR_DOMAIN_SOUND_MODEL_USB) {
+ tot++;
+ if (sound->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB)
+ spec++;
+ }
+ }
+
+ *specified = spec;
+ *total = tot;
+}
+
+static int
+qemuDomainAssignUSBAddresses(virDomainDefPtr def,
+ virDomainObjPtr obj,
+ bool newDomain)
+{
+ int ret = -1;
+ size_t total, specified;
+ virDomainUSBAddressSetPtr addrs = NULL;
+ qemuDomainObjPrivatePtr priv = NULL;
+
+ qemuDomainCountUSBAddresses(def, &specified, &total);
+ VIR_DEBUG("%zu out of %zu USB addresses have been specified", specified,
total);
+
+ /* if ALL USB devices have their addresses assigned
+ * or we're creating a new domain, we can continue allocation,
+ * otherwise QEMU might create an incompatible machine by adding hubs
+ * and own addresses */
+ if (!(newDomain || total == specified))
+ return 0;
+
+ if (!(addrs = virDomainUSBAddressSetCreate()))
+ goto cleanup;
+
+ if (virDomainUSBAddressSetAddControllers(addrs, def) < 0)
+ goto cleanup;
+
+ if (virDomainDeviceInfoIterate(def, virDomainUSBAddressReserve, addrs) < 0)
+ goto cleanup;
+
+ VIR_DEBUG("Existing USB addresses have been reserved");
+
+ if (obj && obj->privateData) {
+ priv = obj->privateData;
+ /* if this is the live domain object, we persist the addresses */
+ priv->usbaddrs = addrs;
+ priv->persistentAddrs = 1;
+ addrs = NULL;
+ }
+ ret = 0;
+
+ cleanup:
+ virDomainUSBAddressSetFree(addrs);
+ return ret;
+}
+
+
static int
qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
virDomainDeviceDefPtr device,
@@ -1774,7 +1905,7 @@ qemuDomainPCIBusFullyReserved(virDomainPCIAddressBusPtr bus)
int qemuDomainAssignAddresses(virDomainDefPtr def,
virQEMUCapsPtr qemuCaps,
virDomainObjPtr obj,
- bool newDomain ATTRIBUTE_UNUSED)
+ bool newDomain)
{
int rc;
@@ -1796,6 +1927,10 @@ int qemuDomainAssignAddresses(virDomainDefPtr def,
if (rc)
return rc;
+ rc = qemuDomainAssignUSBAddresses(def, obj, newDomain);
+ if (rc)
+ return rc;
+
return qemuDomainAssignPCIAddresses(def, qemuCaps, obj);
}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 2af7c59..454f759 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -167,6 +167,7 @@ struct _qemuDomainObjPrivate {
virDomainPCIAddressSetPtr pciaddrs;
virDomainCCWAddressSetPtr ccwaddrs;
virDomainVirtioSerialAddrSetPtr vioserialaddrs;
+ virDomainUSBAddressSetPtr usbaddrs;
int persistentAddrs;
virQEMUCapsPtr qemuCaps;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse-addr.args
b/tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse-addr.args
index 07ea004..2f45b03 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse-addr.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse-addr.args
@@ -2,5 +2,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=none \
/usr/bin/qemu -S \
-M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults \
-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \
--hda /dev/HostVG/QEMUGuest1 -device usb-mouse,id=input0,bus=usb.0,port=4 \
+-hda /dev/HostVG/QEMUGuest1 -device usb-mouse,id=input0,bus=usb.0,port=2 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse-addr.xml
b/tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse-addr.xml
index a996838..dde3517 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse-addr.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse-addr.xml
@@ -22,7 +22,7 @@
<controller type='usb' index='0'/>
<controller type='ide' index='0'/>
<input type='mouse' bus='usb'>
- <address type='usb' bus='0' port='4'/>
+ <address type='usb' bus='0' port='2'/>
</input>
</devices>
</domain>
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 8815087..feb1d85 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1171,6 +1171,9 @@ mymain(void)
DO_TEST("usb-hub",
QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_USB_HUB,
QEMU_CAPS_NODEFCONFIG);
+ DO_TEST_ERROR("usb-hub-conflict",
+ QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_USB_HUB,
+ QEMU_CAPS_NODEFCONFIG);
DO_TEST("usb-ports",
QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_USB_HUB,
QEMU_CAPS_NODEFCONFIG);
--
2.4.6
9:39 a.m.
New subject: [libvirt] [PATCH 7/9] Reserve existing USB addresses
On 08/12/2015 10:52 AM, Ján Tomko wrote:
If USB addresses have been provided for all USB devices,
or we are defining a new domain, reserve the addresses.
Check if they fit on the USB controllers the domain has,
and error out if two devices try to use the same address.
Do not error out on missing hubs.
The input-usbmouse test used port=4 even though the implicit
USB controller only has two.
---
src/conf/domain_addr.c | 109 ++++++++++++++++
src/conf/domain_addr.h | 6 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c | 137 ++++++++++++++++++++-
src/qemu/qemu_domain.h | 1 +
.../qemuxml2argv-input-usbmouse-addr.args | 2 +-
.../qemuxml2argv-input-usbmouse-addr.xml | 2 +-
.../qemuxml2argv-usb-hub-conflict.xml | 22 ++++
tests/qemuxml2argvtest.c | 3 +
9 files changed, 280 insertions(+), 3 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-hub-conflict.xml
Wouldn't qemuDomainObjPrivateFree need to be updated as well to free
'usbaddrs' (similar to pciaddrs, ccwaddrs, vioserialaddrs)?
What about qemuProcessStop where the various addr's have their free
routines called and then the *addrs pointer set to NULL?
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 024d47b..cda6e08 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1313,6 +1313,69 @@ static int
virDomainUSBAddressSetAddController(virDomainUSBAddressSetPtr addrs,
}
+/* Finds the port specified by the port path in the device info.
+ * The USB bus is expected to exist.
+ * The USB hubs will be auto-created.
+ */
+static int
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
+virDomainUSBAddressFindPort(virDomainUSBAddressHubPtr **port,
+ virDomainUSBAddressSetPtr addrs,
+ virDomainDeviceInfoPtr info)
+{
+ ssize_t i;
+ virDomainUSBAddressHubPtr *hub = NULL;
+ char *portstr = NULL;
+ int ret = -1;
+
+ portstr = virDomainUSBAddressGetPortString(info->addr.usb.port);
What happens if portstr == NULL ?
+
+ if (addrs->nbuses <= info->addr.usb.bus ||
+ !addrs->buses[info->addr.usb.bus]) {
Personally it's easier to think about it as if info->addr.usb.bus >
addrs->nbuses...
This would perhaps be a place where a find a bus by ctlr_idx number
could be useful - considering earlier review comments where I noted we
may not want to create 'cont->idx' buses.
+ virReportError(VIR_ERR_XML_ERROR, _("Missing USB bus
%u"),
+ info->addr.usb.bus);
+ goto cleanup;
+ }
+ hub = &(addrs->buses[info->addr.usb.bus]);
I would think !hub would be a problem here since it's the "root/bus
hub"... To start out with at least the root hub better point to
something... Hence my ATTRIBUTE_NONNULL above...
+
+ for (i = 0; i < VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH; i++) {
+ int portnum = info->addr.usb.port[i];
+ if (!portnum)
+ break;
oh or oy - here's the nested ports (eg the dotted octet's)...
Assuming we have "port='1'", then we fall into the rest of this when it
feels to me that we shouldn't. It would seem we're allocating perhaps an
extra layer that we don't want to. To me it seems the current algorithm
would allocate an AddressHub off the bus/root hub at index[0] with
nports=9.
Assuming a 2 port root usb hub, after return from here we'd end up with:
AddressSet
+---------+ AddressHub
| buses[] |----> +--------+
| nbuses | | ports[]|----> +--------+ AddressHub
+---------+ | nports | |ports[0]|----> +--------+
+--------+ |ports[1]| | ports[]| ...
nports == 2 +--------+ | nports |
+--------+
nports == 9
Then the caller would allocate another AddressHub with nports=0 at the
AddressHub on the right at ports[0]. I guess I would have assumed that
the AddressHub would nports=0 could be allocated at ports[0] off the
root/usb hub.
So the rather than actually plugging something directly into the root
usb hub, the algorithm creates an unnecessary hub, leaving
host -> root hub[0] -> hub[0] -> device
instead of
host -> root hub[0] -> device
I think perhaps what's trying to be accomplished is to count the "depth"
of the portstr, where the depth is 1, 2, 3, or 4. If the depth is 1,
then I think we'd return to the caller which would hopefully allocate an
AddressHub w/ nports=0 and plug the device directly into the root hub.
If the depth was 2, 3, or 4, then we'd have to allocate 'depth' nested
hubs each at the location of the port[i] value and then return to the
caller which would plug our device into that last port.
The following is just hard to read/comprehend mentally...
+
+ if (!(*hub)) {
+ /* FIXME: Add the hub to the domain definition */
If we do that - does it get removed from the private area?
+ if (VIR_ALLOC(*hub) < 0)
+ goto cleanup;
+ if (VIR_ALLOC_N((*hub)->ports, 8+1) < 0) {
+ VIR_FREE((*hub));
+ goto cleanup;
+ }
+ (*hub)->nports = 8+1;
"8+1" is such an ugly magic number... Again, like my note in patch6 -
good idea to add a note 'why' the "+1" is there...
If there was a 'ports' allocation API this would be opaque... eg.
newhub = virDomainUSBAddressAllocPort(8); /* or some constant */
if (!newhub)
failure
(*hub) = newhub;
newhub = NULL
+ }
+ if ((*hub)->nports < portnum) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("port %u out of range in port path %s"),
+ portnum, portstr);
+ goto cleanup;
+ }
This check seems out of place. I think this answers a question I posed
somewhere along the way whether "port='3.1'" would be legal in a 2 port
root usb and that "port='1.10'" is perhaps not legal either...
I would think this should go right after we get the portnum before the
break; in the above loop/context.
+ hub = &((*hub)->ports[info->addr.usb.port[i]]);
+ if ((*hub) && (*hub)->nports == 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Port %d is already occupied in port path %s"),
+ portnum, portstr);
+ goto cleanup;
+ }
Similarly this could be out of place too - although perhaps it's better
as an else of the (!(*hub))...
+ }
+
+ ret = 0;
+ *port = hub;
+
+ cleanup:
+ VIR_FREE(portstr);
+ return ret;
+}
+
+
int virDomainUSBAddressSetAddControllers(virDomainUSBAddressSetPtr addrs,
virDomainDefPtr def)
{
@@ -1327,3 +1390,49 @@ int virDomainUSBAddressSetAddControllers(virDomainUSBAddressSetPtr
addrs,
}
return 0;
}
+
+
+int
+virDomainUSBAddressReserve(virDomainDefPtr def ATTRIBUTE_UNUSED,
def "unused" but required since using virDomainDeviceInfoIterate...
Perhaps something to note...
+ virDomainDeviceDefPtr dev
ATTRIBUTE_UNUSED,
+ virDomainDeviceInfoPtr info,
+ void *data)
+{
+ virDomainUSBAddressSetPtr addrs = data;
+ virDomainUSBAddressHubPtr *port = NULL;
+ char *portstr = NULL;
+ int ret = -1;
+
+ if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB)
+ return 0;
+
+ portstr = virDomainUSBAddressGetPortString(info->addr.usb.port);
What happens if portstr == NULL ?
+ VIR_DEBUG("Reserving USB addr bus=%u port=%s",
info->addr.usb.bus, portstr);
+
+ if (virDomainUSBAddressFindPort(&port, addrs, info) < 0)
+ goto cleanup;
+
+ if (dev &&
+ dev->type == VIR_DOMAIN_DEVICE_HUB &&
+ dev->data.hub->type == VIR_DOMAIN_HUB_TYPE_USB) {
+ if (!*port) {
+ if (VIR_ALLOC(*port) < 0)
+ goto cleanup;
+
+ if (VIR_ALLOC_N((*port)->ports, 8 + 1) < 0) {
+ VIR_FREE(*port);
+ goto cleanup;
+ }
+ (*port)->nports = 8 + 1;
Here again a magic 8 + 1... and I'm not sure why this is necessary. It
feels duplicitous with the FindPort API
+ }
+ } else {
+ if (VIR_ALLOC(*port) < 0)
+ goto cleanup;
Ahhh... here's the place where nports == 0 - took me a while to dig into
that... Might be nice to note that this is the "terminus" or final
device of the AddressHub.
FWIW: I'm surprised the bitmap code wasn't used - although that could be
tricky too
+ }
+
+ ret = 0;
+
+ cleanup:
+ VIR_FREE(portstr);
+ return ret;
+}
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 64d35e9..8f866ae 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -269,4 +269,10 @@ int virDomainUSBAddressSetAddControllers(virDomainUSBAddressSetPtr
addrs,
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
void virDomainUSBAddressSetFree(virDomainUSBAddressSetPtr addrs);
+int
+virDomainUSBAddressReserve(virDomainDefPtr def,
+ virDomainDeviceDefPtr dev,
+ virDomainDeviceInfoPtr info,
+ void *data)
+ ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
#endif /* __DOMAIN_ADDR_H__ */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 7db4839..1420b19 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -108,6 +108,7 @@ virDomainPCIAddressSlotInUse;
virDomainPCIAddressValidate;
virDomainUSBAddressGetPortBuf;
virDomainUSBAddressGetPortString;
+virDomainUSBAddressReserve;
virDomainUSBAddressSetAddControllers;
virDomainUSBAddressSetCreate;
virDomainUSBAddressSetFree;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4b520d7..a6a3438 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1563,6 +1563,137 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def,
}
+static void
+qemuDomainCountUSBAddresses(virDomainDefPtr def,
+ size_t *specified,
+ size_t *total)
+{
+ size_t i;
+ size_t spec = 0, tot = 0;
+
+ /* usb-hub */
+ for (i = 0; i < def->nhubs; i++) {
+ virDomainHubDefPtr hub = def->hubs[i];
+ if (hub->type == VIR_DOMAIN_HUB_TYPE_USB) {
+ tot++;
+ if (hub->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB)
+ spec++;
+ }
+ }
+
+ /* 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) {
+ tot++;
+ if (hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB)
+ spec++;
+ }
+ }
+
+ /* usb-storage */
+ for (i = 0; i < def->ndisks; i++) {
+ virDomainDiskDefPtr disk = def->disks[i];
+ if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) {
+ tot++;
+ if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB)
+ spec++;
+ }
+ }
+
+ /* TODO: usb-net */
+
+ /* usb-ccid */
+ for (i = 0; i < def->ncontrollers; i++) {
+ virDomainControllerDefPtr cont = def->controllers[i];
+ if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_CCID) {
+ tot++;
+ if (cont->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB)
+ spec++;
+ }
+ }
+
+ /* 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) {
+ tot++;
+ if (input->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB)
+ spec++;
+ }
+ }
+
+ /* usb-serial */
+ for (i = 0; i < def->nserials; i++) {
+ virDomainChrDefPtr serial = def->serials[i];
+ if (serial->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB) {
+ tot++;
+ if (serial->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB)
+ spec++;
+ }
+ }
+
+ /* usb-audio model=usb */
+ for (i = 0; i < def->nsounds; i++) {
+ virDomainSoundDefPtr sound = def->sounds[i];
+ if (sound->model == VIR_DOMAIN_SOUND_MODEL_USB) {
+ tot++;
+ if (sound->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB)
+ spec++;
+ }
+ }
+
+ *specified = spec;
+ *total = tot;
+}
+
+static int
+qemuDomainAssignUSBAddresses(virDomainDefPtr def,
+ virDomainObjPtr obj,
+ bool newDomain)
+{
+ int ret = -1;
+ size_t total, specified;
+ virDomainUSBAddressSetPtr addrs = NULL;
+ qemuDomainObjPrivatePtr priv = NULL;
+
+ qemuDomainCountUSBAddresses(def, &specified, &total);
+ VIR_DEBUG("%zu out of %zu USB addresses have been specified", specified,
total);
+
+ /* if ALL USB devices have their addresses assigned
+ * or we're creating a new domain, we can continue allocation,
+ * otherwise QEMU might create an incompatible machine by adding hubs
+ * and own addresses */
+ if (!(newDomain || total == specified))
+ return 0;
Does domain persistence matter here? EG do we want to skip this for
transient domains?
John
+
+ if (!(addrs = virDomainUSBAddressSetCreate()))
+ goto cleanup;
+
+ if (virDomainUSBAddressSetAddControllers(addrs, def) < 0)
+ goto cleanup;
+
+ if (virDomainDeviceInfoIterate(def, virDomainUSBAddressReserve, addrs) < 0)
+ goto cleanup;
+
+ VIR_DEBUG("Existing USB addresses have been reserved");
+
+ if (obj && obj->privateData) {
+ priv = obj->privateData;
+ /* if this is the live domain object, we persist the addresses */
+ priv->usbaddrs = addrs;
+ priv->persistentAddrs = 1;
+ addrs = NULL;
+ }
+ ret = 0;
+
+ cleanup:
+ virDomainUSBAddressSetFree(addrs);
+ return ret;
+}
+
+
static int
qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
virDomainDeviceDefPtr device,
@@ -1774,7 +1905,7 @@ qemuDomainPCIBusFullyReserved(virDomainPCIAddressBusPtr bus)
int qemuDomainAssignAddresses(virDomainDefPtr def,
virQEMUCapsPtr qemuCaps,
virDomainObjPtr obj,
- bool newDomain ATTRIBUTE_UNUSED)
+ bool newDomain)
{
int rc;
@@ -1796,6 +1927,10 @@ int qemuDomainAssignAddresses(virDomainDefPtr def,
if (rc)
return rc;
+ rc = qemuDomainAssignUSBAddresses(def, obj, newDomain);
+ if (rc)
+ return rc;
+
return qemuDomainAssignPCIAddresses(def, qemuCaps, obj);
}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 2af7c59..454f759 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -167,6 +167,7 @@ struct _qemuDomainObjPrivate {
virDomainPCIAddressSetPtr pciaddrs;
virDomainCCWAddressSetPtr ccwaddrs;
virDomainVirtioSerialAddrSetPtr vioserialaddrs;
+ virDomainUSBAddressSetPtr usbaddrs;
int persistentAddrs;
virQEMUCapsPtr qemuCaps;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse-addr.args
b/tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse-addr.args
index 07ea004..2f45b03 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse-addr.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse-addr.args
@@ -2,5 +2,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=none \
/usr/bin/qemu -S \
-M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults \
-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \
--hda /dev/HostVG/QEMUGuest1 -device usb-mouse,id=input0,bus=usb.0,port=4 \
+-hda /dev/HostVG/QEMUGuest1 -device usb-mouse,id=input0,bus=usb.0,port=2 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse-addr.xml
b/tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse-addr.xml
index a996838..dde3517 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse-addr.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse-addr.xml
@@ -22,7 +22,7 @@
<controller type='usb' index='0'/>
<controller type='ide' index='0'/>
<input type='mouse' bus='usb'>
- <address type='usb' bus='0' port='4'/>
+ <address type='usb' bus='0' port='2'/>
</input>
</devices>
</domain>
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 8815087..feb1d85 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1171,6 +1171,9 @@ mymain(void)
DO_TEST("usb-hub",
QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_USB_HUB,
QEMU_CAPS_NODEFCONFIG);
+ DO_TEST_ERROR("usb-hub-conflict",
+ QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_USB_HUB,
+ QEMU_CAPS_NODEFCONFIG);
DO_TEST("usb-ports",
QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_USB_HUB,
QEMU_CAPS_NODEFCONFIG);
9:52 a.m.
New subject: [libvirt] [PATCH 8/9] Assign addresses to USB devices
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 | 88 +++++++++++++++++++++-
src/conf/domain_addr.h | 6 ++
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c | 87 +++++++++++++++++++++
...qemuhotplug-console-compat-2+console-virtio.xml | 4 +-
.../qemuxml2argvdata/qemuxml2argv-bios-nvram.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-bios.args | 2 +-
.../qemuxml2argv-console-compat-2.xml | 4 +-
.../qemuxml2argv-controller-order.args | 7 +-
.../qemuxml2argv-controller-order.xml | 2 +
.../qemuxml2argv-disk-usb-device-removable.args | 3 +-
.../qemuxml2argv-disk-usb-device.args | 3 +-
.../qemuxml2argv-graphics-spice-timeout.args | 2 +-
...muxml2argv-hostdev-usb-address-device-boot.args | 2 +-
.../qemuxml2argv-hostdev-usb-address-device.args | 3 +-
.../qemuxml2argv-hugepages-numa.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 | 3 +-
.../qemuxml2argv-smartcard-passthrough-tcp.args | 2 +-
.../qemuxml2argv-sound-device.args | 2 +-
.../qemuxml2argv-usb-port-autoassign.args | 15 ++++
.../qemuxml2argv-usb-port-autoassign.xml | 27 +++++++
tests/qemuxml2argvtest.c | 6 +-
27 files changed, 260 insertions(+), 23 deletions(-)
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 cda6e08..15f4753 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1392,9 +1392,95 @@ int virDomainUSBAddressSetAddControllers(virDomainUSBAddressSetPtr
addrs,
}
+static int
+virDomainUSBAddressFindFreePort(virDomainUSBAddressHubPtr hub,
+ unsigned int *portpath,
+ unsigned int idx)
+{
+ size_t i;
+
+ /* Look for free ports on the current hub */
+ for (i = 1; i < hub->nports; i++) {
+ if (!hub->ports[i]) {
+ VIR_DEBUG("Found a free port %zu at level %u", i, idx);
+ portpath[idx] = i;
+ return 0;
+ }
+ }
+
+ VIR_DEBUG("No ports found on hub %p, trying the hubs on it", hub);
+
+ if (idx >= VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH - 1)
+ return -1;
+
+ /* Recursively search through the ports
+ * that contain another hub */
+ for (i = 1; i < hub->nports; i++) {
+ VIR_DEBUG("level: %u port: %zu, nports: %zu", idx, i,
+ hub->ports[i]->nports);
+
+ if (hub->ports[i]->nports &&
+ virDomainUSBAddressFindFreePort(hub->ports[i],
+ portpath,
+ idx + 1) == 0) {
+ portpath[idx] = i;
+ return 0;
+ }
+
+ }
+ return -1;
+}
+
+
+int
+virDomainUSBAddressAssign(virDomainUSBAddressSetPtr addrs,
+ virDomainDeviceInfoPtr info,
+ bool isHub)
+{
+ unsigned int portpath[VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH] = { 0 };
+ virDomainDeviceDefPtr dev = NULL;
+ char *portstr;
+ size_t i;
+ int ret = -1;
+
+ if (isHub) {
+ if (VIR_ALLOC(dev) < 0)
+ goto cleanup;
+ dev->type = VIR_DOMAIN_DEVICE_HUB;
+ if (VIR_ALLOC(dev->data.hub) < 0)
+ goto cleanup;
+ dev->data.hub->type = VIR_DOMAIN_HUB_TYPE_USB;
+ }
+
+ for (i = 0; i < addrs->nbuses; i++) {
+ virDomainUSBAddressHubPtr hub = addrs->buses[i];
+ if (!hub)
+ continue;
+
+ if (virDomainUSBAddressFindFreePort(hub, portpath, 0) == 0) {
+ info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB;
+ info->addr.usb.bus = i;
+ portstr = virDomainUSBAddressGetPortString(portpath);
+ memcpy(info->addr.usb.port, portpath,
VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH);
+ VIR_DEBUG("Assigning USB addr bus=%u port=%s",
+ info->addr.usb.bus, portstr);
+ if (virDomainUSBAddressReserve(NULL, dev, info, addrs) == 0)
+ ret = 0;
+ VIR_FREE(portstr);
+ goto cleanup;
+ }
+ }
+
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("No free USB
ports"));
+ cleanup:
+ virDomainDeviceDefFree(dev);
+ return ret;
+}
+
+
int
virDomainUSBAddressReserve(virDomainDefPtr def ATTRIBUTE_UNUSED,
- virDomainDeviceDefPtr dev ATTRIBUTE_UNUSED,
+ virDomainDeviceDefPtr dev,
virDomainDeviceInfoPtr info,
void *data)
{
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 8f866ae..8ecf588 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -270,6 +270,12 @@ int virDomainUSBAddressSetAddControllers(virDomainUSBAddressSetPtr
addrs,
void virDomainUSBAddressSetFree(virDomainUSBAddressSetPtr addrs);
int
+virDomainUSBAddressAssign(virDomainUSBAddressSetPtr addrs,
+ virDomainDeviceInfoPtr info,
+ bool hub)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+
+int
virDomainUSBAddressReserve(virDomainDefPtr def,
virDomainDeviceDefPtr dev,
virDomainDeviceInfoPtr info,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1420b19..048ec68 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -106,6 +106,7 @@ virDomainPCIAddressSetFree;
virDomainPCIAddressSetGrow;
virDomainPCIAddressSlotInUse;
virDomainPCIAddressValidate;
+virDomainUSBAddressAssign;
virDomainUSBAddressGetPortBuf;
virDomainUSBAddressGetPortString;
virDomainUSBAddressReserve;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a6a3438..9df542e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1648,6 +1648,88 @@ qemuDomainCountUSBAddresses(virDomainDefPtr def,
*total = tot;
}
+
+static int
+qemuDomainAssignUSBPorts(virDomainUSBAddressSetPtr addrs,
+ virDomainDefPtr def)
+{
+ size_t i;
+
+ /* usb-hub */
+ 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_NONE) {
+ if (virDomainUSBAddressAssign(addrs, &hub->info, true) < 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
&&
+ hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+ if (virDomainUSBAddressAssign(addrs, hostdev->info, false) < 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 &&
+ disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+ if (virDomainUSBAddressAssign(addrs, &disk->info, false) < 0)
+ return -1;
+ }
+ }
+
+ /* TODO: usb-net */
+
+ /* usb-ccid */
+ for (i = 0; i < def->ncontrollers; i++) {
+ virDomainControllerDefPtr cont = def->controllers[i];
+ if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_CCID &&
+ cont->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+ if (virDomainUSBAddressAssign(addrs, &cont->info, false) < 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 &&
+ input->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+ if (virDomainUSBAddressAssign(addrs, &input->info, false) < 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 &&
+ serial->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+ if (virDomainUSBAddressAssign(addrs, &serial->info, false) < 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 &&
+ sound->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+ if (virDomainUSBAddressAssign(addrs, &sound->info, false) < 0)
+ return -1;
+ }
+ }
+ return 0;
+}
+
static int
qemuDomainAssignUSBAddresses(virDomainDefPtr def,
virDomainObjPtr obj,
@@ -1679,6 +1761,11 @@ qemuDomainAssignUSBAddresses(virDomainDefPtr def,
VIR_DEBUG("Existing USB addresses have been reserved");
+ if (qemuDomainAssignUSBPorts(addrs, def) < 0)
+ goto cleanup;
+
+ VIR_DEBUG("Finished assigning USB ports");
+
if (obj && obj->privateData) {
priv = obj->privateData;
/* if this is the live domain object, we persist the addresses */
diff --git a/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2+console-virtio.xml
b/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2+console-virtio.xml
index d848677..9bc01e5 100644
--- a/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2+console-virtio.xml
+++ b/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2+console-virtio.xml
@@ -81,7 +81,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'>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.args
b/tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.args
index b51e8f3..d71ad5d 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.args
@@ -6,5 +6,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=none \
-monitor unix:/tmp/test-monitor,server,nowait -boot c -usb \
-drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-ide0-0-0,format=raw \
-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
--serial pty -device usb-tablet,id=input0 \
+-serial pty -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 e8ef763..ea7d631 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-bios.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-bios.args
@@ -3,5 +3,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=none \
-m 1024 -smp 1 -nographic -nodefaults -device sga \
-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \
-usb -hda /dev/HostVG/QEMUGuest1 -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-console-compat-2.xml
b/tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml
index a7209b2..c9f943f 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'>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-controller-order.args
b/tests/qemuxml2argvdata/qemuxml2argv-controller-order.args
index d68011d..d0c10c5 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-controller-order.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-controller-order.args
@@ -5,7 +5,8 @@ socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon \
chardev=charmonitor,id=monitor,mode=readline -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 -drive \
+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,if=none,id=drive-virtio-disk0,cache=off,aio=native \
-device \
virtio-blk-pci,bus=pci.0,addr=0x5,drive=drive-virtio-disk0,id=virtio-disk0 \
@@ -22,9 +23,9 @@ isa-serial,chardev=charserial0,id=serial0 -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 -spice \
+-device usb-tablet,id=input0,bus=usb.0,port=1.2 -spice \
port=0,addr=0.0.0.0 -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-controller-order.xml
b/tests/qemuxml2argvdata/qemuxml2argv-controller-order.xml
index 07db77e..10f5558 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-controller-order.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-controller-order.xml
@@ -84,6 +84,8 @@
<memballoon model='virtio'>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x06' function='0x0'/>
</memballoon>
+ <hub type='usb'>
+ </hub>
</devices>
<seclabel type='dynamic' model='selinux' relabel='yes'/>
</domain>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.args
b/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.args
index 793c597..53bbdb4 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.args
@@ -4,6 +4,7 @@ pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor \
unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -drive \
file=/dev/HostVG/QEMUGuest1,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,\
-if=none,id=drive-usb-disk0 -device usb-storage,drive=drive-usb-disk0,\
+if=none,id=drive-usb-disk0 \
+-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 d2b80d7..88727fa 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device.args
@@ -4,5 +4,6 @@ pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor \
unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -drive \
file=/dev/HostVG/QEMUGuest1,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,\
-if=none,id=drive-usb-disk0 -device usb-storage,drive=drive-usb-disk0,\
+if=none,id=drive-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 8b5d9ee..2e74a24 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.args
@@ -11,7 +11,7 @@ media=cdrom,id=drive-ide0-1-0 \
-device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 \
-device rtl8139,vlan=0,id=net0,mac=52:54:00:71:70:89,bus=pci.0,addr=0x7 \
-net tap,script=/etc/qemu-ifup,vlan=0,name=hostnet0 -serial pty \
--device usb-tablet,id=input0 \
+-device usb-tablet,id=input0,bus=usb.0,port=1 \
-spice port=5900 -vga std \
-device AC97,id=sound0,bus=pci.0,addr=0x3 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address-device-boot.args
b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address-device-boot.args
index f2cc35d..6b89b79 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address-device-boot.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address-device-boot.args
@@ -3,5 +3,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=none \
pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor \
unix:/tmp/test-monitor,server,nowait -no-acpi -usb -hda \
/dev/HostVG/QEMUGuest1 \
--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 4c73a51..78fed2b 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address-device.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address-device.args
@@ -2,5 +2,6 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=none \
/usr/bin/qemu -S -M \
pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor \
unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \
-/dev/HostVG/QEMUGuest1 -device usb-host,hostbus=14,hostaddr=6,id=hostdev0 \
+/dev/HostVG/QEMUGuest1 \
+-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-hugepages-numa.args
b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.args
index 37511b1..2a15a87 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.args
@@ -34,7 +34,7 @@ chardev=charchannel0,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=0 \
-vga qxl \
-global qxl-vga.ram_size=67108864 \
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-kbd.args
b/tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-kbd.args
index 373c72a..c46eb2d 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-kbd.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-kbd.args
@@ -6,4 +6,4 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=none \
-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 a806d63..e6cff64 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.args
@@ -6,7 +6,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=spice \
-hda /dev/HostVG/QEMUGuest1 \
-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 5e58867..c1d126a 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-controller.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-controller.args
@@ -3,6 +3,6 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=none \
pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev \
socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon \
chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device \
-usb-ccid,id=ccid0 -usb -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 33d3b08..bc8b23a 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host-certificates.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host-certificates.args
@@ -3,7 +3,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=none \
pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev \
socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon \
chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device \
-usb-ccid,id=ccid0 -usb -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 -device \
virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host.args
b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host.args
index 5e58867..c1d126a 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host.args
@@ -3,6 +3,6 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=none \
pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev \
socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon \
chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device \
-usb-ccid,id=ccid0 -usb -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 eed319c..11998d3 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-spicevmc.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-spicevmc.args
@@ -3,6 +3,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=none \
pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev \
socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon \
chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device \
-usb-ccid,id=ccid0 -usb -chardev spicevmc,id=charsmartcard0,name=smartcard \
+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 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-tcp.args
b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-tcp.args
index c350977..284eb58 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-tcp.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-tcp.args
@@ -3,7 +3,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=none \
pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev \
socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon \
chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device \
-usb-ccid,id=ccid0 -usb -chardev \
+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 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-sound-device.args
b/tests/qemuxml2argvdata/qemuxml2argv-sound-device.args
index 3e2b293..bb0c845 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-sound-device.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-sound-device.args
@@ -14,5 +14,5 @@ id=sound6-codec0,bus=sound6.0,cad=0 \
-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-port-autoassign.args
b/tests/qemuxml2argvdata/qemuxml2argv-usb-port-autoassign.args
new file mode 100644
index 0000000..d61cb29
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-port-autoassign.args
@@ -0,0 +1,15 @@
+LC_ALL=C PATH=/bin HOME=/home/test \
+USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/bin/qemu -S -M pc -m 214 \
+-smp 1 -nographic -nodefconfig -nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/test-monitor,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/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index feb1d85..39ce7e2 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -678,7 +678,8 @@ mymain(void)
QEMU_CAPS_BOOT_MENU, QEMU_CAPS_PIIX3_USB_UHCI,
QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_DRIVE_AIO,
QEMU_CAPS_CCID_PASSTHRU, QEMU_CAPS_CHARDEV,
- QEMU_CAPS_CHARDEV_SPICEVMC, QEMU_CAPS_SPICE, QEMU_CAPS_HDA_DUPLEX);
+ QEMU_CAPS_CHARDEV_SPICEVMC, QEMU_CAPS_SPICE, QEMU_CAPS_HDA_DUPLEX,
+ QEMU_CAPS_USB_HUB);
DO_TEST("eoi-disabled", NONE);
DO_TEST("eoi-enabled", NONE);
DO_TEST("pv-spinlock-disabled", NONE);
@@ -1177,6 +1178,9 @@ mymain(void)
DO_TEST("usb-ports",
QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_USB_HUB,
QEMU_CAPS_NODEFCONFIG);
+ DO_TEST("usb-port-autoassign",
+ QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_USB_HUB,
+ QEMU_CAPS_NODEFCONFIG);
DO_TEST("usb-redir",
QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG,
QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_USB_HUB,
--
2.4.6
9:47 a.m.
New subject: [libvirt] [PATCH 8/9] Assign addresses to USB devices
On 08/12/2015 10:52 AM, Ján Tomko wrote:
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 | 88 +++++++++++++++++++++-
src/conf/domain_addr.h | 6 ++
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c | 87 +++++++++++++++++++++
...qemuhotplug-console-compat-2+console-virtio.xml | 4 +-
.../qemuxml2argvdata/qemuxml2argv-bios-nvram.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-bios.args | 2 +-
.../qemuxml2argv-console-compat-2.xml | 4 +-
.../qemuxml2argv-controller-order.args | 7 +-
.../qemuxml2argv-controller-order.xml | 2 +
.../qemuxml2argv-disk-usb-device-removable.args | 3 +-
.../qemuxml2argv-disk-usb-device.args | 3 +-
.../qemuxml2argv-graphics-spice-timeout.args | 2 +-
...muxml2argv-hostdev-usb-address-device-boot.args | 2 +-
.../qemuxml2argv-hostdev-usb-address-device.args | 3 +-
.../qemuxml2argv-hugepages-numa.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 | 3 +-
.../qemuxml2argv-smartcard-passthrough-tcp.args | 2 +-
.../qemuxml2argv-sound-device.args | 2 +-
.../qemuxml2argv-usb-port-autoassign.args | 15 ++++
.../qemuxml2argv-usb-port-autoassign.xml | 27 +++++++
tests/qemuxml2argvtest.c | 6 +-
27 files changed, 260 insertions(+), 23 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-port-autoassign.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-port-autoassign.xml
NB: Less thorough thinking than last 3...
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index cda6e08..15f4753 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1392,9 +1392,95 @@ int virDomainUSBAddressSetAddControllers(virDomainUSBAddressSetPtr
addrs,
}
+static int
+virDomainUSBAddressFindFreePort(virDomainUSBAddressHubPtr hub,
+ unsigned int *portpath,
+ unsigned int idx)
+{
+ size_t i;
+
+ /* Look for free ports on the current hub */
+ for (i = 1; i < hub->nports; i++) {
+ if (!hub->ports[i]) {
+ VIR_DEBUG("Found a free port %zu at level %u", i, idx);
+ portpath[idx] = i;
+ return 0;
+ }
+ }
This seems to indicate something can be plugged directly into the root
hub... Definitely would help me for some kind of picture <sigh>.
+
+ VIR_DEBUG("No ports found on hub %p, trying the hubs on it", hub);
+
+ if (idx >= VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH - 1)
+ return -1;
+
+ /* Recursively search through the ports
+ * that contain another hub */
+ for (i = 1; i < hub->nports; i++) {
+ VIR_DEBUG("level: %u port: %zu, nports: %zu", idx, i,
+ hub->ports[i]->nports);
+
+ if (hub->ports[i]->nports &&
+ virDomainUSBAddressFindFreePort(hub->ports[i],
+ portpath,
+ idx + 1) == 0) {
+ portpath[idx] = i;
+ return 0;
+ }
+
+ }
+ return -1;
+}
+
+
+int
+virDomainUSBAddressAssign(virDomainUSBAddressSetPtr addrs,
+ virDomainDeviceInfoPtr info,
+ bool isHub)
+{
+ unsigned int portpath[VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH] = { 0 };
+ virDomainDeviceDefPtr dev = NULL;
+ char *portstr;
+ size_t i;
+ int ret = -1;
+
+ if (isHub) {
+ if (VIR_ALLOC(dev) < 0)
+ goto cleanup;
+ dev->type = VIR_DOMAIN_DEVICE_HUB;
+ if (VIR_ALLOC(dev->data.hub) < 0)
+ goto cleanup;
+ dev->data.hub->type = VIR_DOMAIN_HUB_TYPE_USB;
+ }
+
+ for (i = 0; i < addrs->nbuses; i++) {
+ virDomainUSBAddressHubPtr hub = addrs->buses[i];
+ if (!hub)
+ continue;
+
+ if (virDomainUSBAddressFindFreePort(hub, portpath, 0) == 0) {
+ info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB;
+ info->addr.usb.bus = i;
+ portstr = virDomainUSBAddressGetPortString(portpath);
What happens if portstr == NULL ?
+ memcpy(info->addr.usb.port, portpath,
VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH);
+ VIR_DEBUG("Assigning USB addr bus=%u port=%s",
+ info->addr.usb.bus, portstr);
+ if (virDomainUSBAddressReserve(NULL, dev, info, addrs) == 0)
+ ret = 0;
+ VIR_FREE(portstr);
+ goto cleanup;
+ }
+ }
+
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("No free USB
ports"));
+ cleanup:
+ virDomainDeviceDefFree(dev);
+ return ret;
+}
+
+
int
virDomainUSBAddressReserve(virDomainDefPtr def ATTRIBUTE_UNUSED,
- virDomainDeviceDefPtr dev ATTRIBUTE_UNUSED,
+ virDomainDeviceDefPtr dev,
virDomainDeviceInfoPtr info,
void *data)
{
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 8f866ae..8ecf588 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -270,6 +270,12 @@ int virDomainUSBAddressSetAddControllers(virDomainUSBAddressSetPtr
addrs,
void virDomainUSBAddressSetFree(virDomainUSBAddressSetPtr addrs);
int
+virDomainUSBAddressAssign(virDomainUSBAddressSetPtr addrs,
+ virDomainDeviceInfoPtr info,
+ bool hub)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+
+int
virDomainUSBAddressReserve(virDomainDefPtr def,
virDomainDeviceDefPtr dev,
virDomainDeviceInfoPtr info,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1420b19..048ec68 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -106,6 +106,7 @@ virDomainPCIAddressSetFree;
virDomainPCIAddressSetGrow;
virDomainPCIAddressSlotInUse;
virDomainPCIAddressValidate;
+virDomainUSBAddressAssign;
virDomainUSBAddressGetPortBuf;
virDomainUSBAddressGetPortString;
virDomainUSBAddressReserve;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a6a3438..9df542e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1648,6 +1648,88 @@ qemuDomainCountUSBAddresses(virDomainDefPtr def,
*total = tot;
}
+
+static int
+qemuDomainAssignUSBPorts(virDomainUSBAddressSetPtr addrs,
+ virDomainDefPtr def)
+{
+ size_t i;
+
+ /* usb-hub */
+ 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_NONE) {
+ if (virDomainUSBAddressAssign(addrs, &hub->info, true) < 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
&&
+ hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+ if (virDomainUSBAddressAssign(addrs, hostdev->info, false) < 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 &&
+ disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+ if (virDomainUSBAddressAssign(addrs, &disk->info, false) < 0)
+ return -1;
+ }
+ }
+
+ /* TODO: usb-net */
??
John
+
+ /* usb-ccid */
+ for (i = 0; i < def->ncontrollers; i++) {
+ virDomainControllerDefPtr cont = def->controllers[i];
+ if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_CCID &&
+ cont->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+ if (virDomainUSBAddressAssign(addrs, &cont->info, false) < 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 &&
+ input->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+ if (virDomainUSBAddressAssign(addrs, &input->info, false) < 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 &&
+ serial->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+ if (virDomainUSBAddressAssign(addrs, &serial->info, false) < 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 &&
+ sound->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+ if (virDomainUSBAddressAssign(addrs, &sound->info, false) < 0)
+ return -1;
+ }
+ }
+ return 0;
+}
+
static int
qemuDomainAssignUSBAddresses(virDomainDefPtr def,
virDomainObjPtr obj,
@@ -1679,6 +1761,11 @@ qemuDomainAssignUSBAddresses(virDomainDefPtr def,
VIR_DEBUG("Existing USB addresses have been reserved");
+ if (qemuDomainAssignUSBPorts(addrs, def) < 0)
+ goto cleanup;
+
+ VIR_DEBUG("Finished assigning USB ports");
+
if (obj && obj->privateData) {
priv = obj->privateData;
/* if this is the live domain object, we persist the addresses */
diff --git a/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2+console-virtio.xml
b/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2+console-virtio.xml
index d848677..9bc01e5 100644
--- a/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2+console-virtio.xml
+++ b/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2+console-virtio.xml
@@ -81,7 +81,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'>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.args
b/tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.args
index b51e8f3..d71ad5d 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.args
@@ -6,5 +6,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=none \
-monitor unix:/tmp/test-monitor,server,nowait -boot c -usb \
-drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-ide0-0-0,format=raw \
-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
--serial pty -device usb-tablet,id=input0 \
+-serial pty -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 e8ef763..ea7d631 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-bios.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-bios.args
@@ -3,5 +3,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=none \
-m 1024 -smp 1 -nographic -nodefaults -device sga \
-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \
-usb -hda /dev/HostVG/QEMUGuest1 -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-console-compat-2.xml
b/tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml
index a7209b2..c9f943f 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'>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-controller-order.args
b/tests/qemuxml2argvdata/qemuxml2argv-controller-order.args
index d68011d..d0c10c5 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-controller-order.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-controller-order.args
@@ -5,7 +5,8 @@ socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon \
chardev=charmonitor,id=monitor,mode=readline -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 -drive \
+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,if=none,id=drive-virtio-disk0,cache=off,aio=native \
-device \
virtio-blk-pci,bus=pci.0,addr=0x5,drive=drive-virtio-disk0,id=virtio-disk0 \
@@ -22,9 +23,9 @@ isa-serial,chardev=charserial0,id=serial0 -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 -spice \
+-device usb-tablet,id=input0,bus=usb.0,port=1.2 -spice \
port=0,addr=0.0.0.0 -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-controller-order.xml
b/tests/qemuxml2argvdata/qemuxml2argv-controller-order.xml
index 07db77e..10f5558 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-controller-order.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-controller-order.xml
@@ -84,6 +84,8 @@
<memballoon model='virtio'>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x06' function='0x0'/>
</memballoon>
+ <hub type='usb'>
+ </hub>
</devices>
<seclabel type='dynamic' model='selinux'
relabel='yes'/>
</domain>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.args
b/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.args
index 793c597..53bbdb4 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.args
@@ -4,6 +4,7 @@ pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor \
unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -drive \
file=/dev/HostVG/QEMUGuest1,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,\
-if=none,id=drive-usb-disk0 -device usb-storage,drive=drive-usb-disk0,\
+if=none,id=drive-usb-disk0 \
+-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 d2b80d7..88727fa 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device.args
@@ -4,5 +4,6 @@ pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor \
unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -drive \
file=/dev/HostVG/QEMUGuest1,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,\
-if=none,id=drive-usb-disk0 -device usb-storage,drive=drive-usb-disk0,\
+if=none,id=drive-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 8b5d9ee..2e74a24 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.args
@@ -11,7 +11,7 @@ media=cdrom,id=drive-ide0-1-0 \
-device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 \
-device rtl8139,vlan=0,id=net0,mac=52:54:00:71:70:89,bus=pci.0,addr=0x7 \
-net tap,script=/etc/qemu-ifup,vlan=0,name=hostnet0 -serial pty \
--device usb-tablet,id=input0 \
+-device usb-tablet,id=input0,bus=usb.0,port=1 \
-spice port=5900 -vga std \
-device AC97,id=sound0,bus=pci.0,addr=0x3 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address-device-boot.args
b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address-device-boot.args
index f2cc35d..6b89b79 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address-device-boot.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address-device-boot.args
@@ -3,5 +3,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=none \
pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor \
unix:/tmp/test-monitor,server,nowait -no-acpi -usb -hda \
/dev/HostVG/QEMUGuest1 \
--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 4c73a51..78fed2b 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address-device.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address-device.args
@@ -2,5 +2,6 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=none \
/usr/bin/qemu -S -M \
pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor \
unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \
-/dev/HostVG/QEMUGuest1 -device usb-host,hostbus=14,hostaddr=6,id=hostdev0 \
+/dev/HostVG/QEMUGuest1 \
+-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-hugepages-numa.args
b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.args
index 37511b1..2a15a87 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.args
@@ -34,7 +34,7 @@ chardev=charchannel0,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=0 \
-vga qxl \
-global qxl-vga.ram_size=67108864 \
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-kbd.args
b/tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-kbd.args
index 373c72a..c46eb2d 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-kbd.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-kbd.args
@@ -6,4 +6,4 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=none \
-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 a806d63..e6cff64 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.args
@@ -6,7 +6,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=spice \
-hda /dev/HostVG/QEMUGuest1 \
-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 5e58867..c1d126a 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-controller.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-controller.args
@@ -3,6 +3,6 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=none \
pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev \
socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon \
chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device \
-usb-ccid,id=ccid0 -usb -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 33d3b08..bc8b23a 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host-certificates.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host-certificates.args
@@ -3,7 +3,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=none \
pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev \
socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon \
chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device \
-usb-ccid,id=ccid0 -usb -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 -device \
virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host.args
b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host.args
index 5e58867..c1d126a 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host.args
@@ -3,6 +3,6 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=none \
pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev \
socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon \
chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device \
-usb-ccid,id=ccid0 -usb -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 eed319c..11998d3 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-spicevmc.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-spicevmc.args
@@ -3,6 +3,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=none \
pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev \
socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon \
chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device \
-usb-ccid,id=ccid0 -usb -chardev spicevmc,id=charsmartcard0,name=smartcard \
+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 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-tcp.args
b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-tcp.args
index c350977..284eb58 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-tcp.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-tcp.args
@@ -3,7 +3,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=none \
pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev \
socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon \
chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device \
-usb-ccid,id=ccid0 -usb -chardev \
+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 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-sound-device.args
b/tests/qemuxml2argvdata/qemuxml2argv-sound-device.args
index 3e2b293..bb0c845 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-sound-device.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-sound-device.args
@@ -14,5 +14,5 @@ id=sound6-codec0,bus=sound6.0,cad=0 \
-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-port-autoassign.args
b/tests/qemuxml2argvdata/qemuxml2argv-usb-port-autoassign.args
new file mode 100644
index 0000000..d61cb29
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-port-autoassign.args
@@ -0,0 +1,15 @@
+LC_ALL=C PATH=/bin HOME=/home/test \
+USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/bin/qemu -S -M pc -m 214 \
+-smp 1 -nographic -nodefconfig -nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/test-monitor,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/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index feb1d85..39ce7e2 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -678,7 +678,8 @@ mymain(void)
QEMU_CAPS_BOOT_MENU, QEMU_CAPS_PIIX3_USB_UHCI,
QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_DRIVE_AIO,
QEMU_CAPS_CCID_PASSTHRU, QEMU_CAPS_CHARDEV,
- QEMU_CAPS_CHARDEV_SPICEVMC, QEMU_CAPS_SPICE, QEMU_CAPS_HDA_DUPLEX);
+ QEMU_CAPS_CHARDEV_SPICEVMC, QEMU_CAPS_SPICE, QEMU_CAPS_HDA_DUPLEX,
+ QEMU_CAPS_USB_HUB);
DO_TEST("eoi-disabled", NONE);
DO_TEST("eoi-enabled", NONE);
DO_TEST("pv-spinlock-disabled", NONE);
@@ -1177,6 +1178,9 @@ mymain(void)
DO_TEST("usb-ports",
QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_USB_HUB,
QEMU_CAPS_NODEFCONFIG);
+ DO_TEST("usb-port-autoassign",
+ QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_USB_HUB,
+ QEMU_CAPS_NODEFCONFIG);
DO_TEST("usb-redir",
QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG,
QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_USB_HUB,
9:52 a.m.
New subject: [libvirt] [PATCH 9/9] Assign addresses on USB device hotplug
USB disks, redirected devices, host devices and serial devices
are supported.
---
src/conf/domain_addr.c | 34 +++++++++++++++++++
src/conf/domain_addr.h | 4 +++
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c | 5 +++
src/qemu/qemu_hotplug.c | 39 ++++++++++++++++++++++
.../qemuhotplug-hotplug-base+disk-usb.xml | 1 +
6 files changed, 84 insertions(+)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 15f4753..42c0837 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1522,3 +1522,37 @@ virDomainUSBAddressReserve(virDomainDefPtr def ATTRIBUTE_UNUSED,
VIR_FREE(portstr);
return ret;
}
+
+
+int
+virDomainUSBAddressRelease(virDomainUSBAddressSetPtr addrs,
+ virDomainDeviceInfoPtr info)
+{
+ virDomainUSBAddressHubPtr *port = NULL;
+ char *portstr = NULL;
+ int ret = -1;
+
+ if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB)
+ return 0;
+
+ portstr = virDomainUSBAddressGetPortString(info->addr.usb.port);
+ VIR_DEBUG("Releasing USB addr bus=%u port=%s", info->addr.usb.bus,
portstr);
+
+ if (virDomainUSBAddressFindPort(&port, addrs, info) < 0)
+ goto cleanup;
+
+ if (!*port) {
+ if ((*port)->nports) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("usb-hub hotunplug is not yet implemented"));
+ goto cleanup;
+ }
+ VIR_FREE(*port);
+ }
+
+ ret = 0;
+
+ cleanup:
+ VIR_FREE(portstr);
+ return ret;
+}
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 8ecf588..fbd9557 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -281,4 +281,8 @@ virDomainUSBAddressReserve(virDomainDefPtr def,
virDomainDeviceInfoPtr info,
void *data)
ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
+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 048ec68..9c53b13 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -109,6 +109,7 @@ virDomainPCIAddressValidate;
virDomainUSBAddressAssign;
virDomainUSBAddressGetPortBuf;
virDomainUSBAddressGetPortString;
+virDomainUSBAddressRelease;
virDomainUSBAddressReserve;
virDomainUSBAddressSetAddControllers;
virDomainUSBAddressSetCreate;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9df542e..277e431 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2106,6 +2106,11 @@ 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 8e38153..e72237a 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -669,6 +669,7 @@ qemuDomainAttachUSBMassStorageDevice(virConnectPtr conn,
char *devstr = NULL;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
const char *src = virDomainDiskGetSource(disk);
+ bool releaseaddr;
for (i = 0; i < vm->def->ndisks; i++) {
if (STREQ(vm->def->disks[i]->dst, disk->dst)) {
@@ -678,6 +679,16 @@ qemuDomainAttachUSBMassStorageDevice(virConnectPtr conn,
}
}
+ if (priv->usbaddrs) {
+ if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) {
+ if (virDomainUSBAddressReserve(NULL, NULL, &disk->info,
priv->usbaddrs) < 0)
+ goto cleanup;
+ } else if (virDomainUSBAddressAssign(priv->usbaddrs, &disk->info,
false) < 0) {
+ goto cleanup;
+ }
+ releaseaddr = true;
+ }
+
if (qemuDomainPrepareDisk(driver, vm, disk, NULL, false) < 0)
goto cleanup;
@@ -728,6 +739,8 @@ qemuDomainAttachUSBMassStorageDevice(virConnectPtr conn,
virDomainDiskInsertPreAlloced(vm->def, disk);
cleanup:
+ if (ret < 0 && releaseaddr)
+ virDomainUSBAddressRelease(priv->usbaddrs, &disk->info);
VIR_FREE(devstr);
VIR_FREE(drivestr);
virObjectUnref(cfg);
@@ -1536,6 +1549,16 @@ 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 (chr->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) {
+ if (virDomainUSBAddressReserve(NULL, NULL, &chr->info,
priv->usbaddrs) < 0)
+ return -1;
+ } else if (virDomainUSBAddressAssign(priv->usbaddrs, &chr->info, false)
< 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,
@@ -1845,11 +1868,22 @@ 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 (hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) {
+ if (virDomainUSBAddressReserve(NULL, NULL, hostdev->info,
priv->usbaddrs) < 0)
+ return -1;
+ } else if (virDomainUSBAddressAssign(priv->usbaddrs, hostdev->info, false)
< 0) {
+ goto cleanup;
+ }
+ releaseaddr = true;
+ }
+
if (qemuPrepareHostUSBDevices(driver, vm->def->name, &hostdev, 1, 0) <
0)
goto cleanup;
@@ -1902,6 +1936,8 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
VIR_WARN("Unable to restore host device labelling on hotplug
fail");
if (added)
qemuDomainReAttachHostUSBDevices(driver, vm->def->name, &hostdev,
1);
+ if (releaseaddr)
+ virDomainUSBAddressRelease(priv->usbaddrs, hostdev->info);
}
VIR_FREE(devstr);
return ret;
@@ -2856,6 +2892,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;
@@ -2949,6 +2987,7 @@ qemuDomainRemoveUSBHostDevice(virQEMUDriverPtr driver,
virDomainHostdevDefPtr hostdev)
{
qemuDomainReAttachHostUSBDevices(driver, vm->def->name, &hostdev, 1);
+ qemuDomainReleaseDeviceAddress(vm, hostdev->info, NULL);
}
static void
diff --git a/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+disk-usb.xml
b/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+disk-usb.xml
index 7904c4f..ac69490 100644
--- a/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+disk-usb.xml
+++ b/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+disk-usb.xml
@@ -25,6 +25,7 @@
<target dev='sdq' bus='usb'/>
<readonly/>
<shareable/>
+ <address type='usb' bus='0' port='1'/>
</disk>
<controller type='usb' index='0'>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x2'/>
--
2.4.6
5:18 p.m.
New subject: [libvirt] [PATCH 9/9] Assign addresses on USB device hotplug
On 08/12/2015 10:52 AM, Ján Tomko wrote:
USB disks, redirected devices, host devices and serial devices
are supported.
---
src/conf/domain_addr.c | 34 +++++++++++++++++++
src/conf/domain_addr.h | 4 +++
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c | 5 +++
src/qemu/qemu_hotplug.c | 39 ++++++++++++++++++++++
.../qemuhotplug-hotplug-base+disk-usb.xml | 1 +
6 files changed, 84 insertions(+)
I ran all the changes through my Coverity checker - figured I'd post
this data first...
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 15f4753..42c0837 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1522,3 +1522,37 @@ virDomainUSBAddressReserve(virDomainDefPtr def ATTRIBUTE_UNUSED,
VIR_FREE(portstr);
return ret;
}
+
+
+int
+virDomainUSBAddressRelease(virDomainUSBAddressSetPtr addrs,
+ virDomainDeviceInfoPtr info)
+{
+ virDomainUSBAddressHubPtr *port = NULL;
+ char *portstr = NULL;
+ int ret = -1;
+
+ if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB)
+ return 0;
+
+ portstr = virDomainUSBAddressGetPortString(info->addr.usb.port);
+ VIR_DEBUG("Releasing USB addr bus=%u port=%s", info->addr.usb.bus,
portstr);
+
+ if (virDomainUSBAddressFindPort(&port, addrs, info) < 0)
+ goto cleanup;
+
+ if (!*port) {
+ if ((*port)->nports) {
Coverity complains here ... checking (!*port) and then using (*port) is
counter-productive. I assume you meant "if (!port)"
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("usb-hub hotunplug is not yet implemented"));
+ goto cleanup;
+ }
+ VIR_FREE(*port);
+ }
+
+ ret = 0;
+
+ cleanup:
+ VIR_FREE(portstr);
+ return ret;
+}
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 8ecf588..fbd9557 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -281,4 +281,8 @@ virDomainUSBAddressReserve(virDomainDefPtr def,
virDomainDeviceInfoPtr info,
void *data)
ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
+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 048ec68..9c53b13 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -109,6 +109,7 @@ virDomainPCIAddressValidate;
virDomainUSBAddressAssign;
virDomainUSBAddressGetPortBuf;
virDomainUSBAddressGetPortString;
+virDomainUSBAddressRelease;
virDomainUSBAddressReserve;
virDomainUSBAddressSetAddControllers;
virDomainUSBAddressSetCreate;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9df542e..277e431 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2106,6 +2106,11 @@ 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 8e38153..e72237a 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -669,6 +669,7 @@ qemuDomainAttachUSBMassStorageDevice(virConnectPtr conn,
char *devstr = NULL;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
const char *src = virDomainDiskGetSource(disk);
+ bool releaseaddr;
Not initialized for all paths to cleanup - if initialized to false will
remove 2 Coverity issues...
for (i = 0; i < vm->def->ndisks; i++) {
if (STREQ(vm->def->disks[i]->dst, disk->dst)) {
@@ -678,6 +679,16 @@ qemuDomainAttachUSBMassStorageDevice(virConnectPtr conn,
}
}
+ if (priv->usbaddrs) {
Because you check for priv->usbaddrs here...
+ if (disk->info.type ==
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) {
+ if (virDomainUSBAddressReserve(NULL, NULL, &disk->info,
priv->usbaddrs) < 0)
+ goto cleanup;
+ } else if (virDomainUSBAddressAssign(priv->usbaddrs, &disk->info,
false) < 0) {
+ goto cleanup;
+ }
+ releaseaddr = true;
+ }
+
if (qemuDomainPrepareDisk(driver, vm, disk, NULL, false) < 0)
goto cleanup;
@@ -728,6 +739,8 @@ qemuDomainAttachUSBMassStorageDevice(virConnectPtr conn,
virDomainDiskInsertPreAlloced(vm->def, disk);
cleanup:
+ if (ret < 0 && releaseaddr)
+ virDomainUSBAddressRelease(priv->usbaddrs, &disk->info);
... coverity complains here since priv->usbaddrs was checked for NULL,
but if releaseaddr was initialized to false, then both issues are cleared.
VIR_FREE(devstr);
VIR_FREE(drivestr);
virObjectUnref(cfg);
@@ -1536,6 +1549,16 @@ 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 (chr->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) {
+ if (virDomainUSBAddressReserve(NULL, NULL, &chr->info,
priv->usbaddrs) < 0)
+ return -1;
+ } else if (virDomainUSBAddressAssign(priv->usbaddrs, &chr->info,
false) < 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,
@@ -1845,11 +1868,22 @@ 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 (hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) {
+ if (virDomainUSBAddressReserve(NULL, NULL, hostdev->info,
priv->usbaddrs) < 0)
+ return -1;
+ } else if (virDomainUSBAddressAssign(priv->usbaddrs, hostdev->info, false)
< 0) {
+ goto cleanup;
+ }
+ releaseaddr = true;
+ }
+
if (qemuPrepareHostUSBDevices(driver, vm->def->name, &hostdev, 1, 0) <
0)
goto cleanup;
@@ -1902,6 +1936,8 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
VIR_WARN("Unable to restore host device labelling on hotplug
fail");
if (added)
qemuDomainReAttachHostUSBDevices(driver, vm->def->name, &hostdev,
1);
+ if (releaseaddr)
+ virDomainUSBAddressRelease(priv->usbaddrs, hostdev->info);
}
VIR_FREE(devstr);
return ret;
@@ -2856,6 +2892,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;
@@ -2949,6 +2987,7 @@ qemuDomainRemoveUSBHostDevice(virQEMUDriverPtr driver,
virDomainHostdevDefPtr hostdev)
{
qemuDomainReAttachHostUSBDevices(driver, vm->def->name, &hostdev, 1);
+ qemuDomainReleaseDeviceAddress(vm, hostdev->info, NULL);
}
static void
diff --git a/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+disk-usb.xml
b/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+disk-usb.xml
index 7904c4f..ac69490 100644
--- a/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+disk-usb.xml
+++ b/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+disk-usb.xml
@@ -25,6 +25,7 @@
<target dev='sdq' bus='usb'/>
<readonly/>
<shareable/>
+ <address type='usb' bus='0' port='1'/>
</disk>
<controller type='usb' index='0'>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x2'/>
9:49 a.m.
New subject: [libvirt] [PATCH 9/9] Assign addresses on USB device hotplug
On 08/12/2015 10:52 AM, Ján Tomko wrote:
USB disks, redirected devices, host devices and serial devices
are supported.
---
src/conf/domain_addr.c | 34 +++++++++++++++++++
src/conf/domain_addr.h | 4 +++
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c | 5 +++
src/qemu/qemu_hotplug.c | 39 ++++++++++++++++++++++
.../qemuhotplug-hotplug-base+disk-usb.xml | 1 +
6 files changed, 84 insertions(+)
Like patch 8 - less in depth analysis here.
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 15f4753..42c0837 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1522,3 +1522,37 @@ virDomainUSBAddressReserve(virDomainDefPtr def ATTRIBUTE_UNUSED,
VIR_FREE(portstr);
return ret;
}
+
+
+int
+virDomainUSBAddressRelease(virDomainUSBAddressSetPtr addrs,
+ virDomainDeviceInfoPtr info)
+{
+ virDomainUSBAddressHubPtr *port = NULL;
+ char *portstr = NULL;
+ int ret = -1;
+
+ if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB)
+ return 0;
+
+ portstr = virDomainUSBAddressGetPortString(info->addr.usb.port);
What if portstr == NULL
+ VIR_DEBUG("Releasing USB addr bus=%u port=%s",
info->addr.usb.bus, portstr);
+
+ if (virDomainUSBAddressFindPort(&port, addrs, info) < 0)
+ goto cleanup;
+
+ if (!*port) {
+ if ((*port)->nports) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("usb-hub hotunplug is not yet implemented"));
+ goto cleanup;
+ }
+ VIR_FREE(*port);
+ }
+
+ ret = 0;
+
+ cleanup:
+ VIR_FREE(portstr);
+ return ret;
+}
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 8ecf588..fbd9557 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -281,4 +281,8 @@ virDomainUSBAddressReserve(virDomainDefPtr def,
virDomainDeviceInfoPtr info,
void *data)
ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
+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 048ec68..9c53b13 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -109,6 +109,7 @@ virDomainPCIAddressValidate;
virDomainUSBAddressAssign;
virDomainUSBAddressGetPortBuf;
virDomainUSBAddressGetPortString;
+virDomainUSBAddressRelease;
virDomainUSBAddressReserve;
virDomainUSBAddressSetAddControllers;
virDomainUSBAddressSetCreate;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9df542e..277e431 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2106,6 +2106,11 @@ 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 8e38153..e72237a 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -669,6 +669,7 @@ qemuDomainAttachUSBMassStorageDevice(virConnectPtr conn,
char *devstr = NULL;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
const char *src = virDomainDiskGetSource(disk);
+ bool releaseaddr;
for (i = 0; i < vm->def->ndisks; i++) {
if (STREQ(vm->def->disks[i]->dst, disk->dst)) {
@@ -678,6 +679,16 @@ qemuDomainAttachUSBMassStorageDevice(virConnectPtr conn,
}
}
+ if (priv->usbaddrs) {
+ if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) {
+ if (virDomainUSBAddressReserve(NULL, NULL, &disk->info,
priv->usbaddrs) < 0)
+ goto cleanup;
+ } else if (virDomainUSBAddressAssign(priv->usbaddrs, &disk->info,
false) < 0) {
+ goto cleanup;
+ }
+ releaseaddr = true;
+ }
+
if (qemuDomainPrepareDisk(driver, vm, disk, NULL, false) < 0)
goto cleanup;
@@ -728,6 +739,8 @@ qemuDomainAttachUSBMassStorageDevice(virConnectPtr conn,
virDomainDiskInsertPreAlloced(vm->def, disk);
cleanup:
+ if (ret < 0 && releaseaddr)
+ virDomainUSBAddressRelease(priv->usbaddrs, &disk->info);
VIR_FREE(devstr);
VIR_FREE(drivestr);
virObjectUnref(cfg);
@@ -1536,6 +1549,16 @@ 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) {
What if !priv->usbaddrs
John
+ if (chr->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB)
{
+ if (virDomainUSBAddressReserve(NULL, NULL, &chr->info,
priv->usbaddrs) < 0)
+ return -1;
+ } else if (virDomainUSBAddressAssign(priv->usbaddrs, &chr->info,
false) < 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,
@@ -1845,11 +1868,22 @@ 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 (hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) {
+ if (virDomainUSBAddressReserve(NULL, NULL, hostdev->info,
priv->usbaddrs) < 0)
+ return -1;
+ } else if (virDomainUSBAddressAssign(priv->usbaddrs, hostdev->info, false)
< 0) {
+ goto cleanup;
+ }
+ releaseaddr = true;
+ }
+
if (qemuPrepareHostUSBDevices(driver, vm->def->name, &hostdev, 1, 0) <
0)
goto cleanup;
@@ -1902,6 +1936,8 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
VIR_WARN("Unable to restore host device labelling on hotplug
fail");
if (added)
qemuDomainReAttachHostUSBDevices(driver, vm->def->name, &hostdev,
1);
+ if (releaseaddr)
+ virDomainUSBAddressRelease(priv->usbaddrs, hostdev->info);
}
VIR_FREE(devstr);
return ret;
@@ -2856,6 +2892,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;
@@ -2949,6 +2987,7 @@ qemuDomainRemoveUSBHostDevice(virQEMUDriverPtr driver,
virDomainHostdevDefPtr hostdev)
{
qemuDomainReAttachHostUSBDevices(driver, vm->def->name, &hostdev, 1);
+ qemuDomainReleaseDeviceAddress(vm, hostdev->info, NULL);
}
static void
diff --git a/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+disk-usb.xml
b/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+disk-usb.xml
index 7904c4f..ac69490 100644
--- a/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+disk-usb.xml
+++ b/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+disk-usb.xml
@@ -25,6 +25,7 @@
<target dev='sdq' bus='usb'/>
<readonly/>
<shareable/>
+ <address type='usb' bus='0' port='1'/>
</disk>
<controller type='usb' index='0'>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x2'/>
3377
days inactive
3393
days old
20 comments
2 participants
participants (2)
-
John Ferlan -
Ján Tomko