From patchwork Tue Mar 8 16:36:34 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Cole Robinson X-Patchwork-Id: 63693 Delivered-To: patch@linaro.org Received: by 10.112.199.169 with SMTP id jl9csp2130520lbc; Tue, 8 Mar 2016 08:39:52 -0800 (PST) X-Received: by 10.28.176.200 with SMTP id z191mr20742540wme.91.1457455188571; Tue, 08 Mar 2016 08:39:48 -0800 (PST) Return-Path: Received: from mx6-phx2.redhat.com (mx6-phx2.redhat.com. [209.132.183.39]) by mx.google.com with ESMTPS id y186si5466234wmy.43.2016.03.08.08.39.47 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 08 Mar 2016 08:39:48 -0800 (PST) Received-SPF: pass (google.com: domain of libvir-list-bounces@redhat.com designates 209.132.183.39 as permitted sender) client-ip=209.132.183.39; Authentication-Results: mx.google.com; spf=pass (google.com: domain of libvir-list-bounces@redhat.com designates 209.132.183.39 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by mx6-phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u28GbBCW000958; Tue, 8 Mar 2016 11:37:11 -0500 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id u28Gaobg012364 for ; Tue, 8 Mar 2016 11:36:50 -0500 Received: from colepc.redhat.com (ovpn-113-126.phx2.redhat.com [10.3.113.126]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u28GalpR004012; Tue, 8 Mar 2016 11:36:49 -0500 From: Cole Robinson To: libvirt-list@redhat.com Date: Tue, 8 Mar 2016 11:36:34 -0500 Message-Id: <209031ef4d69aabf415b64b180139fe6de82b033.1457454944.git.crobinso@redhat.com> In-Reply-To: References: In-Reply-To: References: X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-loop: libvir-list@redhat.com Cc: Andrea Bolognani Subject: [libvirt] [PATCH 3/8] qemu: Assign device addresses in PostParse X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com 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(-) -- 2.5.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list 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 @@ - + +
+ 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))) {