[libvirt] [PATCH v2 0/2] reorder usb controllers correctly
new in v2: - used git format-patch -b - added a test case Pavel Hrdina (2): domain-conf: reorder usb controllers so the master is first test: add test case for usb controller order src/conf/domain_conf.c | 21 +++++++++---- .../qemuxml2argv-controller-usb-order.xml | 32 ++++++++++++++++++++ .../qemuxml2xmlout-controller-usb-order.xml | 34 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 4 files changed, 83 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-controller-usb-order.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-controller-usb-order.xml -- 2.6.2
USB controllers can share the same 'index' which indicates, that there is some sort of master-companion relationship. Reorder the controllers in XML in to place the master controller before its companions. This is required by QEMU to not fail with error message: error: internal error: process exited while connecting to monitor: 2015-10-26T16:25:17.630265Z qemu-system-x86_64: -device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x6: USB bus 'usb.0' not found Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1166452 Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0c559d2..3f22de2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13397,6 +13397,7 @@ void virDomainControllerInsertPreAlloced(virDomainDefPtr def, int idx; /* Tenatively plan to insert controller at the end. */ int insertAt = -1; + virDomainControllerDefPtr current = NULL; /* Then work backwards looking for controllers of * the same type. If we find a controller with a @@ -13404,19 +13405,29 @@ void virDomainControllerInsertPreAlloced(virDomainDefPtr def, * that position */ for (idx = (def->ncontrollers - 1); idx >= 0; idx--) { + current = def->controllers[idx]; + if (current->type == controller->type) { + if (current->idx > controller->idx) { /* If bus matches and current controller is after - * new controller, then new controller should go here */ - if (def->controllers[idx]->type == controller->type && - def->controllers[idx]->idx > controller->idx) { + * new controller, then new controller should go here + * */ insertAt = idx; - } else if (def->controllers[idx]->type == controller->type && - insertAt == -1) { + } else if (controller->info.mastertype == VIR_DOMAIN_CONTROLLER_MASTER_NONE && + current->info.mastertype != VIR_DOMAIN_CONTROLLER_MASTER_NONE && + current->idx == controller->idx) { + /* If bus matches and index matches and new controller is + * master and current isn't a master, then new controller + * should go here to be placed before its companion + */ + insertAt = idx; + } else if (insertAt == -1) { /* Last controller with match bus is before the * new controller, then put new controller just after */ insertAt = idx + 1; } } + } /* VIR_INSERT_ELEMENT_INPLACE will never return an error here. */ ignore_value(VIR_INSERT_ELEMENT_INPLACE(def->controllers, insertAt, -- 2.6.2
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- .../qemuxml2argv-controller-usb-order.xml | 32 ++++++++++++++++++++ .../qemuxml2xmlout-controller-usb-order.xml | 34 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 3 files changed, 67 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-controller-usb-order.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-controller-usb-order.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-controller-usb-order.xml b/tests/qemuxml2argvdata/qemuxml2argv-controller-usb-order.xml new file mode 100644 index 0000000..68ebbf2 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-controller-usb-order.xml @@ -0,0 +1,32 @@ +<domain type='kvm'> + <name>rhel7</name> + <uuid>c9b867fb-7274-4a22-8884-0867d05b38cf</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static'>2</vcpu> + <os> + <type arch='x86_64' machine='pc-i440fx-2.3'>hvm</type> + <boot dev='hd'/> + </os> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <pm> + <suspend-to-mem enabled='no'/> + <suspend-to-disk enabled='no'/> + </pm> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' index='0' model='ich9-uhci1'> + <master startport='0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0' multifunction='on'/> + </controller> + <controller type='usb' index='0' model='ich9-ehci1'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x7' multifunction='on'/> + </controller> + <controller type='usb' index='1' model='nec-xhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/> + </controller> + </devices> +</domain> + diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-controller-usb-order.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-controller-usb-order.xml new file mode 100644 index 0000000..a3c22e8 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-controller-usb-order.xml @@ -0,0 +1,34 @@ +<domain type='kvm'> + <name>rhel7</name> + <uuid>c9b867fb-7274-4a22-8884-0867d05b38cf</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static'>2</vcpu> + <os> + <type arch='x86_64' machine='pc-i440fx-2.3'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <pm> + <suspend-to-mem enabled='no'/> + <suspend-to-disk enabled='no'/> + </pm> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' index='0' model='ich9-ehci1'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x7' multifunction='on'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci1'> + <master startport='0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0' multifunction='on'/> + </controller> + <controller type='usb' index='1' model='nec-xhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 5a9c67d..5e6fc9a 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -515,6 +515,7 @@ mymain(void) DO_TEST_DIFFERENT("usb-redir-filter"); DO_TEST_DIFFERENT("usb-redir-filter-version"); DO_TEST("blkdeviotune"); + DO_TEST_DIFFERENT("controller-usb-order"); DO_TEST_FULL("seclabel-dynamic-baselabel", false, WHEN_INACTIVE); DO_TEST_FULL("seclabel-dynamic-override", false, WHEN_INACTIVE); -- 2.6.2
On Tue, Oct 27, 2015 at 10:37:27 +0100, Pavel Hrdina wrote:
new in v2: - used git format-patch -b
I think Jan meant that it was hard to review with that option due to the fact that it contains both whitespace and functional changes, not that you should actually use it to send patches. They can't really be applied that way. Peter
participants (2)
-
Pavel Hrdina -
Peter Krempa