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);