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