Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1438682
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
New in v2:
- documentation fixed
- <currentMemory> removed from text XMLs
- reduced one file by using link
docs/formatdomain.html.in | 4 ++--
docs/schemas/domaincommon.rng | 1 +
src/conf/domain_addr.c | 1 +
src/conf/domain_conf.c | 1 +
src/conf/domain_conf.h | 1 +
src/qemu/qemu_command.c | 8 ++++++--
src/qemu/qemu_domain.c | 12 +++++++-----
src/qemu/qemu_domain_address.c | 1 +
.../qemuxml2argv-usb-controller-qemu-xhci-limit.xml | 14 ++++++++++++++
...uxml2argv-usb-controller-qemu-xhci-unavailable.xml | 1 +
.../qemuxml2argv-usb-controller-qemu-xhci.args | 19 +++++++++++++++++++
.../qemuxml2argv-usb-controller-qemu-xhci.xml | 14 ++++++++++++++
tests/qemuxml2argvtest.c | 4 ++++
13 files changed, 72 insertions(+), 9 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-controller-qemu-xhci-limit.xml
create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-usb-controller-qemu-xhci-unavailable.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-controller-qemu-xhci.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-controller-qemu-xhci.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index e31a271a58..e458d2f447 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3463,8 +3463,8 @@
<code>model</code>, which is one of "piix3-uhci", "piix4-uhci",
"ehci", "ich9-ehci1", "ich9-uhci1", "ich9-uhci2", "ich9-uhci3",
"vt82c686b-uhci", "pci-ohci", "nec-xhci", "qusb1" (xen pvusb
- with qemu backend, version 1.1) or "qusb2" (xen pvusb with qemu
- backend, version 2.0). Additionally,
+ with qemu backend, version 1.1), "qusb2" (xen pvusb with qemu
+ backend, version 2.0) or "qemu-xhci". Additionally,
<span class="since">since 0.10.0</span>, if the USB bus needs to
be explicitly disabled for the guest, <code>model='none'</code>
may be used. <span class="since">Since 1.0.5</span>, no default
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index eb4b0f7437..86309b2c3a 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1877,6 +1877,7 @@
<value>none</value>
<value>qusb1</value>
<value>qusb2</value>
+ <value>qemu-xhci</value>
</choice>
</attribute>
</optional>
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 8b61091996..639168effa 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1673,6 +1673,7 @@ virDomainUSBAddressControllerModelToPorts(virDomainControllerDefPtr cont)
return 3;
case VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI:
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI:
if (cont->opts.usbopts.ports != -1)
return cont->opts.usbopts.ports;
return 4;
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 61006dea7d..3ba570e0a2 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -366,6 +366,7 @@ VIR_ENUM_IMPL(virDomainControllerModelUSB, VIR_DOMAIN_CONTROLLER_MODEL_USB_LAST,
"nec-xhci",
"qusb1",
"qusb2",
+ "qemu-xhci",
"none")
VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 3b6b174516..31c7a924d7 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -735,6 +735,7 @@ typedef enum {
VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI,
VIR_DOMAIN_CONTROLLER_MODEL_USB_QUSB1,
VIR_DOMAIN_CONTROLLER_MODEL_USB_QUSB2,
+ VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI,
VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE,
VIR_DOMAIN_CONTROLLER_MODEL_USB_LAST
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1116d2cd5b..df03fa275d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -152,6 +152,7 @@ VIR_ENUM_IMPL(qemuControllerModelUSB, VIR_DOMAIN_CONTROLLER_MODEL_USB_LAST,
"nec-usb-xhci",
"qusb1",
"qusb2",
+ "qemu-xhci",
"none");
VIR_ENUM_DECL(qemuDomainFSDriver)
@@ -2558,6 +2559,8 @@ qemuControllerModelUSBToCaps(int model)
return QEMU_CAPS_PCI_OHCI;
case VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI:
return QEMU_CAPS_NEC_USB_XHCI;
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI:
+ return QEMU_CAPS_DEVICE_QEMU_XHCI;
default:
return -1;
}
@@ -2592,8 +2595,9 @@ qemuBuildUSBControllerDevStr(virDomainControllerDefPtr def,
virBufferAsprintf(buf, "%s", smodel);
if (def->opts.usbopts.ports != -1) {
- if (model != VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI ||
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_NEC_USB_XHCI_PORTS)) {
+ if ((model != VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI ||
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_NEC_USB_XHCI_PORTS)) &&
+ model != VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("usb controller type %s doesn't support 'ports' "
"with this QEMU binary"), smodel);
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 6fc8c29f3e..2d14f47f28 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3171,7 +3171,7 @@ qemuDomainShmemDefPostParse(virDomainShmemDefPtr shm)
}
-#define QEMU_USB_NEC_XHCI_MAXPORTS 15
+#define QEMU_USB_XHCI_MAXPORTS 15
static int
@@ -3239,11 +3239,13 @@ qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont,
virDomainVirtTypeToString(def->virtType));
return -1;
}
- if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI &&
- cont->opts.usbopts.ports > QEMU_USB_NEC_XHCI_MAXPORTS) {
+ if ((cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI ||
+ cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI) &&
+ cont->opts.usbopts.ports > QEMU_USB_XHCI_MAXPORTS) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("nec-xhci controller only supports up to %u ports"),
- QEMU_USB_NEC_XHCI_MAXPORTS);
+ _("'%s' controller only supports up to '%u' ports"),
+ virDomainControllerModelUSBTypeToString(cont->model),
+ QEMU_USB_XHCI_MAXPORTS);
return -1;
}
break;
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 064d05079c..3da6b7369d 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -502,6 +502,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
case VIR_DOMAIN_CONTROLLER_TYPE_USB:
switch ((virDomainControllerModelUSB) cont->model) {
case VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI:
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI:
return pcieFlags;
case VIR_DOMAIN_CONTROLLER_MODEL_USB_EHCI:
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-controller-qemu-xhci-limit.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb-controller-qemu-xhci-limit.xml
new file mode 100644
index 0000000000..27cc99127f
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-controller-qemu-xhci-limit.xml
@@ -0,0 +1,14 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc'>hvm</type>
+ </os>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <controller type='usb' index='0' model='qemu-xhci' ports='16'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-controller-qemu-xhci-unavailable.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb-controller-qemu-xhci-unavailable.xml
new file mode 120000
index 0000000000..989306fa7f
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-controller-qemu-xhci-unavailable.xml
@@ -0,0 +1 @@
+qemuxml2argv-usb-controller-qemu-xhci.xml
\ No newline at end of file
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-controller-qemu-xhci.args b/tests/qemuxml2argvdata/qemuxml2argv-usb-controller-qemu-xhci.args
new file mode 100644
index 0000000000..4f967b7b22
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-controller-qemu-xhci.args
@@ -0,0 +1,19 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name QEMUGuest1 \
+-S \
+-M pc \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
+-no-acpi \
+-boot c \
+-device qemu-xhci,p2=8,p3=8,id=usb,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-controller-qemu-xhci.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb-controller-qemu-xhci.xml
new file mode 100644
index 0000000000..b63f9e1c40
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-controller-qemu-xhci.xml
@@ -0,0 +1,14 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc'>hvm</type>
+ </os>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <controller type='usb' index='0' model='qemu-xhci' ports='8'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index ad05122436..8ad3838059 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1416,6 +1416,10 @@ mymain(void)
DO_TEST_PARSE_ERROR("usb-controller-xhci-limit",
QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_PIIX3_USB_UHCI,
QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_NEC_USB_XHCI_PORTS);
+ DO_TEST("usb-controller-qemu-xhci", QEMU_CAPS_DEVICE_QEMU_XHCI);
+ DO_TEST_FAILURE("usb-controller-qemu-xhci-unavailable", NONE);
+ DO_TEST_PARSE_ERROR("usb-controller-qemu-xhci-limit",
+ QEMU_CAPS_DEVICE_QEMU_XHCI);
DO_TEST("smbios", QEMU_CAPS_SMBIOS_TYPE);
DO_TEST_PARSE_ERROR("smbios-date", QEMU_CAPS_SMBIOS_TYPE);
--
2.12.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, 2017-04-27 at 18:09 +0200, Pavel Hrdina wrote:
> - reduced one file by using link
Sorry I was not clear enough the first time around, but what
I meant was that you can literally...
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-controller-qemu-xhci-unavailable.xml
> @@ -0,0 +1 @@
> +qemuxml2argv-usb-controller-qemu-xhci.xml
> \ No newline at end of file
... not add this file at all...
> @@ -1416,6 +1416,10 @@ mymain(void)
> DO_TEST_PARSE_ERROR("usb-controller-xhci-limit",
> QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_PIIX3_USB_UHCI,
> QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_NEC_USB_XHCI_PORTS);
> + DO_TEST("usb-controller-qemu-xhci", QEMU_CAPS_DEVICE_QEMU_XHCI);
> + DO_TEST_FAILURE("usb-controller-qemu-xhci-unavailable", NONE);
... and just use the usb-controller-qemu-xhci input file
again for the failing test case.
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Apr 27, 2017 at 08:40:22PM +0200, Andrea Bolognani wrote:
> On Thu, 2017-04-27 at 18:09 +0200, Pavel Hrdina wrote:
> > - reduced one file by using link
>
> Sorry I was not clear enough the first time around, but what
> I meant was that you can literally...
>
> > --- /dev/null
> > +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-controller-qemu-xhci-unavailable.xml
> > @@ -0,0 +1 @@
> > +qemuxml2argv-usb-controller-qemu-xhci.xml
> > \ No newline at end of file
>
> ... not add this file at all...
>
> > @@ -1416,6 +1416,10 @@ mymain(void)
> > DO_TEST_PARSE_ERROR("usb-controller-xhci-limit",
> > QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_PIIX3_USB_UHCI,
> > QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_NEC_USB_XHCI_PORTS);
> > + DO_TEST("usb-controller-qemu-xhci", QEMU_CAPS_DEVICE_QEMU_XHCI);
> > + DO_TEST_FAILURE("usb-controller-qemu-xhci-unavailable", NONE);
>
> ... and just use the usb-controller-qemu-xhci input file
> again for the failing test case.
No, this is not just what file should be used, it's also a name of the
test case and it should be unique. It was clear the first time, but it
was not a good suggestion so I've used link instead.
Pavel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, 2017-04-27 at 20:45 +0200, Pavel Hrdina wrote:
> > > @@ -1416,6 +1416,10 @@ mymain(void)
> > > DO_TEST_PARSE_ERROR("usb-controller-xhci-limit",
> > > QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_PIIX3_USB_UHCI,
> > > QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_NEC_USB_XHCI_PORTS);
> > > + DO_TEST("usb-controller-qemu-xhci", QEMU_CAPS_DEVICE_QEMU_XHCI);
> > > + DO_TEST_FAILURE("usb-controller-qemu-xhci-unavailable", NONE);
> >
> > ... and just use the usb-controller-qemu-xhci input file
> > again for the failing test case.
>
> No, this is not just what file should be used, it's also a name of the
> test case and it should be unique.
There are already several test cases where we use a single
input file with different capabilities, eg. all the
aarch64-gic-* (those were introduced by me, so they don't
count for the sake of argument), machine-aeskeywrap-on-caps,
shmem...
I could point out more if I bothered looking further:
$ VIR_TEST_DEBUG=1 ./tests/qemuxml2argvtest 2>&1 | \
grep -E '^[0-9]+)' | awk '{print $4}' | wc -l
698
$ VIR_TEST_DEBUG=1 ./tests/qemuxml2argvtest 2>&1 | \
grep -E '^[0-9]+)' | awk '{print $4}' | sort -u | wc -l
644
Since you clearly have a strong preference for one approach,
arguably the most correct one, let's go with that.
> It was clear the first time, but it
> was not a good suggestion so I've used link instead.
It would have been easier if you'd let me know you disagreed
with my comments by replying to them instead of me having to
find out and ask, wouldn't it? :)
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Apr 28, 2017 at 10:07:43AM +0200, Andrea Bolognani wrote:
> On Thu, 2017-04-27 at 20:45 +0200, Pavel Hrdina wrote:
> > > > @@ -1416,6 +1416,10 @@ mymain(void)
> > > > DO_TEST_PARSE_ERROR("usb-controller-xhci-limit",
> > > > QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_PIIX3_USB_UHCI,
> > > > QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_NEC_USB_XHCI_PORTS);
> > > > + DO_TEST("usb-controller-qemu-xhci", QEMU_CAPS_DEVICE_QEMU_XHCI);
> > > > + DO_TEST_FAILURE("usb-controller-qemu-xhci-unavailable", NONE);
> > >
> > > ... and just use the usb-controller-qemu-xhci input file
> > > again for the failing test case.
> >
> > No, this is not just what file should be used, it's also a name of the
> > test case and it should be unique.
>
> There are already several test cases where we use a single
> input file with different capabilities, eg. all the
> aarch64-gic-* (those were introduced by me, so they don't
> count for the sake of argument), machine-aeskeywrap-on-caps,
> shmem...
>
> I could point out more if I bothered looking further:
>
> $ VIR_TEST_DEBUG=1 ./tests/qemuxml2argvtest 2>&1 | \
> grep -E '^[0-9]+)' | awk '{print $4}' | wc -l
> 698
> $ VIR_TEST_DEBUG=1 ./tests/qemuxml2argvtest 2>&1 | \
> grep -E '^[0-9]+)' | awk '{print $4}' | sort -u | wc -l
> 644
>
> Since you clearly have a strong preference for one approach,
> arguably the most correct one, let's go with that.
>
> > It was clear the first time, but it
> > was not a good suggestion so I've used link instead.
>
> It would have been easier if you'd let me know you disagreed
> with my comments by replying to them instead of me having to
> find out and ask, wouldn't it? :)
I didn't know that there is already a case where we use the name
twice so I didn't thought about that at all, otherwise I would
let you know, I just thought that it was a honest mistake, not
an intention :).
Thanks
Pavel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.