Re: [libvirt] [PATCH 2/6] tests: qemuxm2argv: Add infrastructure for testing with real qemuCaps

On 04/04/2018 04:13 AM, Peter Krempa wrote: > Allow testing of XML->argv conversion with using a real capability map > as used in the qemucapabilitiestest. This allows specifying the required > qemu version with the test rather than having to enumerate all the > required capabilities. > > Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; > --- > tests/qemuxml2argvtest.c | 31 +++++++++++++++++++++++++++++++ > 1 file changed, 31 insertions(+) > > diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c > index d79913dd0a..c540ce2f50 100644 > --- a/tests/qemuxml2argvtest.c > +++ b/tests/qemuxml2argvtest.c > @@ -654,6 +654,37 @@ mymain(void) > if (VIR_STRDUP_QUIET(driver.config->memoryBackingDir, "/var/lib/libvirt/qemu/ram") < 0) > return EXIT_FAILURE; > > + > +# define DO_TEST_CAPS_ARCH(name, migrateFrom, flags, \ > + parseFlags, gic, arch, ver) \ > + do { \ > + static struct testInfo info = { \ > + name, NULL, migrateFrom, migrateFrom ? 7 : -1, (flags), parseFlags, \ > + false, NULL \ > + }; \ > + info.skipLegacyCPUs = skipLegacyCPUs; \ > + if (testInitQEMUCaps(&info, gic) < 0) \ > + return EXIT_FAILURE; \ > + if (!(info.qemuCaps = qemuTestParseCapabilitiesArch(virArchFromString(arch), \ > + abs_srcdir "/qemucapabilitiesdata/caps_" \ > + ver "." arch ".xml"))) \ > + return EXIT_FAILURE; \ > + if (virTestRun("QEMU XML-2-ARGV " name, \ > + testCompareXMLToArgv, &info) < 0) \ > + ret = -1; \ > + if (info.vm && virTestRun("QEMU XML-2-startup-XML " name, \ > + testCompareXMLToStartupXML, &info) < 0) \ > + ret = -1; \ > + virObjectUnref(info.qemuCaps); \ > + virObjectUnref(info.vm); \ > + } while (0) > + > +# define DO_TEST_CAPS_FULL(name, flags, parseFlags, ver) \ > + DO_TEST_CAPS_ARCH(name, NULL, flags, parseFlags, GIC_NONE, "x86_64", ver) Assumes x64_64...

> + > +# define DO_TEST_CAPS(name, ver) \ > + DO_TEST_CAPS_FULL(name, 0, 0, ver) > + > # define DO_TEST_FULL(name, migrateFrom, migrateFd, flags, \ > parseFlags, gic, ...) \ > do { \ > Do you expect to see wide spread and future use of this model as opposed to the existing DO_TEST model? Shouldn't at least some of the existing tests be converted, then?

Perhaps the macro(s) should be DO_TEST_VERS[ION] or DO_TEST_<ARCH>_VERS instead since DO_TEST runs the test with specific capabilities and this new model runs the test with a specific version for a specific platform.

How do or should we enforce that when adding a new test from this point forward that the DO_TEST_CAPS model is chosen over DO_TEST? I suppose

there is still value in DO_TEST since one can pick and choose which CAP is needed - although I'll freely admit that's usually a cut-n-paste from some other test and change the name type activity. I think I have too many questions for an R-b at this point. John