On 07/01/2016 11:38 AM, Ján Tomko wrote:
> We were requiring a USB port path in the schema, but not enforcing it.
> Omitting the USB port would lead to libvirt formatting it as (null).
> Such domain cannot be started and will disappear after libvirtd restart
> (since it cannot parse back the XML).
>
> Only format the port if it has been specified and mark it as optional
> in the XML schema.
> ---
> docs/schemas/domaincommon.rng | 8 +++--
> src/conf/domain_conf.c | 5 ++-
> src/qemu/qemu_command.c | 3 +-
> .../qemuxml2argv-usb-port-missing.args | 26 ++++++++++++++++
> .../qemuxml2argv-usb-port-missing.xml | 25 +++++++++++++++
> tests/qemuxml2argvtest.c | 3 ++
> .../qemuxml2xmlout-usb-port-missing.xml | 36 ++++++++++++++++++++++
> tests/qemuxml2xmltest.c | 1 +
> 8 files changed, 100 insertions(+), 7 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-port-missing.args
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-port-missing.xml
> create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-usb-port-missing.xml
>
As pointed out to me during the LUKS changes, if your "data" XML is
essentially the same as your output XML, you could make a file link from
output to data. See the *gic* files for an example. All I did was copy
the output to data, removed output, and then recreated output as file
link to data. It's not a "requirement" and it doesn't really matter to
me, but I think in the effort to reduce the size of things is what
started that. Of course, it's hard to know without tripping across it a
few times...
As long as there's no ABI issues that you can come up with and you fix
the .args file, then ACK
John
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 563cb3c..0876daa 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -4055,9 +4055,11 @@
> <attribute name="bus">
> <ref name="usbAddr"/>
> </attribute>
> - <attribute name="port">
> - <ref name="usbPort"/>
> - </attribute>
> + <optional>
> + <attribute name="port">
> + <ref name="usbPort"/>
> + </attribute>
> + </optional>
> </define>
> <define name="spaprvioaddress">
> <optional>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index b88afbc..61ad136 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -4859,9 +4859,8 @@ 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'", info->addr.usb.bus);
> + virBufferEscapeString(buf, " port='%s'", info->addr.usb.port);
> break;
>
> case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO:
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 3898ed7..90b7684 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -375,7 +375,8 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
> VIR_DOMAIN_CONTROLLER_TYPE_USB,
> info->addr.usb.bus)))
> goto cleanup;
> - virBufferAsprintf(buf, ",bus=%s.0,port=%s", contAlias, info->addr.usb.port);
> + virBufferAsprintf(buf, ",bus=%s.0", contAlias);
> + virBufferEscapeString(buf, ",port=%s", 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);
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-port-missing.args b/tests/qemuxml2argvdata/qemuxml2argv-usb-port-missing.args
> new file mode 100644
> index 0000000..d43c58d
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-port-missing.args
> @@ -0,0 +1,26 @@
> +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 "fail" now without the ",sockets=1,cores=1,threads=1" due to
commit id 'e114b09157'
> +-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 \
> +-device usb-hub,id=hub1,bus=usb.0 \
> +-device usb-mouse,id=input0,bus=usb.0 \
> +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-port-missing.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb-port-missing.xml
> new file mode 100644
> index 0000000..593fcd1
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-port-missing.xml
> @@ -0,0 +1,25 @@
> +<domain type='qemu'>
> + <name>QEMUGuest1</name>
> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> + <memory unit='KiB'>219136</memory>
> + <currentMemory unit='KiB'>219136</currentMemory>
> + <vcpu placement='static'>1</vcpu>
> + <os>
> + <type arch='i686' machine='pc'>hvm</type>
> + <boot dev='hd'/>
> + </os>
> + <devices>
> + <emulator>/usr/bin/qemu</emulator>
> + <controller type='usb' index='0'/>
> + <memballoon model='virtio'/>
> + <input type='mouse' bus='usb'>
> + <address type='usb' bus='0'/>
> + </input>
> + <hub type='usb'>
> + <address type='usb' bus='0'/>
> + </hub>
> + <hub type='usb'>
> + <address type='usb' bus='0'/>
> + </hub>
> + </devices>
> +</domain>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index a73db5e..4389e24 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-port-missing",
> + QEMU_CAPS_CHARDEV, QEMU_CAPS_USB_HUB,
> + QEMU_CAPS_NODEFCONFIG);
> DO_TEST("usb-ports",
> QEMU_CAPS_CHARDEV, QEMU_CAPS_USB_HUB,
> QEMU_CAPS_NODEFCONFIG);
> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-usb-port-missing.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-usb-port-missing.xml
> new file mode 100644
> index 0000000..2e29cbd
> --- /dev/null
> +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-usb-port-missing.xml
> @@ -0,0 +1,36 @@
> +<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>
> + <clock offset='utc'/>
> + <on_poweroff>destroy</on_poweroff>
> + <on_reboot>restart</on_reboot>
> + <on_crash>destroy</on_crash>
> + <devices>
> + <emulator>/usr/bin/qemu</emulator>
> + <controller type='usb' index='0'>
> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
> + </controller>
> + <controller type='pci' index='0' model='pci-root'/>
> + <input type='mouse' bus='usb'>
> + <address type='usb' bus='0'/>
> + </input>
> + <input type='mouse' bus='ps2'/>
> + <input type='keyboard' bus='ps2'/>
> + <hub type='usb'>
> + <address type='usb' bus='0'/>
> + </hub>
> + <hub type='usb'>
> + <address type='usb' bus='0'/>
> + </hub>
> + <memballoon model='virtio'>
> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
> + </memballoon>
> + </devices>
> +</domain>
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index 7db9cb7..c6ef28c 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -535,6 +535,7 @@ mymain(void)
> DO_TEST("interface-server");
> DO_TEST("virtio-lun");
>
> + DO_TEST("usb-port-missing");
> DO_TEST("usb-redir");
> DO_TEST("usb-redir-filter");
> DO_TEST("usb-redir-filter-version");
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list