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