Re: [PATCH v2 5/6] tests: qemuxml2xml: assign device addresses

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

 



On 02/09/2016 10:59 AM, Cole Robinson wrote:
We use the PreFormat callback for this. Many test cases need to be extended
to pass in proper qemuCaps flags so AssignAddresses doesn't throw errors.

One test case (pcie-root-port-too-many) is dropped, since it was meant
only for checking an error condition in qemuxml2argv, and one we add in
AssignAddresses it errors here too.

Long term I think AssignAddresses should be handled in qemu's PostParse
callback, but that's not entirely straightforward. Handling it here
means we can get the test suite churn over with.
---

I'm too lazy to do it myself, but it would be comforting to move the xml2xmlout data files into the xml2argvdata directory and run qemuxml2argvtest with the existing .args files to verify that all of these PCI addresses you've just generated are exactly the same as the ones that have been auto-generated over the last several years and put into the .args files...

(Hmm - if the xml files in all the test data directories had their "qemuxml2XXX-" prefixes removed as we had earlier discussed, such an operation would be trivial :-)

diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index e3b61c0..a06aa4d 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -37,13 +37,24 @@ struct testInfo {
  };
static int
+qemuXML2XMLPreFormatCallback(virDomainDefPtr def, const void *opaque)
+{
+    const struct testInfo *info = opaque;
+
+    if (qemuDomainAssignAddresses(def, info->qemuCaps, NULL))
+        return -1;
+
+    return 0;
+}
+
+static int
  testXML2XMLActive(const void *opaque)
  {
      const struct testInfo *info = opaque;
return testCompareDomXML2XMLFiles(driver.caps, driver.xmlopt,
                                        info->inName, info->outActiveName, true,
-                                      NULL, NULL);
+                                      qemuXML2XMLPreFormatCallback, opaque);
  }
@@ -54,7 +65,7 @@ testXML2XMLInactive(const void *opaque) return testCompareDomXML2XMLFiles(driver.caps, driver.xmlopt, info->inName,
                                        info->outInactiveName, false,
-                                      NULL, NULL);
+                                      qemuXML2XMLPreFormatCallback, opaque);
  }
@@ -138,6 +149,9 @@ testCompareStatusXMLToXMLFiles(const void *opaque)
          goto cleanup;
      }
+ if (qemuDomainAssignAddresses(obj->def, data->qemuCaps, NULL))
+        goto cleanup;
+

Why is this needed? I though that's what the pre-format callback was for.


      /* format it back */
      if (!(actual = virDomainObjFormat(driver.xmlopt, obj, NULL,
                                        VIR_DOMAIN_DEF_FORMAT_SECURE))) {
@@ -379,14 +393,28 @@ mymain(void)
      DO_TEST("disk-drive-network-rbd-ipv6");
      DO_TEST("disk-drive-network-rbd-ceph-env");
      DO_TEST("disk-drive-network-sheepdog");
-    DO_TEST("disk-scsi-device");
+    DO_TEST_FULL("disk-scsi-device", WHEN_ACTIVE,
+            QEMU_CAPS_NODEFCONFIG,
+            QEMU_CAPS_SCSI_LSI);


The indentation of all of the followon lines to DO_TEST_FULL()'s are off. It's consistent, but since it doesn't match what all of our editors likely do automatically, so it will start to look ugly as new cases are added. (I didn't look - is the indentation also incorrect in qemuxml2argvtest.c (which is where I assume you got the caps lists for each test)?)

ACK with the question about the extra call to qemuDomainAssignAddresses() answered and the indentation fixed so that it matches what an editor's auto-indent would do (putting the following lines one space past the column of the opening "(" )


--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]