[libvirt] [PATCH v2 0/4] tests: qemuxml2argv: support optional arguments
Small/completion follow up to v1 here, which is mostly pushed: https://www.redhat.com/archives/libvir-list/2019-March/msg00943.html Patch 1 and 3 are new additions based on Andrea's reviews. Rest of the review comments should be addressed Cole Robinson (4): tests: qemuxml2argv: Tweak TEST_CAPS_PATH tests: qemuxml2argv: move DO_CAPS_TEST* qemuCaps init tests: qemuxml2argv: report error on ARG_* collisions tests: qemuxml2argv: add DO_TEST_INTERNAL tests/qemuxml2argvtest.c | 102 +++++++++++++++++++++++++-------------- 1 file changed, 66 insertions(+), 36 deletions(-) -- 2.21.0
Make it an actual path and not a string prefix Signed-off-by: Cole Robinson <crobinso@redhat.com> --- tests/qemuxml2argvtest.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 14d3df02a9..d982a497a9 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -819,7 +819,7 @@ mymain(void) * the test cases should be forked using DO_TEST_CAPS_VER with the appropriate * version. */ -# define TEST_CAPS_PATH abs_srcdir "/qemucapabilitiesdata/caps_" +# define TEST_CAPS_PATH abs_srcdir "/qemucapabilitiesdata" # define DO_TEST_CAPS_INTERNAL(_name, arch, ver, ...) \ do { \ @@ -827,7 +827,7 @@ mymain(void) .name = _name, \ .suffix = "." arch "-" ver, \ }; \ - static const char *capsfile = TEST_CAPS_PATH ver "." arch ".xml"; \ + static const char *capsfile = TEST_CAPS_PATH "/caps_" ver "." arch ".xml"; \ static bool stripmachinealiases; \ if (STREQ(ver, "latest")) { \ capsfile = virHashLookup(capslatest, arch); \ -- 2.21.0
On Thu, 2019-03-21 at 15:55 -0400, Cole Robinson wrote:
Make it an actual path and not a string prefix
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- tests/qemuxml2argvtest.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization
Move DO_CAPS_TEST* qemuCaps init and all the associated setup into testInfoSetArgs, adding ARG_CAPS_ARCH and ARG_CAPS_VER options and using those to build the capsfile path locally Signed-off-by: Cole Robinson <crobinso@redhat.com> --- tests/qemuxml2argvtest.c | 69 +++++++++++++++++++++++++++------------- 1 file changed, 47 insertions(+), 22 deletions(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index d982a497a9..191a43726d 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -605,6 +605,8 @@ testCompareXMLToArgv(const void *data) return ret; } +# define TEST_CAPS_PATH abs_srcdir "/qemucapabilitiesdata" + typedef enum { ARG_QEMU_CAPS, ARG_GIC, @@ -612,6 +614,8 @@ typedef enum { ARG_MIGRATE_FD, ARG_FLAGS, ARG_PARSEFLAGS, + ARG_CAPS_ARCH, + ARG_CAPS_VER, /* ARG_END is our va_args sentinel. The value QEMU_CAPS_LATEST is * necessary to handle the DO_TEST(..., NONE) case, which through macro @@ -628,15 +632,19 @@ typedef enum { } testInfoArgName; static int -testInfoSetArgs(struct testInfo *info, ...) +testInfoSetArgs(struct testInfo *info, + virHashTablePtr capslatest, ...) { va_list argptr; testInfoArgName argname; virQEMUCapsPtr qemuCaps = NULL; int gic = GIC_NONE; + char *capsarch = NULL; + char *capsver = NULL; + VIR_AUTOFREE(char *) capsfile = NULL; int ret = -1; - va_start(argptr, info); + va_start(argptr, capslatest); while ((argname = va_arg(argptr, testInfoArgName)) < ARG_END) { switch (argname) { case ARG_QEMU_CAPS: @@ -665,6 +673,14 @@ testInfoSetArgs(struct testInfo *info, ...) info->parseFlags = va_arg(argptr, int); break; + case ARG_CAPS_ARCH: + capsarch = va_arg(argptr, char *); + break; + + case ARG_CAPS_VER: + capsver = va_arg(argptr, char *); + break; + case ARG_END: default: fprintf(stderr, "Unexpected test info argument"); @@ -672,13 +688,32 @@ testInfoSetArgs(struct testInfo *info, ...) } } - if (!info->qemuCaps) { - if (!qemuCaps) { - fprintf(stderr, "No qemuCaps generated\n"); + if (!qemuCaps && capsarch && capsver) { + bool stripmachinealiases = false; + + if (STREQ(capsver, "latest")) { + if (VIR_STRDUP(capsfile, virHashLookup(capslatest, capsarch)) < 0) + goto cleanup; + stripmachinealiases = true; + } else if (virAsprintf(&capsfile, "%s/caps_%s.%s.xml", + TEST_CAPS_PATH, capsver, capsarch) < 0) { goto cleanup; } - VIR_STEAL_PTR(info->qemuCaps, qemuCaps); + + if (!(qemuCaps = qemuTestParseCapabilitiesArch(virArchFromString(capsarch), + capsfile))) + goto cleanup; + + if (stripmachinealiases) + virQEMUCapsStripMachineAliases(qemuCaps); + info->flags |= FLAG_REAL_CAPS; + } + + if (!qemuCaps) { + fprintf(stderr, "No qemuCaps generated\n"); + goto cleanup; } + VIR_STEAL_PTR(info->qemuCaps, qemuCaps); if (gic != GIC_NONE && testQemuCapsSetGIC(info->qemuCaps, gic) < 0) goto cleanup; @@ -819,28 +854,17 @@ mymain(void) * the test cases should be forked using DO_TEST_CAPS_VER with the appropriate * version. */ -# define TEST_CAPS_PATH abs_srcdir "/qemucapabilitiesdata" - # define DO_TEST_CAPS_INTERNAL(_name, arch, ver, ...) \ do { \ static struct testInfo info = { \ .name = _name, \ .suffix = "." arch "-" ver, \ }; \ - static const char *capsfile = TEST_CAPS_PATH "/caps_" ver "." arch ".xml"; \ - static bool stripmachinealiases; \ - if (STREQ(ver, "latest")) { \ - capsfile = virHashLookup(capslatest, arch); \ - stripmachinealiases = true; \ - } \ - if (!(info.qemuCaps = qemuTestParseCapabilitiesArch(virArchFromString(arch), \ - capsfile))) \ - return EXIT_FAILURE; \ - if (stripmachinealiases) \ - virQEMUCapsStripMachineAliases(info.qemuCaps); \ - if (testInfoSetArgs(&info, __VA_ARGS__, ARG_END) < 0) \ + if (testInfoSetArgs(&info, capslatest, \ + ARG_CAPS_ARCH, arch, \ + ARG_CAPS_VER, ver, \ + __VA_ARGS__, ARG_END) < 0) \ return EXIT_FAILURE; \ - info.flags |= FLAG_REAL_CAPS; \ if (virTestRun("QEMU XML-2-ARGV " _name "." arch "-" ver, \ testCompareXMLToArgv, &info) < 0) \ ret = -1; \ @@ -876,7 +900,8 @@ mymain(void) static struct testInfo info = { \ .name = _name, \ }; \ - if (testInfoSetArgs(&info, __VA_ARGS__, QEMU_CAPS_LAST, ARG_END) < 0) \ + if (testInfoSetArgs(&info, capslatest, \ + __VA_ARGS__, QEMU_CAPS_LAST, ARG_END) < 0) \ return EXIT_FAILURE; \ if (virTestRun("QEMU XML-2-ARGV " _name, \ testCompareXMLToArgv, &info) < 0) \ -- 2.21.0
On Thu, 2019-03-21 at 15:55 -0400, Cole Robinson wrote: [...]
+ if (!(qemuCaps = qemuTestParseCapabilitiesArch(virArchFromString(capsarch), + capsfile))) + goto cleanup;
I'd put curly brackets around the body here. [...]
+ if (testInfoSetArgs(&info, capslatest, \ + ARG_CAPS_ARCH, arch, \ + ARG_CAPS_VER, ver, \ + __VA_ARGS__, ARG_END) < 0) \
I'd put ARG_END on its own line both here... [...]
+ if (testInfoSetArgs(&info, capslatest, \ + __VA_ARGS__, QEMU_CAPS_LAST, ARG_END) < 0) \
... and here. Regardless of whether or not you decide to make those tweaks, Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization
* ARG_CAPS_ARCH must be specified with ARG_CAPS_VER * ARG_QEMU_CAPS shouldn't be specified with ARG_CAPS_* Signed-off-by: Cole Robinson <crobinso@redhat.com> --- tests/qemuxml2argvtest.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 191a43726d..34394181c1 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -688,6 +688,18 @@ testInfoSetArgs(struct testInfo *info, } } + if (!!capsarch ^ !!capsver) { + fprintf(stderr, "ARG_CAPS_ARCH and ARG_CAPS_VER " + "must be specified together.\n"); + goto cleanup; + } + + if (qemuCaps && (capsarch || capsver)) { + fprintf(stderr, "ARG_QEMU_CAPS can not be combined with ARG_CAPS_ARCH " + "or ARG_CAPS_VER\n"); + goto cleanup; + } + if (!qemuCaps && capsarch && capsver) { bool stripmachinealiases = false; -- 2.21.0
On Thu, 2019-03-21 at 15:55 -0400, Cole Robinson wrote:
* ARG_CAPS_ARCH must be specified with ARG_CAPS_VER * ARG_QEMU_CAPS shouldn't be specified with ARG_CAPS_*
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- tests/qemuxml2argvtest.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization
Base macro to unify the actual testCompareXMLToArgv test calls Reviewed-by: Andrea Bolognani <abologna@redhat.com> Signed-off-by: Cole Robinson <crobinso@redhat.com> --- tests/qemuxml2argvtest.c | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 34394181c1..4b05b4e846 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -866,23 +866,27 @@ mymain(void) * the test cases should be forked using DO_TEST_CAPS_VER with the appropriate * version. */ -# define DO_TEST_CAPS_INTERNAL(_name, arch, ver, ...) \ +# define DO_TEST_INTERNAL(_name, _suffix, ...) \ do { \ static struct testInfo info = { \ .name = _name, \ - .suffix = "." arch "-" ver, \ + .suffix = _suffix, \ }; \ if (testInfoSetArgs(&info, capslatest, \ - ARG_CAPS_ARCH, arch, \ - ARG_CAPS_VER, ver, \ __VA_ARGS__, ARG_END) < 0) \ return EXIT_FAILURE; \ - if (virTestRun("QEMU XML-2-ARGV " _name "." arch "-" ver, \ + if (virTestRun("QEMU XML-2-ARGV " _name _suffix, \ testCompareXMLToArgv, &info) < 0) \ ret = -1; \ testInfoClear(&info); \ } while (0) +# define DO_TEST_CAPS_INTERNAL(name, arch, ver, ...) \ + DO_TEST_INTERNAL(name, "." arch "-" ver, \ + ARG_CAPS_ARCH, arch, \ + ARG_CAPS_VER, ver, \ + __VA_ARGS__) + # define DO_TEST_CAPS_ARCH_VER(name, arch, ver) \ DO_TEST_CAPS_INTERNAL(name, arch, ver, ARG_END) @@ -907,19 +911,8 @@ mymain(void) ARG_FLAGS, FLAG_EXPECT_PARSE_ERROR) -# define DO_TEST_FULL(_name, ...) \ - do { \ - static struct testInfo info = { \ - .name = _name, \ - }; \ - if (testInfoSetArgs(&info, capslatest, \ - __VA_ARGS__, QEMU_CAPS_LAST, ARG_END) < 0) \ - return EXIT_FAILURE; \ - if (virTestRun("QEMU XML-2-ARGV " _name, \ - testCompareXMLToArgv, &info) < 0) \ - ret = -1; \ - testInfoClear(&info); \ - } while (0) +# define DO_TEST_FULL(name, ...) \ + DO_TEST_INTERNAL(name, "", __VA_ARGS__, QEMU_CAPS_LAST) /* All the following macros require an explicit QEMU_CAPS_* list * at the end of the argument list, or the NONE placeholder. -- 2.21.0
On Thu, 2019-03-21 at 15:55 -0400, Cole Robinson wrote: [...]
+# define DO_TEST_FULL(name, ...) \ + DO_TEST_INTERNAL(name, "", __VA_ARGS__, QEMU_CAPS_LAST)
I'd put the last two arguments on a separate line. Either way, Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization
participants (2)
-
Andrea Bolognani -
Cole Robinson