[PATCH v2 21/23] testutilsqemu: Improve error propagation from 'testQemuInfoSetArgs'
Peter Krempa
pkrempa at redhat.com
Tue Aug 17 16:25:22 UTC 2021
Previously we've ran into problems when 'testQemuInfoSetArgs' failed as
calling the actual test executor could lead to a crash if the data
wasn't prepared but reporting an error doesn't play nicely with our test
output which is handled by 'virTestRun'.
To avoid the issue (and as a side effect improve compilation times of
the test files) split up testQemuInfoSetArgs into two functions.
The first is still called 'testQemuInfoSetArgs' and just blindly
populates arguments into a sub-struct of testQemuInfo. This function no
longer reports errors
A new function 'testQemuInfoInitArgs' which is meant to be called from
the test executor then checks errors and prepares the test data. This
one can fail and the test will be marked as failed appropriately.
A nice side effect is that this vastly improves compile times of
qemuxml2xmltest and qemuxml2argvtest.
Signed-off-by: Peter Krempa <pkrempa at redhat.com>
---
tests/qemustatusxml2xmltest.c | 12 +++--
tests/qemuxml2argvtest.c | 6 ++-
tests/qemuxml2xmltest.c | 17 +++----
tests/testutilsqemu.c | 93 ++++++++++++++++++++---------------
tests/testutilsqemu.h | 18 +++++--
5 files changed, 87 insertions(+), 59 deletions(-)
diff --git a/tests/qemustatusxml2xmltest.c b/tests/qemustatusxml2xmltest.c
index 995ef68a4c..d58f4b69da 100644
--- a/tests/qemustatusxml2xmltest.c
+++ b/tests/qemustatusxml2xmltest.c
@@ -23,6 +23,12 @@ testCompareStatusXMLToXMLFiles(const void *opaque)
g_autofree char *actual = NULL;
int ret = -1;
+ if (testQemuInfoInitArgs((struct testQemuInfo *) data) < 0)
+ return -1;
+
+ if (qemuTestCapsCacheInsert(driver.qemuCapsCache, data->qemuCaps) < 0)
+ return -1;
+
if (!(obj = virDomainObjParseFile(data->infile, driver.xmlopt,
VIR_DOMAIN_DEF_PARSE_STATUS |
VIR_DOMAIN_DEF_PARSE_ACTUAL_NET |
@@ -112,11 +118,7 @@ mymain(void)
static struct testQemuInfo info = { \
.name = _name, \
}; \
- if (testQemuInfoSetArgs(&info, &testConf, ARG_END) < 0 || \
- qemuTestCapsCacheInsert(driver.qemuCapsCache, info.qemuCaps) < 0) { \
- VIR_TEST_DEBUG("Failed to generate status test data for '%s'", _name); \
- return -1; \
- } \
+ testQemuInfoSetArgs(&info, &testConf, ARG_END); \
testInfoSetStatusPaths(&info); \
\
if (virTestRun("QEMU status XML-2-XML " _name, \
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 0dbb6ad94b..34b92c4a35 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -653,6 +653,9 @@ testCompareXMLToArgv(const void *data)
virArch arch = VIR_ARCH_NONE;
g_autoptr(virIdentity) sysident = virIdentityGetSystem();
+ if (testQemuInfoInitArgs((struct testQemuInfo *) info) < 0)
+ goto cleanup;
+
if (info->arch != VIR_ARCH_NONE && info->arch != VIR_ARCH_X86_64)
qemuTestSetHostArch(&driver, info->arch);
@@ -943,8 +946,7 @@ mymain(void)
static struct testQemuInfo info = { \
.name = _name, \
}; \
- if (testQemuInfoSetArgs(&info, &testConf, __VA_ARGS__) < 0) \
- ret = -1; \
+ testQemuInfoSetArgs(&info, &testConf, __VA_ARGS__); \
testInfoSetPaths(&info, _suffix); \
if (virTestRun("QEMU XML-2-ARGV " _name _suffix, \
testCompareXMLToArgv, &info) < 0) \
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index e71b77b967..92fab7d169 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -32,11 +32,14 @@ enum {
static int
testXML2XMLCommon(const struct testQemuInfo *info)
{
- if (!(info->flags & FLAG_REAL_CAPS)) {
+ if (testQemuInfoInitArgs((struct testQemuInfo *) info) < 0)
+ return -1;
+
+ if (!(info->flags & FLAG_REAL_CAPS))
virQEMUCapsInitQMPBasicArch(info->qemuCaps);
- if (qemuTestCapsCacheInsert(driver.qemuCapsCache, info->qemuCaps) < 0)
- return -1;
- }
+
+ if (qemuTestCapsCacheInsert(driver.qemuCapsCache, info->qemuCaps) < 0)
+ return -1;
return 0;
}
@@ -154,11 +157,7 @@ mymain(void)
static struct testQemuInfo info = { \
.name = _name, \
}; \
- if (testQemuInfoSetArgs(&info, &testConf, __VA_ARGS__) < 0 || \
- qemuTestCapsCacheInsert(driver.qemuCapsCache, info.qemuCaps) < 0) { \
- VIR_TEST_DEBUG("Failed to generate test data for '%s'", _name); \
- ret = -1; \
- } \
+ testQemuInfoSetArgs(&info, &testConf, __VA_ARGS__); \
\
if (when & WHEN_INACTIVE) { \
testInfoSetPaths(&info, suffix, WHEN_INACTIVE); \
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index 821f6e2707..81d4a44f9a 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -677,38 +677,32 @@ testQemuCapsIterate(const char *suffix,
}
-int
+void
testQemuInfoSetArgs(struct testQemuInfo *info,
struct testQemuConf *conf, ...)
{
va_list argptr;
testQemuInfoArgName argname;
- g_autoptr(virQEMUCaps) fakeCaps = virQEMUCapsNew();
- bool fakeCapsUsed = false;
- int gic = GIC_NONE;
- char *capsarch = NULL;
- char *capsver = NULL;
- g_autofree char *capsfile = NULL;
int flag;
- int ret = -1;
- if (!fakeCaps)
+ if (!(info->args.fakeCaps = virQEMUCapsNew()))
abort();
info->conf = conf;
+ info->args.newargs = true;
va_start(argptr, conf);
while ((argname = va_arg(argptr, testQemuInfoArgName)) != ARG_END) {
switch (argname) {
case ARG_QEMU_CAPS:
- fakeCapsUsed = true;
+ info->args.fakeCapsUsed = true;
while ((flag = va_arg(argptr, int)) < QEMU_CAPS_LAST)
- virQEMUCapsSet(fakeCaps, flag);
+ virQEMUCapsSet(info->args.fakeCaps, flag);
break;
case ARG_GIC:
- gic = va_arg(argptr, int);
+ info->args.gic = va_arg(argptr, int);
break;
case ARG_MIGRATE_FROM:
@@ -728,55 +722,77 @@ testQemuInfoSetArgs(struct testQemuInfo *info,
break;
case ARG_CAPS_ARCH:
- capsarch = va_arg(argptr, char *);
+ info->args.capsarch = va_arg(argptr, char *);
break;
case ARG_CAPS_VER:
- capsver = va_arg(argptr, char *);
+ info->args.capsver = va_arg(argptr, char *);
break;
case ARG_END:
default:
- fprintf(stderr, "Unexpected test info argument");
- goto cleanup;
+ info->args.invalidarg = true;
+ break;
}
+
+ if (info->args.invalidarg)
+ break;
}
- if (!!capsarch ^ !!capsver) {
- fprintf(stderr, "ARG_CAPS_ARCH and ARG_CAPS_VER "
- "must be specified together.\n");
- goto cleanup;
+ va_end(argptr);
+}
+
+
+int
+testQemuInfoInitArgs(struct testQemuInfo *info)
+{
+ g_autofree char *capsfile = NULL;
+
+ if (!info->args.newargs)
+ return 0;
+
+ info->args.newargs = false;
+
+ if (info->args.invalidarg) {
+ fprintf(stderr, "Invalid agument encountered by 'testQemuInfoSetArgs'\n");
+ return -1;
}
- if (capsarch && capsver) {
+ if (!!info->args.capsarch ^ !!info->args.capsver) {
+ fprintf(stderr, "ARG_CAPS_ARCH and ARG_CAPS_VER must be specified together.\n");
+ return -1;
+ }
+
+ if (info->args.capsarch && info->args.capsver) {
bool stripmachinealiases = false;
virQEMUCaps *cachedcaps = NULL;
- if (fakeCapsUsed) {
- fprintf(stderr, "ARG_QEMU_CAPS can not be combined with ARG_CAPS_ARCH "
- "or ARG_CAPS_VER\n");
- goto cleanup;
+ if (info->args.fakeCapsUsed) {
+ fprintf(stderr, "ARG_QEMU_CAPS can not be combined with ARG_CAPS_ARCH or ARG_CAPS_VER\n");
+ return -1;
}
- info->arch = virArchFromString(capsarch);
+ info->arch = virArchFromString(info->args.capsarch);
- if (STREQ(capsver, "latest")) {
- capsfile = g_strdup(virHashLookup(info->conf->capslatest, capsarch));
+ if (STREQ(info->args.capsver, "latest")) {
+ capsfile = g_strdup(virHashLookup(info->conf->capslatest, info->args.capsarch));
stripmachinealiases = true;
} else {
capsfile = g_strdup_printf("%s/caps_%s.%s.xml",
- TEST_QEMU_CAPS_PATH, capsver, capsarch);
+ TEST_QEMU_CAPS_PATH,
+ info->args.capsver,
+ info->args.capsarch);
}
if (!g_hash_table_lookup_extended(info->conf->capscache, capsfile, NULL, (void **) &cachedcaps)) {
if (!(cachedcaps = qemuTestParseCapabilitiesArch(info->arch, capsfile)))
- goto cleanup;
+ return -1;
g_hash_table_insert(info->conf->capscache, g_strdup(capsfile), cachedcaps);
}
if (!(info->qemuCaps = virQEMUCapsNewCopy(cachedcaps)))
- goto cleanup;
+ return -1;
if (stripmachinealiases)
virQEMUCapsStripMachineAliases(info->qemuCaps);
@@ -787,18 +803,14 @@ testQemuInfoSetArgs(struct testQemuInfo *info,
capsfile[strlen(capsfile) - 3] = '\0';
info->schemafile = g_strdup_printf("%sreplies", capsfile);
} else {
- info->qemuCaps = g_steal_pointer(&fakeCaps);
+ info->qemuCaps = g_steal_pointer(&info->args.fakeCaps);
}
- if (gic != GIC_NONE && testQemuCapsSetGIC(info->qemuCaps, gic) < 0)
- goto cleanup;
-
- ret = 0;
-
- cleanup:
- va_end(argptr);
+ if (info->args.gic != GIC_NONE &&
+ testQemuCapsSetGIC(info->qemuCaps, info->args.gic) < 0)
+ return -1;
- return ret;
+ return 0;
}
@@ -810,4 +822,5 @@ testQemuInfoClear(struct testQemuInfo *info)
VIR_FREE(info->schemafile);
VIR_FREE(info->errfile);
virObjectUnref(info->qemuCaps);
+ g_clear_pointer(&info->args.fakeCaps, virObjectUnref);
}
diff --git a/tests/testutilsqemu.h b/tests/testutilsqemu.h
index af7e756c05..d59fa53239 100644
--- a/tests/testutilsqemu.h
+++ b/tests/testutilsqemu.h
@@ -60,6 +60,16 @@ struct testQemuConf {
GHashTable *qapiSchemaCache;
};
+struct testQemuArgs {
+ bool newargs;
+ virQEMUCaps *fakeCaps;
+ bool fakeCapsUsed;
+ char *capsver;
+ char *capsarch;
+ int gic;
+ bool invalidarg;
+};
+
struct testQemuInfo {
const char *name;
char *infile;
@@ -73,6 +83,7 @@ struct testQemuInfo {
virArch arch;
char *schemafile;
+ struct testQemuArgs args;
struct testQemuConf *conf;
};
@@ -116,9 +127,10 @@ int testQemuCapsIterate(const char *suffix,
testQemuCapsIterateCallback callback,
void *opaque);
-int testQemuInfoSetArgs(struct testQemuInfo *info,
- struct testQemuConf *conf,
- ...);
+void testQemuInfoSetArgs(struct testQemuInfo *info,
+ struct testQemuConf *conf,
+ ...);
+int testQemuInfoInitArgs(struct testQemuInfo *info);
void testQemuInfoClear(struct testQemuInfo *info);
#endif
--
2.31.1
More information about the libvir-list
mailing list