[libvirt] [PATCH v2 0/5] add support for qemu-xhci USB controller
Pavel Hrdina (5): qemu: change the logic of setting default USB controller qemu: use nec-usb-xhci as a default controller for aarch64 if available qemu: introduce QEMU_CAPS_DEVICE_QEMU_XHCI qemu: add support for qemu-xhci USB controller qemu: use qemu-xhci USB controller by default for ppc64 and aarch64 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_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 8 +++- src/qemu/qemu_domain.c | 47 ++++++++++++++-------- src/qemu/qemu_domain_address.c | 1 + tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + ...muxml2argv-aarch64-usb-controller-nec-xhci.args | 19 +++++++++ ...emuxml2argv-aarch64-usb-controller-nec-xhci.xml | 16 ++++++++ ...uxml2argv-aarch64-usb-controller-qemu-xhci.args | 19 +++++++++ ...muxml2argv-aarch64-usb-controller-qemu-xhci.xml | 16 ++++++++ ...emuxml2argv-ppc64-usb-controller-qemu-xhci.args | 19 +++++++++ ...qemuxml2argv-ppc64-usb-controller-qemu-xhci.xml | 1 + ...qemuxml2argv-usb-controller-qemu-xhci-limit.xml | 14 +++++++ ...l2argv-usb-controller-qemu-xhci-unavailable.xml | 1 + .../qemuxml2argv-usb-controller-qemu-xhci.args | 19 +++++++++ .../qemuxml2argv-usb-controller-qemu-xhci.xml | 14 +++++++ tests/qemuxml2argvtest.c | 15 +++++++ 22 files changed, 201 insertions(+), 20 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-aarch64-usb-controller-nec-xhci.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-aarch64-usb-controller-nec-xhci.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-aarch64-usb-controller-qemu-xhci.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-aarch64-usb-controller-qemu-xhci.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ppc64-usb-controller-qemu-xhci.args create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-ppc64-usb-controller-qemu-xhci.xml 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 -- 2.12.2
The new logic will set the piix3-uhci if available regardless of any architecture and it will be updated to better model based on architecture and device existence. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_domain.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6824af565b..5cdb99cb4b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3200,11 +3200,17 @@ qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont, * when the relevant device is not available. * * See qemuBuildControllerDevCommandLine() */ - if (ARCH_IS_S390(def->os.arch) && - cont->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { - /* set the default USB model to none for s390 unless an - * address is found */ - cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE; + + /* Default USB controller is piix3-uhci if available. */ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX3_USB_UHCI)) + cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI; + + if (ARCH_IS_S390(def->os.arch)) { + if (cont->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + /* set the default USB model to none for s390 unless an + * address is found */ + cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE; + } } else if (ARCH_IS_PPC64(def->os.arch)) { /* To not break migration we need to set default USB controller * for ppc64 to pci-ohci if we cannot change ABI of the VM. @@ -3215,11 +3221,10 @@ qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont, cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI; } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_OHCI)) { cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI; + } else { + /* Explicitly fallback to legacy USB controller for PPC64. */ + cont->model = -1; } - } else { - /* Default USB controller for anything else is piix3-uhci */ - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX3_USB_UHCI)) - cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI; } } /* forbid usb model 'qusb1' and 'qusb2' in this kind of hyperviosr */ -- 2.12.2
On Thu, 2017-04-27 at 18:09 +0200, Pavel Hrdina wrote:
@@ -3200,11 +3200,17 @@ qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont, * when the relevant device is not available. * * See qemuBuildControllerDevCommandLine() */ - if (ARCH_IS_S390(def->os.arch) && - cont->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { - /* set the default USB model to none for s390 unless an - * address is found */ - cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE; + + /* Default USB controller is piix3-uhci if available. */ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX3_USB_UHCI)) + cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI; + + if (ARCH_IS_S390(def->os.arch)) { + if (cont->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + /* set the default USB model to none for s390 unless an + * address is found */ + cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE; + }
You should add an else branch where you unset cont->model explicitly, like you do for ppc64 below, otherwise s390 guests will start being assigned piix3-uhci controllers in some situations. We don't have checks for that in our test suite, and it's probably not going to be much of an issue in the real world at least as far as downstream distros are concerned, but it should be avoided nonetheless. Overall I'm not convinced this is any more readable than what we had before or that it makes subsequent changes any easier, so I'd prefer if you'd just drop this patch. -- Andrea Bolognani / Red Hat / Virtualization
On Thu, Apr 27, 2017 at 08:14:17PM +0200, Andrea Bolognani wrote:
On Thu, 2017-04-27 at 18:09 +0200, Pavel Hrdina wrote:
@@ -3200,11 +3200,17 @@ qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont, * when the relevant device is not available. * * See qemuBuildControllerDevCommandLine() */ - if (ARCH_IS_S390(def->os.arch) && - cont->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { - /* set the default USB model to none for s390 unless an - * address is found */ - cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE; + + /* Default USB controller is piix3-uhci if available. */ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX3_USB_UHCI)) + cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI; + + if (ARCH_IS_S390(def->os.arch)) { + if (cont->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + /* set the default USB model to none for s390 unless an + * address is found */ + cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE; + }
You should add an else branch where you unset cont->model explicitly, like you do for ppc64 below, otherwise s390 guests will start being assigned piix3-uhci controllers in some situations.
That would change the output of this code, this patch preserver the ouput, just changes the logic to be easier to follow.
We don't have checks for that in our test suite, and it's probably not going to be much of an issue in the real world at least as far as downstream distros are concerned, but it should be avoided nonetheless.
Overall I'm not convinced this is any more readable than what we had before or that it makes subsequent changes any easier, so I'd prefer if you'd just drop this patch.
The issue with original code is that the if else-if else condition is not consistent. The first if checks S390 and address type together, however the second else-if checks only for PPC64, the capability checks are inside that block. So if arch is S390 but the address type is different from VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE it continues to the last else block and sets the model to piix3-uhci it it's available. The second else-if for PPC64 doesn't fallback to piix3-uhci it the architecture is PPC64 but none of the capabilities from that block are set. This patch changes the logic and also makes the if else-if more clearer so the first check is only for architecture and after that in each block we test for capabilities. Pavel
On Thu, 2017-04-27 at 20:58 +0200, Pavel Hrdina wrote:
The issue with original code is that the if else-if else condition is not consistent. The first if checks S390 and address type together, however the second else-if checks only for PPC64, the capability checks are inside that block. So if arch is S390 but the address type is different from VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE it continues to the last else block and sets the model to piix3-uhci it it's available. The second else-if for PPC64 doesn't fallback to piix3-uhci it the architecture is PPC64 but none of the capabilities from that block are set. This patch changes the logic and also makes the if else-if more clearer so the first check is only for architecture and after that in each block we test for capabilities.
I see now that I was mistaken and your change is correct. Sorry for the noise. -- Andrea Bolognani / Red Hat / Virtualization
This is a USB3 controller and it's a better choice than piix3-uhci. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_domain.c | 3 +++ .../qemuxml2argv-aarch64-usb-controller-nec-xhci.args | 19 +++++++++++++++++++ .../qemuxml2argv-aarch64-usb-controller-nec-xhci.xml | 16 ++++++++++++++++ tests/qemuxml2argvtest.c | 4 ++++ 4 files changed, 42 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-aarch64-usb-controller-nec-xhci.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-aarch64-usb-controller-nec-xhci.xml diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 5cdb99cb4b..6fc8c29f3e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3225,6 +3225,9 @@ qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont, /* Explicitly fallback to legacy USB controller for PPC64. */ cont->model = -1; } + } else if (def->os.arch == VIR_ARCH_AARCH64) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NEC_USB_XHCI)) + cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI; } } /* forbid usb model 'qusb1' and 'qusb2' in this kind of hyperviosr */ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-usb-controller-nec-xhci.args b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-usb-controller-nec-xhci.args new file mode 100644 index 0000000000..e97431f8be --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-usb-controller-nec-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-aarch64 \ +-name QEMUGuest1 \ +-S \ +-M virt \ +-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 nec-usb-xhci,id=usb,bus=pcie.0,addr=0x1 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-usb-controller-nec-xhci.xml b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-usb-controller-nec-xhci.xml new file mode 100644 index 0000000000..1b7320f433 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-usb-controller-nec-xhci.xml @@ -0,0 +1,16 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index c1b014b7df..ad05122436 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2469,6 +2469,10 @@ mymain(void) DO_TEST("ppc64-usb-controller-legacy", QEMU_CAPS_PIIX3_USB_UHCI); + DO_TEST("aarch64-usb-controller-nec-xhci", + QEMU_CAPS_OBJECT_GPEX, + QEMU_CAPS_NEC_USB_XHCI); + DO_TEST_PARSE_FLAGS_ERROR("missing-machine", VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS, NONE); -- 2.12.2
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + 3 files changed, 4 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index cc3e1f8084..9a97c18d47 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -364,6 +364,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "query-cpu-definitions", /* 250 */ "block-write-threshold", "query-named-block-nodes", + "qemu-xhci", ); @@ -1609,6 +1610,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "vhost-scsi", QEMU_CAPS_DEVICE_VHOST_SCSI }, { "nvdimm", QEMU_CAPS_DEVICE_NVDIMM }, { "pcie-root-port", QEMU_CAPS_DEVICE_PCIE_ROOT_PORT }, + { "qemu-xhci", QEMU_CAPS_DEVICE_QEMU_XHCI }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBalloon[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index a7eec52a94..9c96e22acf 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -401,6 +401,7 @@ typedef enum { QEMU_CAPS_QUERY_CPU_DEFINITIONS, /* qmp query-cpu-definitions */ QEMU_CAPS_BLOCK_WRITE_THRESHOLD, /* BLOCK_WRITE_THRESHOLD event */ QEMU_CAPS_QUERY_NAMED_BLOCK_NODES, /* qmp query-named-block-nodes */ + QEMU_CAPS_DEVICE_QEMU_XHCI, /* -device qemu-xhci */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml index d238078af9..4769e1b858 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml @@ -208,6 +208,7 @@ <flag name='query-cpu-definitions'/> <flag name='block-write-threshold'/> <flag name='query-named-block-nodes'/> + <flag name='qemu-xhci'/> <version>2009000</version> <kvmVersion>0</kvmVersion> <package> (v2.9.0)</package> -- 2.12.2
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
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
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
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
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
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1438682 Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- New in v2: - <currentMemory> removed from test XMLs - other elements removed from test XMLs - <memballoon> model set to 'none' - removed QEMU_CAPS_DEVICE_IOH3420 from test capabilities src/qemu/qemu_domain.c | 11 ++++++++--- ...qemuxml2argv-aarch64-usb-controller-qemu-xhci.args | 19 +++++++++++++++++++ .../qemuxml2argv-aarch64-usb-controller-qemu-xhci.xml | 16 ++++++++++++++++ .../qemuxml2argv-ppc64-usb-controller-qemu-xhci.args | 19 +++++++++++++++++++ .../qemuxml2argv-ppc64-usb-controller-qemu-xhci.xml | 1 + tests/qemuxml2argvtest.c | 7 +++++++ 6 files changed, 70 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-aarch64-usb-controller-qemu-xhci.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-aarch64-usb-controller-qemu-xhci.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ppc64-usb-controller-qemu-xhci.args create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-ppc64-usb-controller-qemu-xhci.xml diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2d14f47f28..e1839a739e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3214,9 +3214,12 @@ qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont, } else if (ARCH_IS_PPC64(def->os.arch)) { /* To not break migration we need to set default USB controller * for ppc64 to pci-ohci if we cannot change ABI of the VM. - * The nec-usb-xhci controller is used as default only for - * newly defined domains or devices. */ + * The nec-usb-xhci or qemu-xhci controller is used as default + * only for newly defined domains or devices. */ if ((parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QEMU_XHCI)) { + cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI; + } else if ((parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE) && virQEMUCapsGet(qemuCaps, QEMU_CAPS_NEC_USB_XHCI)) { cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI; } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_OHCI)) { @@ -3226,7 +3229,9 @@ qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont, cont->model = -1; } } else if (def->os.arch == VIR_ARCH_AARCH64) { - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NEC_USB_XHCI)) + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QEMU_XHCI)) + cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI; + else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NEC_USB_XHCI)) cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI; } } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-usb-controller-qemu-xhci.args b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-usb-controller-qemu-xhci.args new file mode 100644 index 0000000000..0aa27f7abb --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-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-aarch64 \ +-name QEMUGuest1 \ +-S \ +-M virt \ +-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,id=usb,bus=pcie.0,addr=0x1 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-usb-controller-qemu-xhci.xml b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-usb-controller-qemu-xhci.xml new file mode 100644 index 0000000000..1b7320f433 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-usb-controller-qemu-xhci.xml @@ -0,0 +1,16 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-ppc64-usb-controller-qemu-xhci.args b/tests/qemuxml2argvdata/qemuxml2argv-ppc64-usb-controller-qemu-xhci.args new file mode 100644 index 0000000000..0f19eb7e66 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-ppc64-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-ppc64 \ +-name QEMUGuest1 \ +-S \ +-M pseries \ +-m 256 \ +-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 \ +-boot c \ +-device qemu-xhci,id=usb,bus=pci.0,addr=0x1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-ppc64-usb-controller-qemu-xhci.xml b/tests/qemuxml2argvdata/qemuxml2argv-ppc64-usb-controller-qemu-xhci.xml new file mode 120000 index 0000000000..831d9d4f1c --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-ppc64-usb-controller-qemu-xhci.xml @@ -0,0 +1 @@ +qemuxml2argv-ppc64-usb-controller.xml \ No newline at end of file diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 8ad3838059..a4b8ecb6d9 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2472,6 +2472,13 @@ mymain(void) QEMU_CAPS_PCI_OHCI); DO_TEST("ppc64-usb-controller-legacy", QEMU_CAPS_PIIX3_USB_UHCI); + DO_TEST_FULL("ppc64-usb-controller-qemu-xhci", NULL, -1, 0, + VIR_DOMAIN_DEF_PARSE_ABI_UPDATE, GIC_NONE, + QEMU_CAPS_DEVICE_QEMU_XHCI); + + DO_TEST("aarch64-usb-controller-qemu-xhci", + QEMU_CAPS_OBJECT_GPEX, + QEMU_CAPS_DEVICE_QEMU_XHCI); DO_TEST("aarch64-usb-controller-nec-xhci", QEMU_CAPS_OBJECT_GPEX, -- 2.12.2
On Thu, 2017-04-27 at 18:09 +0200, Pavel Hrdina wrote:
@@ -2472,6 +2472,13 @@ mymain(void) QEMU_CAPS_PCI_OHCI); DO_TEST("ppc64-usb-controller-legacy", QEMU_CAPS_PIIX3_USB_UHCI); + DO_TEST_FULL("ppc64-usb-controller-qemu-xhci", NULL, -1, 0, + VIR_DOMAIN_DEF_PARSE_ABI_UPDATE, GIC_NONE, + QEMU_CAPS_DEVICE_QEMU_XHCI); + + DO_TEST("aarch64-usb-controller-qemu-xhci", + QEMU_CAPS_OBJECT_GPEX, + QEMU_CAPS_DEVICE_QEMU_XHCI);
Please add QEMU_CAPS_NEC_USB_XHCI to both tests in order to show that qemu-xhci is being preferred to nec-xhci even though both are available. -- Andrea Bolognani / Red Hat / Virtualization
On Thu, Apr 27, 2017 at 08:34:57PM +0200, Andrea Bolognani wrote:
On Thu, 2017-04-27 at 18:09 +0200, Pavel Hrdina wrote:
@@ -2472,6 +2472,13 @@ mymain(void) QEMU_CAPS_PCI_OHCI); DO_TEST("ppc64-usb-controller-legacy", QEMU_CAPS_PIIX3_USB_UHCI); + DO_TEST_FULL("ppc64-usb-controller-qemu-xhci", NULL, -1, 0, + VIR_DOMAIN_DEF_PARSE_ABI_UPDATE, GIC_NONE, + QEMU_CAPS_DEVICE_QEMU_XHCI); + + DO_TEST("aarch64-usb-controller-qemu-xhci", + QEMU_CAPS_OBJECT_GPEX, + QEMU_CAPS_DEVICE_QEMU_XHCI);
Please add QEMU_CAPS_NEC_USB_XHCI to both tests in order to show that qemu-xhci is being preferred to nec-xhci even though both are available.
That's a good idea, I'll do that. Pavel
On Thu, 2017-04-27 at 18:09 +0200, Pavel Hrdina wrote:
Pavel Hrdina (5): qemu: change the logic of setting default USB controller qemu: use nec-usb-xhci as a default controller for aarch64 if available qemu: introduce QEMU_CAPS_DEVICE_QEMU_XHCI qemu: add support for qemu-xhci USB controller qemu: use qemu-xhci USB controller by default for ppc64 and aarch64
There are a couple of minor nits in patches 4 and 5, the kind that would not generally require a respin. The issue with patch 1 is also simple enough to be fixed without going through another round of reviews, but I'd prefer if you dropped that patch altogether and if you decide to do so the changes would become significant enough that I feel a respin would be warranted. Having expressed my preference, I leave up to you how to go about dealing with patch 1: you can expect a swift response on my part regardless of whether you ask for a formal ACK or a review of v3 :) -- Andrea Bolognani / Red Hat / Virtualization
On Thu, 2017-04-27 at 20:47 +0200, Andrea Bolognani wrote:
Pavel Hrdina (5): qemu: change the logic of setting default USB controller qemu: use nec-usb-xhci as a default controller for aarch64 if available qemu: introduce QEMU_CAPS_DEVICE_QEMU_XHCI qemu: add support for qemu-xhci USB controller qemu: use qemu-xhci USB controller by default for ppc64 and aarch64 There are a couple of minor nits in patches 4 and 5, the kind that would not generally require a respin. The issue with patch 1 is also simple enough to be fixed without going through another round of reviews, but I'd prefer if you dropped that patch altogether and if you decide to do so the changes would become significant enough that I feel a respin would be warranted. Having expressed my preference, I leave up to you how to go about dealing with patch 1: you can expect a swift response on my part regardless of whether you ask for a formal ACK or a review of v3 :)
Given your follow-up comments: after you address the nit I pointed out in patch 5, you can consider the series Acked-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization
On Fri, Apr 28, 2017 at 10:36:06AM +0200, Andrea Bolognani wrote:
On Thu, 2017-04-27 at 20:47 +0200, Andrea Bolognani wrote:
Pavel Hrdina (5): qemu: change the logic of setting default USB controller qemu: use nec-usb-xhci as a default controller for aarch64 if available qemu: introduce QEMU_CAPS_DEVICE_QEMU_XHCI qemu: add support for qemu-xhci USB controller qemu: use qemu-xhci USB controller by default for ppc64 and aarch64 There are a couple of minor nits in patches 4 and 5, the kind that would not generally require a respin. The issue with patch 1 is also simple enough to be fixed without going through another round of reviews, but I'd prefer if you dropped that patch altogether and if you decide to do so the changes would become significant enough that I feel a respin would be warranted. Having expressed my preference, I leave up to you how to go about dealing with patch 1: you can expect a swift response on my part regardless of whether you ask for a formal ACK or a review of v3 :)
Given your follow-up comments: after you address the nit I pointed out in patch 5, you can consider the series
Acked-by: Andrea Bolognani <abologna@redhat.com>
Thanks for the review, I'll push it shortly. Pavel
participants (2)
-
Andrea Bolognani -
Pavel Hrdina