[libvirt] [PATCH 10/12] tests: qemuxml2xml: Use testQemuInfoSetArgs

Andrea Bolognani abologna at redhat.com
Wed Apr 10 16:53:01 UTC 2019


On Mon, 2019-04-01 at 12:47 -0400, Cole Robinson wrote:
[...]
> @@ -170,6 +150,11 @@ mymain(void)
>      char *fakerootdir;
>      struct testQemuInfo info;
>      virQEMUDriverConfigPtr cfg = NULL;
> +    virHashTablePtr capslatest = NULL;
> +
> +    capslatest = testQemuGetCapsLatest();
> +    if (!capslatest)
> +        abort();

Woah, that's a bit harsh, isn't it? How about a nice and polite

  return EXIT_FAILURE;

instead? :)

[...]
> @@ -192,11 +177,14 @@ mymain(void)
>  
>  # define DO_TEST_FULL(name, when, gic, ...) \
>      do { \
> -        if (testInfoSetCommon(&info, gic) < 0) { \
> +        if (testQemuInfoSetArgs(&info, capslatest, \
> +                                ARG_GIC, gic, \
> +                                ARG_QEMU_CAPS, __VA_ARGS__, QEMU_CAPS_LAST, \
> +                                ARG_END) < 0 || \
> +            qemuTestCapsCacheInsert(driver.qemuCapsCache, info.qemuCaps) < 0) { \

I haven't really spent any time digging, but that call to
qemuTestCapsCacheInsert() looks odd to me. What exactly is the point
in caching if we're going to be using a different set of capabilities
pretty much every single time?

Considering it is pre-existing, of course, your change is perfectly
fine. Feel free to investigate further, though :)

Additional note: xml2argv calls this in the test function rather than
in the corresponding macro, and it would be nice if they would be
made consistent. Follow-up material, of course.

With the abort() call replaced with something more gentle,

  Reviewed-by: Andrea Bolognani <abologna at redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization




More information about the libvir-list mailing list