On Wed, Apr 11, 2018 at 11:16:27 -0400, John Ferlan wrote:
> 
> 
> 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 <pkre...@redhat.com>
> > ---
> >  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...

Yes. Intentionally. Hipster-arches can introduce their own helpers.

> 
> > +
> > +# 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?

I hope so, actually for both cases.

> 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.

Hmm, yes. I can rename it to DO_TEST_VERSION. The default will be for
x86. As said, other arches can add their own macros.

> 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

I think reviewers should ask for using the new test approach for any
positive cases, so that we don't test something that is not possible in
the wild.

In cases when it's required to do so (negative tests, or weird config
tests) use of DO_TEST is justified.

> 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

Attachment: signature.asc
Description: PGP signature

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

Reply via email to