[libvirt] [PATCHv4 9/9] Auto-add one hub if there are too many USB devices
John Ferlan
jferlan at redhat.com
Thu Jul 14 17:00:09 UTC 2016
On 07/01/2016 11:38 AM, Ján Tomko wrote:
> When parsing a command line with USB devices that have
> no address specified, QEMU automatically adds a USB hub
> if the device would fill up all the available USB ports.
>
> To help most of the users, add one hub if there are more
> USB devices than available ports. For wilder configurations,
> expect the user to provide us with more hubs and/or controllers.
> ---
> src/conf/domain_addr.c | 20 +++++++++
> src/conf/domain_addr.h | 2 +
> src/libvirt_private.syms | 1 +
> src/qemu/qemu_domain_address.c | 48 ++++++++++++++++++++++
> .../qemuxml2argv-usb-hub-autoadd.args | 28 +++++++++++++
> .../qemuxml2argv-usb-hub-autoadd.xml | 23 +++++++++++
> tests/qemuxml2argvtest.c | 3 ++
> 7 files changed, 125 insertions(+)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-hub-autoadd.args
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-hub-autoadd.xml
>
Um, just when I thought I figured this USB address stuff out.... I
think I missed or forget something along the way.
Of course by now I've answere my own earlier question about nports for
each usb controller and the bitmap that's created. Still wondering how
1.1.1 gets created, but maybe I'll get that answer. Admittedly I don't
know that much about the USB address/port algorithm.
> diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
> index 491d9c6..0152bbb 100644
> --- a/src/conf/domain_addr.c
> +++ b/src/conf/domain_addr.c
> @@ -1608,6 +1608,26 @@ virDomainUSBAddressFindFreePort(virDomainUSBAddressHubPtr hub,
> }
>
>
This would be All ports rather than Available. The available ports
would be all ports - those in use, where those in use would be
virBitmapCountBits of the hub bitmap.
> +size_t
> +virDomainUSBAddressCountAvailablePorts(virDomainDefPtr def)
> +{
> + size_t i, ret = 0;
> +
> + for (i = 0; i < def->ncontrollers; i++) {
> + virDomainControllerDefPtr cont = def->controllers[i];
> + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB)
> + ret += virDomainUSBAddressControllerModelToPorts(cont);
> + }
> +
> + for (i = 0; i < def->nhubs; i++) {
> + virDomainHubDefPtr hub = def->hubs[i];
> + if (hub->type == VIR_DOMAIN_HUB_TYPE_USB)
> + ret += VIR_DOMAIN_USB_HUB_PORTS;
> + }
> + return ret;
> +}
> +
> +
> static int
> virDomainUSBAddressAssignFromBus(virDomainUSBAddressSetPtr addrs,
> virDomainDeviceInfoPtr info,
> diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
> index 5a3f935..3f1d654 100644
> --- a/src/conf/domain_addr.h
> +++ b/src/conf/domain_addr.h
> @@ -277,6 +277,8 @@ int
> virDomainUSBAddressSetAddHub(virDomainUSBAddressSetPtr addrs,
> virDomainHubDefPtr hub)
> ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
> +size_t
> +virDomainUSBAddressCountAvailablePorts(virDomainDefPtr def);
> void virDomainUSBAddressSetFree(virDomainUSBAddressSetPtr addrs);
>
> int
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index e23bfe3..8358442 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -108,6 +108,7 @@ virDomainPCIAddressSlotInUse;
> virDomainPCIAddressValidate;
> virDomainPCIControllerModelToConnectType;
> virDomainUSBAddressAssign;
> +virDomainUSBAddressCountAvailablePorts;
> virDomainUSBAddressEnsure;
> virDomainUSBAddressPortFormat;
> virDomainUSBAddressPortFormatBuf;
> diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
> index c18182a..39c7d19 100644
> --- a/src/qemu/qemu_domain_address.c
> +++ b/src/qemu/qemu_domain_address.c
> @@ -1679,6 +1679,51 @@ qemuDomainAssignUSBPorts(virDomainUSBAddressSetPtr addrs,
>
>
> static int
> +qemuDomainAssignUSBPortsCounter(virDomainDeviceInfoPtr info ATTRIBUTE_UNUSED,
> + void *opaque)
> +{
> + struct qemuAssignUSBIteratorInfo *data = opaque;
> +
> + data->count++;
> + return 0;
> +}
> +
> +
> +static int
> +qemuDomainUSBAddressAddHubs(virDomainDefPtr def)
> +{
> + struct qemuAssignUSBIteratorInfo data = { .count = 0 };
> + virDomainHubDefPtr hub = NULL;
> + size_t available_ports;
> + int ret = -1;
> +
> + available_ports = virDomainUSBAddressCountAvailablePorts(def);
> + ignore_value(virDomainUSBDeviceDefForeach(def,
> + qemuDomainAssignUSBPortsCounter,
> + &data,
> + false));
> + VIR_DEBUG("Found %zu USB devices and %zu provided USB ports",
> + data.count, available_ports);
> +
> + /* Add one hub if there are more devices than ports
> + * otherwise it's up to the user to specify more hubs/controllers */
What if one hub isn't enough? Wouldn't we need to keep adding until we
have enough? I'm sure a test could be created for it...
> + if (data.count > available_ports) {
> + if (VIR_ALLOC(hub) < 0)
> + return -1;
> + hub->type = VIR_DOMAIN_HUB_TYPE_USB;
> +
> + if (VIR_APPEND_ELEMENT(def->hubs, def->nhubs, hub) < 0)
> + goto cleanup;
> + }
> +
> + ret = 0;
> + cleanup:
> + VIR_FREE(hub);
> + return ret;
> +}
> +
> +
> +static int
> qemuDomainAssignUSBAddresses(virDomainDefPtr def,
> virDomainObjPtr obj)
> {
> @@ -1689,6 +1734,9 @@ qemuDomainAssignUSBAddresses(virDomainDefPtr def,
> if (!(addrs = virDomainUSBAddressSetCreate()))
> goto cleanup;
>
> + if (qemuDomainUSBAddressAddHubs(def) < 0)
> + goto cleanup;
> +
> if (virDomainUSBAddressSetAddControllers(addrs, def) < 0)
> goto cleanup;
>
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-hub-autoadd.args b/tests/qemuxml2argvdata/qemuxml2argv-usb-hub-autoadd.args
> new file mode 100644
> index 0000000..12c9691
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-hub-autoadd.args
> @@ -0,0 +1,28 @@
> +LC_ALL=C \
> +PATH=/bin \
> +HOME=/home/test \
> +USER=test \
> +LOGNAME=test \
> +QEMU_AUDIO_DRV=none \
> +/usr/bin/qemu \
> +-name QEMUGuest1 \
> +-S \
> +-M pc \
> +-m 214 \
> +-smp 1 \
This will need ,sockets=1,cores=1,threads=1
> +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
> +-nographic \
> +-nodefconfig \
> +-nodefaults \
> +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
> +server,nowait \
> +-mon chardev=charmonitor,id=monitor,mode=readline \
> +-no-acpi \
> +-boot c \
> +-usb \
> +-device usb-hub,id=hub0,bus=usb.0,port=1 \
> +-device usb-mouse,id=input0,bus=usb.0,port=2 \
> +-device usb-mouse,id=input1,bus=usb.0,port=1.1 \
> +-device usb-mouse,id=input2,bus=usb.0,port=1.2 \
> +-device usb-tablet,id=input3,bus=usb.0,port=1.3 \
> +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-hub-autoadd.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb-hub-autoadd.xml
> new file mode 100644
> index 0000000..43e0f1f
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-hub-autoadd.xml
> @@ -0,0 +1,23 @@
> +<domain type='qemu'>
> + <name>QEMUGuest1</name>
> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> + <memory unit='KiB'>219136</memory>
> + <currentMemory unit='KiB'>219136</currentMemory>
> + <vcpu placement='static'>1</vcpu>
> + <os>
> + <type arch='i686' machine='pc'>hvm</type>
> + <boot dev='hd'/>
> + </os>
> + <devices>
> + <emulator>/usr/bin/qemu</emulator>
> + <controller type='usb' index='0'/>
So to try and answer something I was thinking about earlier... If I just
change index from 0 to 1, then I get an error:
libvirt: QEMU Driver error : unsupported configuration: Multiple legacy
USB controllers are not supported
If I then adjust the qemuxml2argvtest to add QEMU_CAPS_PIIX3_USB_UHCI,
QEMU_CAPS_PCI_MULTIFUNCTION, then I get the failure:
error : virDomainUSBAddressReserve:1724 : XML error: Duplicate USB
address bus 0 port 1
So is it a "valid" test to have one defined for index='1' and allow the
code to create index='0'?
John
> + <memballoon model='virtio'/>
> + <input type='mouse' bus='usb'>
> + </input>
> + <input type='mouse' bus='usb'>
> + </input>
> + <input type='mouse' bus='usb'>
> + </input>
> + <input type='tablet' bus='usb'/>
> + </devices>
> +</domain>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index a872ba8..5bbd6e3 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -1159,6 +1159,9 @@ mymain(void)
> DO_TEST("usb-hub",
> QEMU_CAPS_CHARDEV, QEMU_CAPS_USB_HUB,
> QEMU_CAPS_NODEFCONFIG);
> + DO_TEST("usb-hub-autoadd",
> + QEMU_CAPS_CHARDEV, QEMU_CAPS_USB_HUB,
> + QEMU_CAPS_NODEFCONFIG);
> DO_TEST_PARSE_ERROR("usb-hub-conflict",
> QEMU_CAPS_CHARDEV, QEMU_CAPS_USB_HUB,
> QEMU_CAPS_NODEFCONFIG);
>
More information about the libvir-list
mailing list