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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list