[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH 3/8] qemu: Assign device addresses in PostParse



In order to make this work, we need to short circuit the normal
virDomainDefPostParse ordering, and manually add stock devices
ourselves, since we need them in the XML before assigning addresses.

The motivation for this is that upcoming patches will want to perform
generic PostParse checks that need to run _after_ the driver has
assigned all its addresses.

Peter objected to this here:
https://www.redhat.com/archives/libvir-list/2016-January/msg00200.html

Suggesting adding an extra PostParse callback instead. I argued
that change isn't necessarily sufficient either, and that this
method should be safe since all these functions already need to be
idemptotent.

The lone test suite change is due to DomainNativeToXML now calling
qemuDomainAssignAddresses... apparently that's the only test which
hits qemu specific address logic.

There's still quite a few manual callers of qemuDomainAssignAddresses
that could be dropped too but it would need additional testing, and
they shouldn't disrupt anything in the interim since extra calls
will be no-ops.
---
 src/qemu/qemu_domain.c                               | 13 ++++++++++++-
 tests/qemuargv2xmldata/qemuargv2xml-pseries-disk.xml |  4 +++-
 tests/qemuxml2argvtest.c                             | 20 +++++++-------------
 tests/qemuxml2xmltest.c                              | 12 ++++--------
 4 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 9044792..d697e99 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1369,7 +1369,7 @@ qemuCanonicalizeMachine(virDomainDefPtr def, virQEMUCapsPtr qemuCaps)
 static int
 qemuDomainDefPostParse(virDomainDefPtr def,
                        virCapsPtr caps,
-                       unsigned int parseFlags ATTRIBUTE_UNUSED,
+                       unsigned int parseFlags,
                        void *opaque)
 {
     virQEMUDriverPtr driver = opaque;
@@ -1408,6 +1408,17 @@ qemuDomainDefPostParse(virDomainDefPtr def,
     if (virSecurityManagerVerify(driver->securityManager, def) < 0)
         goto cleanup;
 
+    /* Device defaults are normally set after calling the driver specific
+       PostParse routine (this function), but we need them earlier. */
+    if (virDomainDefPostParseDevices(def, caps,
+                                     parseFlags, driver->xmlopt) < 0)
+        goto cleanup;
+    if (virDomainDefAddImplicitDevices(def) < 0)
+        goto cleanup;
+
+    if (qemuDomainAssignAddresses(def, qemuCaps, NULL) < 0)
+        goto cleanup;
+
     ret = 0;
  cleanup:
     virObjectUnref(qemuCaps);
diff --git a/tests/qemuargv2xmldata/qemuargv2xml-pseries-disk.xml b/tests/qemuargv2xmldata/qemuargv2xml-pseries-disk.xml
index 97225f4..c0d7e94 100644
--- a/tests/qemuargv2xmldata/qemuargv2xml-pseries-disk.xml
+++ b/tests/qemuargv2xmldata/qemuargv2xml-pseries-disk.xml
@@ -29,7 +29,9 @@
     </disk>
     <controller type='usb' index='0'/>
     <controller type='pci' index='0' model='pci-root'/>
-    <controller type='scsi' index='0'/>
+    <controller type='scsi' index='0'>
+      <address type='spapr-vio' reg='0x2000'/>
+    </controller>
     <input type='keyboard' bus='usb'/>
     <input type='mouse' bus='usb'/>
     <graphics type='sdl'/>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index d29073d..aaec9de 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -277,6 +277,11 @@ static int testCompareXMLToArgvFiles(const char *xml,
     if (virBitmapParse("0-3", '\0', &nodeset, 4) < 0)
         goto out;
 
+    virQEMUCapsSetList(extraFlags,
+                       QEMU_CAPS_NO_ACPI,
+                       QEMU_CAPS_DEVICE,
+                       QEMU_CAPS_LAST);
+
     if (!(vmdef = virDomainDefParseFile(xml, driver.caps, driver.xmlopt,
                                         (VIR_DOMAIN_DEF_PARSE_INACTIVE |
                                          parseFlags)))) {
@@ -302,11 +307,6 @@ static int testCompareXMLToArgvFiles(const char *xml,
     if (qemuProcessPrepareMonitorChr(&monitor_chr, domainLibDir) < 0)
         goto out;
 
-    virQEMUCapsSetList(extraFlags,
-                       QEMU_CAPS_NO_ACPI,
-                       QEMU_CAPS_DEVICE,
-                       QEMU_CAPS_LAST);
-
     if (STREQ(vmdef->os.machine, "pc") &&
         STREQ(vmdef->emulator, "/usr/bin/qemu-system-x86_64")) {
         VIR_FREE(vmdef->os.machine);
@@ -316,12 +316,6 @@ static int testCompareXMLToArgvFiles(const char *xml,
 
     virQEMUCapsFilterByMachineType(extraFlags, vmdef->os.machine);
 
-    if (qemuDomainAssignAddresses(vmdef, extraFlags, NULL)) {
-        if (flags & FLAG_EXPECT_ERROR)
-            goto ok;
-        goto out;
-    }
-
     log = virtTestLogContentAndReset();
     VIR_FREE(log);
     virResetLastError();
@@ -1420,7 +1414,7 @@ mymain(void)
             QEMU_CAPS_PCI_OHCI, QEMU_CAPS_PCI_MULTIFUNCTION);
     DO_TEST("pseries-vio-user-assigned",
             QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG);
-    DO_TEST_ERROR("pseries-vio-address-clash",
+    DO_TEST_PARSE_ERROR("pseries-vio-address-clash",
             QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG);
     DO_TEST("pseries-nvram", QEMU_CAPS_DEVICE_NVRAM);
     DO_TEST("pseries-usb-kbd", QEMU_CAPS_PCI_OHCI,
@@ -1583,7 +1577,7 @@ mymain(void)
             QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
             QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL);
 
-    DO_TEST_ERROR("pcie-root-port-too-many",
+    DO_TEST_PARSE_ERROR("pcie-root-port-too-many",
             QEMU_CAPS_DEVICE_PCI_BRIDGE,
             QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
             QEMU_CAPS_DEVICE_IOH3420,
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 0735677..251effd 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -37,12 +37,11 @@ struct testInfo {
 };
 
 static int
-qemuXML2XMLPreFormatCallback(virDomainDefPtr def, const void *opaque)
+qemuXML2XMLPreFormatCallback(virDomainDefPtr def ATTRIBUTE_UNUSED,
+                             const void *opaque ATTRIBUTE_UNUSED)
 {
-    const struct testInfo *info = opaque;
-
-    if (qemuDomainAssignAddresses(def, info->qemuCaps, NULL))
-        return -1;
+    /* Unused for now. But can eventually be used to test
+       qemuAssignDeviceAliases for example */
 
     return 0;
 }
@@ -151,9 +150,6 @@ testCompareStatusXMLToXMLFiles(const void *opaque)
         goto cleanup;
     }
 
-    if (qemuDomainAssignAddresses(obj->def, data->qemuCaps, NULL))
-        goto cleanup;
-
     /* format it back */
     if (!(actual = virDomainObjFormat(driver.xmlopt, obj, NULL,
                                       VIR_DOMAIN_DEF_FORMAT_SECURE))) {
-- 
2.5.0


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]