2:55 p.m.
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
2:55 p.m.
New subject: [libvirt] [PATCH v2 1/4] tests: qemuxml2argv: Tweak TEST_CAPS_PATH
Make it an actual path and not a string prefix
Signed-off-by: Cole Robinson <crobinso(a)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
4:06 a.m.
New subject: [libvirt] [PATCH v2 1/4] tests: qemuxml2argv: Tweak TEST_CAPS_PATH
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(a)redhat.com>
---
tests/qemuxml2argvtest.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
2:55 p.m.
New subject: [libvirt] [PATCH v2 2/4] tests: qemuxml2argv: move DO_CAPS_TEST* qemuCaps init
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(a)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
4:13 a.m.
New subject: [libvirt] [PATCH v2 2/4] tests: qemuxml2argv: move DO_CAPS_TEST* qemuCaps init
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(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
2:55 p.m.
New subject: [libvirt] [PATCH v2 3/4] tests: qemuxml2argv: report error on ARG_* collisions
* 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(a)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
4:20 a.m.
New subject: [libvirt] [PATCH v2 3/4] tests: qemuxml2argv: report error on ARG_* collisions
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(a)redhat.com>
---
tests/qemuxml2argvtest.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
2:55 p.m.
New subject: [libvirt] [PATCH v2 4/4] tests: qemuxml2argv: add DO_TEST_INTERNAL
Base macro to unify the actual testCompareXMLToArgv test calls
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
Signed-off-by: Cole Robinson <crobinso(a)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
4:22 a.m.
New subject: [libvirt] [PATCH v2 4/4] tests: qemuxml2argv: add DO_TEST_INTERNAL
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(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
2072
days inactive
2073
days old
8 comments
2 participants
participants (2)
-
Andrea Bolognani -
Cole Robinson