virDomainAssignControllerIndexes: Ensure controller ordering after assigning indexes

Similarly to auto-adding of controllers, the assignment of indexes can
cause them to be considered in different ordering according to the logic
in 'virDomainControllerInsert' than they currently are.

To prevent changes in commandline between first run after defining a VM
xml and any subsequent run or restart of the daemon, we need to reorder
them when assigning the index.

The simplest method is to assign indexes and then create a new list of
controllers and re-instert them.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

diff --git a/src/conf/domain_postparse.c b/src/conf/domain_postparse.c
index ee27023f3ed3bff7802a4ec92fcd9f5b18df60cf..cafa2d235d80fe06c47af198d6b25deeb1095f07 100644 (file)
--- a/src/conf/domain_postparse.c
+++ b/src/conf/domain_postparse.c
@@ -1335,6 +1335,7 @@ virDomainAssignControllerIndexes(virDomainDef *def)
      * unused index.
      */
     size_t outer;
+    bool reinsert = false;
 
     for (outer = 0; outer < def->ncontrollers; outer++) {
         virDomainControllerDef *cont = def->controllers[outer];
@@ -1363,6 +1364,13 @@ virDomainAssignControllerIndexes(virDomainDef *def)
              */
             int prevIdx;
 
+            /* virDomainControllerInsert enforces an ordering of USB2 controllers
+             * based on their master port, which doesn't happen on the initial
+             * parse if the index wasn't yet allocated. If we encounter a USB2
+             * controller where we populated the index, we need to re-shuffle
+             * the controllers after allocating the index */
+            reinsert = true;
+
             prevIdx = outer - 1;
             while (prevIdx >= 0 &&
                    def->controllers[prevIdx]->type != VIR_DOMAIN_CONTROLLER_TYPE_USB)
@@ -1399,6 +1407,19 @@ virDomainAssignControllerIndexes(virDomainDef *def)
         if (!prev)
             cont->idx = virDomainControllerFindUnusedIndex(def, cont->type);
     }
+
+    if (reinsert) {
+        g_autofree virDomainControllerDef **controllers = g_steal_pointer(&def->controllers);
+        size_t ncontrollers = def->ncontrollers;
+        size_t i;
+
+        def->controllers = g_new0(virDomainControllerDef *, ncontrollers);
+        def->ncontrollers = 0;
+
+        for (i = 0; i < ncontrollers; i++) {
+            virDomainControllerInsertPreAlloced(def, controllers[i]);
+        }
+    }
 }
 
 

diff --git a/tests/qemuxml2argvdata/autoindex.x86_64-latest.args b/tests/qemuxml2argvdata/autoindex.x86_64-latest.args
index e712a324d875cb9a28c56610f2b2f8b592aafd8d..099f3c90297df395a1f2f3b000918cc5288528d2 100644 (file)
--- a/tests/qemuxml2argvdata/autoindex.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/autoindex.x86_64-latest.args
@@ -46,15 +46,15 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-q35-test/.config \
 -device '{"driver":"ich9-usb-uhci3","masterbus":"usb.0","firstport":4,"bus":"pcie.0","addr":"0x1d.0x2"}' \
 -device '{"driver":"ich9-usb-ehci1","id":"usb1","bus":"pcie.0","addr":"0x1a.0x7"}' \
 -device '{"driver":"ich9-usb-uhci1","masterbus":"usb1.0","firstport":0,"bus":"pcie.0","multifunction":true,"addr":"0x1a"}' \
+-device '{"driver":"ich9-usb-ehci1","id":"usb2","bus":"pci.2","addr":"0x1.0x7"}' \
 -device '{"driver":"ich9-usb-uhci1","masterbus":"usb2.0","firstport":0,"bus":"pci.2","multifunction":true,"addr":"0x1"}' \
 -device '{"driver":"ich9-usb-uhci2","masterbus":"usb2.0","firstport":2,"bus":"pci.2","addr":"0x1.0x1"}' \
 -device '{"driver":"ich9-usb-uhci3","masterbus":"usb2.0","firstport":4,"bus":"pci.2","addr":"0x1.0x2"}' \
--device '{"driver":"ich9-usb-ehci1","id":"usb2","bus":"pci.2","addr":"0x1.0x7"}' \
 -device '{"driver":"nec-usb-xhci","id":"usb3","bus":"pci.5","addr":"0x0"}' \
+-device '{"driver":"ich9-usb-ehci1","id":"usb4","bus":"pci.2","addr":"0x2.0x7"}' \
 -device '{"driver":"ich9-usb-uhci1","masterbus":"usb4.0","firstport":0,"bus":"pci.2","multifunction":true,"addr":"0x2"}' \
 -device '{"driver":"ich9-usb-uhci2","masterbus":"usb4.0","firstport":2,"bus":"pci.2","addr":"0x2.0x1"}' \
 -device '{"driver":"ich9-usb-uhci3","masterbus":"usb4.0","firstport":4,"bus":"pci.2","addr":"0x2.0x2"}' \
--device '{"driver":"ich9-usb-ehci1","id":"usb4","bus":"pci.2","addr":"0x2.0x7"}' \
 -blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \
 -blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}' \
 -device '{"driver":"ide-hd","bus":"ide.0","drive":"libvirt-1-format","id":"sata0-0-0","bootindex":1}' \

diff --git a/tests/qemuxml2xmloutdata/autoindex.x86_64-latest.xml b/tests/qemuxml2xmloutdata/autoindex.x86_64-latest.xml
index c43be23768571da64014a27922e8af641b5bec81..357055fff3c8e2812c68b9ff8184c3d968081a81 100644 (file)
--- a/tests/qemuxml2xmloutdata/autoindex.x86_64-latest.xml
+++ b/tests/qemuxml2xmloutdata/autoindex.x86_64-latest.xml
@@ -114,6 +114,9 @@
       <master startport='0'/>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x1a' function='0x0' multifunction='on'/>
     </controller>
+    <controller type='usb' index='2' model='ich9-ehci1'>
+      <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x7'/>
+    </controller>
     <controller type='usb' index='2' model='ich9-uhci1'>
       <master startport='0'/>
       <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x0' multifunction='on'/>
@@ -126,12 +129,12 @@
       <master startport='4'/>
       <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x2'/>
     </controller>
-    <controller type='usb' index='2' model='ich9-ehci1'>
-      <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x7'/>
-    </controller>
     <controller type='usb' index='3' model='nec-xhci'>
       <address type='pci' domain='0x0000' bus='0x05' slot='0x00' function='0x0'/>
     </controller>
+    <controller type='usb' index='4' model='ich9-ehci1'>
+      <address type='pci' domain='0x0000' bus='0x02' slot='0x02' function='0x7'/>
+    </controller>
     <controller type='usb' index='4' model='ich9-uhci1'>
       <master startport='0'/>
       <address type='pci' domain='0x0000' bus='0x02' slot='0x02' function='0x0' multifunction='on'/>
@@ -144,9 +147,6 @@
       <master startport='4'/>
       <address type='pci' domain='0x0000' bus='0x02' slot='0x02' function='0x2'/>
     </controller>
-    <controller type='usb' index='4' model='ich9-ehci1'>
-      <address type='pci' domain='0x0000' bus='0x02' slot='0x02' function='0x7'/>
-    </controller>
     <controller type='sata' index='0'>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/>
     </controller>