testutilsqemu: Improve error propagation from 'testQemuInfoSetArgs'

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@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

diff --git a/tests/qemustatusxml2xmltest.c b/tests/qemustatusxml2xmltest.c
index 995ef68a4c7a257fae2b4e82a24b67832e931bd0..d58f4b69dab8942cbfbcf6033abec0167b469125 100644 (file)
--- 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 db76c37001f5683929f7fd4537295e1462f314c0..2f7e088fd04093766b7ac02cc6dd5c879c93b96e 100644 (file)
--- 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 e71b77b967ae38ef683daa931dea55bc1aca5956..92fab7d1694193a0ccf4a643175f07b0961a49bd 100644 (file)
--- 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 821f6e2707eabf87e8a872086f1332b9dba53cdf..81d4a44f9a832df3ff15ca7d5af6b9491953cd8b 100644 (file)
--- 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 af7e756c0502a6942056514af6ab29f41dec6ea2..d59fa5323986264dd7dc04d0174b78d3d9ec8424 100644 (file)
--- 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