[libvirt PATCH 0/9] tests: cputest: use g_auto more (glib chronicles)
Also improve the compilation times (unoptimized optimization routines take too long when there are too many branches in a function, which is the case of cputest's main) and switch qemuxml2* to use the same approach. Ján Tomko (9): qemu: monitor: define cleanup function for qemuMonitorCPUModelInfo tests: use g_auto in cpuTestMakeQEMUCaps tests: cputest: use g_auto for virQEMUCaps tests: cputest: use g_auto for virCPUData tests: cputest: use g_auto for virCPUDef tests: cputest: use g_autofree tests: cputest: remove unnecessary labels tests: cputests: introduce and use virTestRunLog tests: qemuxml2*test: switch to virRunTestLog src/qemu/qemu_monitor.h | 1 + tests/cputest.c | 239 +++++++++++----------------------- tests/qemustatusxml2xmltest.c | 3 +- tests/qemuxml2argvtest.c | 10 +- tests/qemuxml2xmltest.c | 9 +- tests/testutils.c | 30 +++++ tests/testutils.h | 4 + tests/testutilsqemu.h | 1 - 8 files changed, 114 insertions(+), 183 deletions(-) -- 2.31.1
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_monitor.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index f2659d650e..8af271dc96 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1214,6 +1214,7 @@ int qemuMonitorGetCPUModelExpansion(qemuMonitor *mon, qemuMonitorCPUModelInfo **model_info); void qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfo *model_info); +G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuMonitorCPUModelInfo, qemuMonitorCPUModelInfoFree); int qemuMonitorGetCPUModelBaseline(qemuMonitor *mon, virCPUDef *cpu_a, -- 2.31.1
Refactor to use automatic cleanup and remove the goto's. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/cputest.c | 31 ++++++++++--------------------- 1 file changed, 10 insertions(+), 21 deletions(-) diff --git a/tests/cputest.c b/tests/cputest.c index 7816de87f7..a87aaa64d0 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -463,18 +463,18 @@ typedef enum { static virQEMUCaps * cpuTestMakeQEMUCaps(const struct data *data) { - virQEMUCaps *qemuCaps = NULL; - qemuMonitorTest *testMon = NULL; - qemuMonitorCPUModelInfo *model = NULL; - virCPUDef *cpu = NULL; + g_autoptr(virQEMUCaps) qemuCaps = NULL; + g_autoptr(qemuMonitorTest) testMon = NULL; + g_autoptr(qemuMonitorCPUModelInfo) model = NULL; + g_autoptr(virCPUDef) cpu = NULL; bool fail_no_props = true; - char *json = NULL; + g_autofree char *json = NULL; json = g_strdup_printf("%s/cputestdata/%s-cpuid-%s.json", abs_srcdir, virArchToString(data->arch), data->host); if (!(testMon = qemuMonitorTestNewFromFile(json, driver.xmlopt, true))) - goto error; + return NULL; qemuMonitorTestAllowUnusedCommands(testMon); @@ -488,10 +488,10 @@ cpuTestMakeQEMUCaps(const struct data *data) if (qemuMonitorGetCPUModelExpansion(qemuMonitorTestGetMonitor(testMon), QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC, cpu, true, fail_no_props, &model) < 0) - goto error; + return NULL; if (!(qemuCaps = virQEMUCapsNew())) - goto error; + return NULL; virQEMUCapsSet(qemuCaps, QEMU_CAPS_KVM); if (data->flags == JSON_MODELS || @@ -504,20 +504,9 @@ cpuTestMakeQEMUCaps(const struct data *data) if (virQEMUCapsProbeCPUDefinitionsTest(qemuCaps, qemuMonitorTestGetMonitor(testMon)) < 0) - goto error; + return NULL; - cleanup: - qemuMonitorCPUModelInfoFree(model); - qemuMonitorTestFree(testMon); - virCPUDefFree(cpu); - VIR_FREE(json); - - return qemuCaps; - - error: - virObjectUnref(qemuCaps); - qemuCaps = NULL; - goto cleanup; + return g_steal_pointer(&qemuCaps); } -- 2.31.1
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/cputest.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/tests/cputest.c b/tests/cputest.c index a87aaa64d0..78521a5cf9 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -514,7 +514,7 @@ static int cpuTestGetCPUModels(const struct data *data, virDomainCapsCPUModels **models) { - virQEMUCaps *qemuCaps; + g_autoptr(virQEMUCaps) qemuCaps = NULL; *models = NULL; @@ -527,8 +527,6 @@ cpuTestGetCPUModels(const struct data *data, *models = virQEMUCapsGetCPUModels(qemuCaps, VIR_DOMAIN_VIRT_KVM, NULL, NULL); - virObjectUnref(qemuCaps); - return 0; } @@ -861,7 +859,7 @@ static int cpuTestJSONCPUID(const void *arg) { const struct data *data = arg; - virQEMUCaps *qemuCaps = NULL; + g_autoptr(virQEMUCaps) qemuCaps = NULL; virCPUDef *cpu = NULL; char *result = NULL; int ret = -1; @@ -883,7 +881,6 @@ cpuTestJSONCPUID(const void *arg) ret = cpuTestCompareXML(data->arch, cpu, result); cleanup: - virObjectUnref(qemuCaps); virCPUDefFree(cpu); VIR_FREE(result); return ret; @@ -894,7 +891,7 @@ static int cpuTestJSONSignature(const void *arg) { const struct data *data = arg; - virQEMUCaps *qemuCaps = NULL; + g_autoptr(virQEMUCaps) qemuCaps = NULL; virCPUData *hostData = NULL; qemuMonitorCPUModelInfo *modelInfo; int ret = -1; @@ -909,7 +906,6 @@ cpuTestJSONSignature(const void *arg) ret = cpuTestCompareSignature(data, hostData); cleanup: - virObjectUnref(qemuCaps); virCPUDataFree(hostData); return ret; } -- 2.31.1
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/cputest.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/tests/cputest.c b/tests/cputest.c index 78521a5cf9..5cf6db9be3 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -404,7 +404,7 @@ cpuTestHasFeature(const void *arg) const struct data *data = arg; int ret = -1; virCPUDef *host = NULL; - virCPUData *hostData = NULL; + g_autoptr(virCPUData) hostData = NULL; int result; if (!(host = cpuTestLoadXML(data->arch, data->host))) @@ -434,7 +434,6 @@ cpuTestHasFeature(const void *arg) ret = 0; cleanup: - virCPUDataFree(hostData); virCPUDefFree(host); return ret; } @@ -552,7 +551,7 @@ cpuTestCPUID(bool guest, const void *arg) { const struct data *data = arg; int ret = -1; - virCPUData *hostData = NULL; + g_autoptr(virCPUData) hostData = NULL; char *hostFile = NULL; char *host = NULL; virCPUDef *cpu = NULL; @@ -596,7 +595,6 @@ cpuTestCPUID(bool guest, const void *arg) cleanup: VIR_FREE(hostFile); VIR_FREE(host); - virCPUDataFree(hostData); virCPUDefFree(cpu); VIR_FREE(result); virObjectUnref(models); @@ -646,7 +644,7 @@ static int cpuTestCPUIDSignature(const void *arg) { const struct data *data = arg; - virCPUData *hostData = NULL; + g_autoptr(virCPUData) hostData = NULL; char *hostFile = NULL; char *host = NULL; int ret = -1; @@ -661,7 +659,6 @@ cpuTestCPUIDSignature(const void *arg) ret = cpuTestCompareSignature(data, hostData); cleanup: - virCPUDataFree(hostData); VIR_FREE(hostFile); VIR_FREE(host); return ret; @@ -765,10 +762,10 @@ cpuTestUpdateLive(const void *arg) virCPUDef *cpu = NULL; char *enabledFile = NULL; char *enabled = NULL; - virCPUData *enabledData = NULL; + g_autoptr(virCPUData) enabledData = NULL; char *disabledFile = NULL; char *disabled = NULL; - virCPUData *disabledData = NULL; + g_autoptr(virCPUData) disabledData = NULL; char *expectedFile = NULL; virCPUDef *expected = NULL; virDomainCapsCPUModels *hvModels = NULL; @@ -842,10 +839,8 @@ cpuTestUpdateLive(const void *arg) virCPUDefFree(cpu); VIR_FREE(enabledFile); VIR_FREE(enabled); - virCPUDataFree(enabledData); VIR_FREE(disabledFile); VIR_FREE(disabled); - virCPUDataFree(disabledData); VIR_FREE(expectedFile); virCPUDefFree(expected); virObjectUnref(hvModels); @@ -892,7 +887,7 @@ cpuTestJSONSignature(const void *arg) { const struct data *data = arg; g_autoptr(virQEMUCaps) qemuCaps = NULL; - virCPUData *hostData = NULL; + g_autoptr(virCPUData) hostData = NULL; qemuMonitorCPUModelInfo *modelInfo; int ret = -1; @@ -906,7 +901,6 @@ cpuTestJSONSignature(const void *arg) ret = cpuTestCompareSignature(data, hostData); cleanup: - virCPUDataFree(hostData); return ret; } #endif -- 2.31.1
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/cputest.c | 36 ++++++++++++------------------------ 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/tests/cputest.c b/tests/cputest.c index 5cf6db9be3..9fd5b38b78 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -193,8 +193,8 @@ cpuTestCompare(const void *arg) { const struct data *data = arg; int ret = -1; - virCPUDef *host = NULL; - virCPUDef *cpu = NULL; + g_autoptr(virCPUDef) host = NULL; + g_autoptr(virCPUDef) cpu = NULL; virCPUCompareResult result; if (!(host = cpuTestLoadXML(data->arch, data->host)) || @@ -217,8 +217,6 @@ cpuTestCompare(const void *arg) ret = 0; cleanup: - virCPUDefFree(host); - virCPUDefFree(cpu); return ret; } @@ -228,8 +226,8 @@ cpuTestGuestCPU(const void *arg) { const struct data *data = arg; int ret = -2; - virCPUDef *host = NULL; - virCPUDef *cpu = NULL; + g_autoptr(virCPUDef) host = NULL; + g_autoptr(virCPUDef) cpu = NULL; virCPUCompareResult cmpResult; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; char *result = NULL; @@ -268,8 +266,6 @@ cpuTestGuestCPU(const void *arg) cleanup: VIR_FREE(result); - virCPUDefFree(host); - virCPUDefFree(cpu); if (ret == data->result) { /* We got the result we expected, whether it was @@ -370,9 +366,9 @@ cpuTestUpdate(const void *arg) { const struct data *data = arg; int ret = -1; - virCPUDef *host = NULL; - virCPUDef *migHost = NULL; - virCPUDef *cpu = NULL; + g_autoptr(virCPUDef) host = NULL; + g_autoptr(virCPUDef) migHost = NULL; + g_autoptr(virCPUDef) cpu = NULL; char *result = NULL; if (!(host = cpuTestLoadXML(data->arch, data->host)) || @@ -390,9 +386,6 @@ cpuTestUpdate(const void *arg) ret = cpuTestCompareXML(data->arch, cpu, result); cleanup: - virCPUDefFree(host); - virCPUDefFree(cpu); - virCPUDefFree(migHost); VIR_FREE(result); return ret; } @@ -403,7 +396,7 @@ cpuTestHasFeature(const void *arg) { const struct data *data = arg; int ret = -1; - virCPUDef *host = NULL; + g_autoptr(virCPUDef) host = NULL; g_autoptr(virCPUData) hostData = NULL; int result; @@ -434,7 +427,6 @@ cpuTestHasFeature(const void *arg) ret = 0; cleanup: - virCPUDefFree(host); return ret; } @@ -554,7 +546,7 @@ cpuTestCPUID(bool guest, const void *arg) g_autoptr(virCPUData) hostData = NULL; char *hostFile = NULL; char *host = NULL; - virCPUDef *cpu = NULL; + g_autoptr(virCPUDef) cpu = NULL; char *result = NULL; virDomainCapsCPUModels *models = NULL; @@ -595,7 +587,6 @@ cpuTestCPUID(bool guest, const void *arg) cleanup: VIR_FREE(hostFile); VIR_FREE(host); - virCPUDefFree(cpu); VIR_FREE(result); virObjectUnref(models); return ret; @@ -759,7 +750,7 @@ cpuTestUpdateLive(const void *arg) { const struct data *data = arg; char *cpuFile = NULL; - virCPUDef *cpu = NULL; + g_autoptr(virCPUDef) cpu = NULL; char *enabledFile = NULL; char *enabled = NULL; g_autoptr(virCPUData) enabledData = NULL; @@ -767,7 +758,7 @@ cpuTestUpdateLive(const void *arg) char *disabled = NULL; g_autoptr(virCPUData) disabledData = NULL; char *expectedFile = NULL; - virCPUDef *expected = NULL; + g_autoptr(virCPUDef) expected = NULL; virDomainCapsCPUModels *hvModels = NULL; virDomainCapsCPUModels *models = NULL; int ret = -1; @@ -836,13 +827,11 @@ cpuTestUpdateLive(const void *arg) cleanup: VIR_FREE(cpuFile); - virCPUDefFree(cpu); VIR_FREE(enabledFile); VIR_FREE(enabled); VIR_FREE(disabledFile); VIR_FREE(disabled); VIR_FREE(expectedFile); - virCPUDefFree(expected); virObjectUnref(hvModels); virObjectUnref(models); return ret; @@ -855,7 +844,7 @@ cpuTestJSONCPUID(const void *arg) { const struct data *data = arg; g_autoptr(virQEMUCaps) qemuCaps = NULL; - virCPUDef *cpu = NULL; + g_autoptr(virCPUDef) cpu = NULL; char *result = NULL; int ret = -1; @@ -876,7 +865,6 @@ cpuTestJSONCPUID(const void *arg) ret = cpuTestCompareXML(data->arch, cpu, result); cleanup: - virCPUDefFree(cpu); VIR_FREE(result); return ret; } -- 2.31.1
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/cputest.c | 58 ++++++++++++++++--------------------------------- 1 file changed, 19 insertions(+), 39 deletions(-) diff --git a/tests/cputest.c b/tests/cputest.c index 9fd5b38b78..2dc7e2e5ae 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -138,8 +138,8 @@ cpuTestCompareXML(virArch arch, virCPUDef *cpu, const char *name) { - char *xml = NULL; - char *actual = NULL; + g_autofree char *xml = NULL; + g_autofree char *actual = NULL; int ret = -1; xml = g_strdup_printf("%s/cputestdata/%s-%s.xml", abs_srcdir, @@ -154,8 +154,6 @@ cpuTestCompareXML(virArch arch, ret = 0; cleanup: - VIR_FREE(xml); - VIR_FREE(actual); return ret; } @@ -230,7 +228,7 @@ cpuTestGuestCPU(const void *arg) g_autoptr(virCPUDef) cpu = NULL; virCPUCompareResult cmpResult; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - char *result = NULL; + g_autofree char *result = NULL; if (!(host = cpuTestLoadXML(data->arch, data->host)) || !(cpu = cpuTestLoadXML(data->arch, data->name))) @@ -265,8 +263,6 @@ cpuTestGuestCPU(const void *arg) ret = 0; cleanup: - VIR_FREE(result); - if (ret == data->result) { /* We got the result we expected, whether it was * a success or a failure */ @@ -292,7 +288,7 @@ cpuTestBaseline(const void *arg) virCPUDef **cpus = NULL; virCPUDef *baseline = NULL; unsigned int ncpus = 0; - char *result = NULL; + g_autofree char *result = NULL; const char *suffix; size_t i; @@ -356,7 +352,6 @@ cpuTestBaseline(const void *arg) VIR_FREE(cpus); } virCPUDefFree(baseline); - VIR_FREE(result); return ret; } @@ -369,7 +364,7 @@ cpuTestUpdate(const void *arg) g_autoptr(virCPUDef) host = NULL; g_autoptr(virCPUDef) migHost = NULL; g_autoptr(virCPUDef) cpu = NULL; - char *result = NULL; + g_autofree char *result = NULL; if (!(host = cpuTestLoadXML(data->arch, data->host)) || !(cpu = cpuTestLoadXML(data->arch, data->name))) @@ -386,7 +381,6 @@ cpuTestUpdate(const void *arg) ret = cpuTestCompareXML(data->arch, cpu, result); cleanup: - VIR_FREE(result); return ret; } @@ -544,10 +538,10 @@ cpuTestCPUID(bool guest, const void *arg) const struct data *data = arg; int ret = -1; g_autoptr(virCPUData) hostData = NULL; - char *hostFile = NULL; - char *host = NULL; + g_autofree char *hostFile = NULL; + g_autofree char *host = NULL; g_autoptr(virCPUDef) cpu = NULL; - char *result = NULL; + g_autofree char *result = NULL; virDomainCapsCPUModels *models = NULL; hostFile = g_strdup_printf("%s/cputestdata/%s-cpuid-%s.xml", abs_srcdir, @@ -585,9 +579,6 @@ cpuTestCPUID(bool guest, const void *arg) ret = cpuTestCompareXML(data->arch, cpu, result); cleanup: - VIR_FREE(hostFile); - VIR_FREE(host); - VIR_FREE(result); virObjectUnref(models); return ret; } @@ -636,8 +627,8 @@ cpuTestCPUIDSignature(const void *arg) { const struct data *data = arg; g_autoptr(virCPUData) hostData = NULL; - char *hostFile = NULL; - char *host = NULL; + g_autofree char *hostFile = NULL; + g_autofree char *host = NULL; int ret = -1; hostFile = g_strdup_printf("%s/cputestdata/%s-cpuid-%s.xml", abs_srcdir, @@ -650,8 +641,6 @@ cpuTestCPUIDSignature(const void *arg) ret = cpuTestCompareSignature(data, hostData); cleanup: - VIR_FREE(hostFile); - VIR_FREE(host); return ret; } @@ -749,15 +738,15 @@ static int cpuTestUpdateLive(const void *arg) { const struct data *data = arg; - char *cpuFile = NULL; + g_autofree char *cpuFile = NULL; g_autoptr(virCPUDef) cpu = NULL; - char *enabledFile = NULL; - char *enabled = NULL; + g_autofree char *enabledFile = NULL; + g_autofree char *enabled = NULL; g_autoptr(virCPUData) enabledData = NULL; - char *disabledFile = NULL; - char *disabled = NULL; + g_autofree char *disabledFile = NULL; + g_autofree char *disabled = NULL; g_autoptr(virCPUData) disabledData = NULL; - char *expectedFile = NULL; + g_autofree char *expectedFile = NULL; g_autoptr(virCPUDef) expected = NULL; virDomainCapsCPUModels *hvModels = NULL; virDomainCapsCPUModels *models = NULL; @@ -826,12 +815,6 @@ cpuTestUpdateLive(const void *arg) ret = cpuTestUpdateLiveCompare(data->arch, cpu, expected); cleanup: - VIR_FREE(cpuFile); - VIR_FREE(enabledFile); - VIR_FREE(enabled); - VIR_FREE(disabledFile); - VIR_FREE(disabled); - VIR_FREE(expectedFile); virObjectUnref(hvModels); virObjectUnref(models); return ret; @@ -845,7 +828,7 @@ cpuTestJSONCPUID(const void *arg) const struct data *data = arg; g_autoptr(virQEMUCaps) qemuCaps = NULL; g_autoptr(virCPUDef) cpu = NULL; - char *result = NULL; + g_autofree char *result = NULL; int ret = -1; result = g_strdup_printf("cpuid-%s-json", data->host); @@ -865,7 +848,6 @@ cpuTestJSONCPUID(const void *arg) ret = cpuTestCompareXML(data->arch, cpu, result); cleanup: - VIR_FREE(result); return ret; } @@ -957,7 +939,7 @@ mymain(void) models == NULL ? NULL : #models, \ flags, result \ }; \ - char *testLabel; \ + g_autofree char *testLabel = NULL; \ \ g_free(virTestLogContentAndReset());\ \ @@ -975,7 +957,6 @@ mymain(void) ret = -1; \ } \ \ - VIR_FREE(testLabel); \ } while (0) #define DO_TEST_COMPARE(arch, host, cpu, result) \ @@ -997,7 +978,7 @@ mymain(void) #define DO_TEST_BASELINE(arch, name, flags, result) \ do { \ const char *suffix = ""; \ - char *label; \ + g_autofree char *label = NULL; \ if ((flags) & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) \ suffix = " (expanded)"; \ if ((flags) & VIR_CONNECT_BASELINE_CPU_MIGRATABLE) \ @@ -1005,7 +986,6 @@ mymain(void) label = g_strdup_printf("%s%s", name, suffix); \ DO_TEST(arch, cpuTestBaseline, label, NULL, \ "baseline-" name, NULL, flags, result); \ - VIR_FREE(label); \ } while (0) #define DO_TEST_HASFEATURE(arch, host, feature, result) \ -- 2.31.1
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/cputest.c | 72 +++++++++++++++---------------------------------- 1 file changed, 22 insertions(+), 50 deletions(-) diff --git a/tests/cputest.c b/tests/cputest.c index 2dc7e2e5ae..c8030be093 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -140,21 +140,17 @@ cpuTestCompareXML(virArch arch, { g_autofree char *xml = NULL; g_autofree char *actual = NULL; - int ret = -1; xml = g_strdup_printf("%s/cputestdata/%s-%s.xml", abs_srcdir, virArchToString(arch), name); if (!(actual = virCPUDefFormat(cpu, NULL))) - goto cleanup; + return -1; if (virTestCompareToFile(actual, xml) < 0) - goto cleanup; + return -1; - ret = 0; - - cleanup: - return ret; + return 0; } @@ -190,14 +186,13 @@ static int cpuTestCompare(const void *arg) { const struct data *data = arg; - int ret = -1; g_autoptr(virCPUDef) host = NULL; g_autoptr(virCPUDef) cpu = NULL; virCPUCompareResult result; if (!(host = cpuTestLoadXML(data->arch, data->host)) || !(cpu = cpuTestLoadXML(data->arch, data->name))) - goto cleanup; + return -1; result = virCPUCompare(host->arch, host, cpu, false); if (data->result == VIR_CPU_COMPARE_ERROR) @@ -209,13 +204,10 @@ cpuTestCompare(const void *arg) cpuTestCompResStr(result)); /* Pad to line up with test name ... in virTestRun */ VIR_TEST_VERBOSE("%74s", "... "); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - return ret; + return 0; } @@ -360,7 +352,6 @@ static int cpuTestUpdate(const void *arg) { const struct data *data = arg; - int ret = -1; g_autoptr(virCPUDef) host = NULL; g_autoptr(virCPUDef) migHost = NULL; g_autoptr(virCPUDef) cpu = NULL; @@ -368,20 +359,17 @@ cpuTestUpdate(const void *arg) if (!(host = cpuTestLoadXML(data->arch, data->host)) || !(cpu = cpuTestLoadXML(data->arch, data->name))) - goto cleanup; + return -1; if (!(migHost = virCPUCopyMigratable(data->arch, host))) - goto cleanup; + return -1; if (virCPUUpdate(host->arch, cpu, migHost) < 0) - goto cleanup; + return -1; result = g_strdup_printf("%s+%s", data->host, data->name); - ret = cpuTestCompareXML(data->arch, cpu, result); - - cleanup: - return ret; + return cpuTestCompareXML(data->arch, cpu, result); } @@ -389,17 +377,16 @@ static int cpuTestHasFeature(const void *arg) { const struct data *data = arg; - int ret = -1; g_autoptr(virCPUDef) host = NULL; g_autoptr(virCPUData) hostData = NULL; int result; if (!(host = cpuTestLoadXML(data->arch, data->host))) - goto cleanup; + return -1; if (cpuEncode(host->arch, host, NULL, &hostData, NULL, NULL, NULL, NULL) < 0) - goto cleanup; + return -1; result = virCPUCheckFeature(host->arch, host, data->name); @@ -415,13 +402,10 @@ cpuTestHasFeature(const void *arg) cpuTestBoolWithErrorStr(result)); /* Pad to line up with test name ... in virTestRun */ VIR_TEST_VERBOSE("%74s", "... "); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - return ret; + return 0; } @@ -629,19 +613,15 @@ cpuTestCPUIDSignature(const void *arg) g_autoptr(virCPUData) hostData = NULL; g_autofree char *hostFile = NULL; g_autofree char *host = NULL; - int ret = -1; hostFile = g_strdup_printf("%s/cputestdata/%s-cpuid-%s.xml", abs_srcdir, virArchToString(data->arch), data->host); if (virTestLoadFile(hostFile, &host) < 0 || !(hostData = virCPUDataParse(host))) - goto cleanup; + return -1; - ret = cpuTestCompareSignature(data, hostData); - - cleanup: - return ret; + return cpuTestCompareSignature(data, hostData); } @@ -829,12 +809,11 @@ cpuTestJSONCPUID(const void *arg) g_autoptr(virQEMUCaps) qemuCaps = NULL; g_autoptr(virCPUDef) cpu = NULL; g_autofree char *result = NULL; - int ret = -1; result = g_strdup_printf("cpuid-%s-json", data->host); if (!(qemuCaps = cpuTestMakeQEMUCaps(data))) - goto cleanup; + return -1; cpu = virCPUDefNew(); cpu->arch = data->arch; @@ -843,12 +822,9 @@ cpuTestJSONCPUID(const void *arg) cpu->fallback = VIR_CPU_FALLBACK_FORBID; if (virQEMUCapsInitCPUModel(qemuCaps, VIR_DOMAIN_VIRT_KVM, cpu, false) != 0) - goto cleanup; + return -1; - ret = cpuTestCompareXML(data->arch, cpu, result); - - cleanup: - return ret; + return cpuTestCompareXML(data->arch, cpu, result); } @@ -859,19 +835,15 @@ cpuTestJSONSignature(const void *arg) g_autoptr(virQEMUCaps) qemuCaps = NULL; g_autoptr(virCPUData) hostData = NULL; qemuMonitorCPUModelInfo *modelInfo; - int ret = -1; if (!(qemuCaps = cpuTestMakeQEMUCaps(data))) - goto cleanup; + return -1; modelInfo = virQEMUCapsGetCPUModelInfo(qemuCaps, VIR_DOMAIN_VIRT_KVM); if (!(hostData = virQEMUCapsGetCPUModelX86Data(qemuCaps, modelInfo, false))) - goto cleanup; + return -1; - ret = cpuTestCompareSignature(data, hostData); - - cleanup: - return ret; + return cpuTestCompareSignature(data, hostData); } #endif -- 2.31.1
A helper that resets the log before each test and prints it on failure. It also takes the return variable as an argument, so it can be used to eliminate number of branches the compiler has to consider in the main function. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/cputest.c | 14 +------------- tests/testutils.c | 30 ++++++++++++++++++++++++++++++ tests/testutils.h | 4 ++++ 3 files changed, 35 insertions(+), 13 deletions(-) diff --git a/tests/cputest.c b/tests/cputest.c index c8030be093..3e99b9486b 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -912,23 +912,11 @@ mymain(void) flags, result \ }; \ g_autofree char *testLabel = NULL; \ - \ - g_free(virTestLogContentAndReset());\ \ testLabel = g_strdup_printf("%s(%s): %s", #api, \ virArchToString(arch), name); \ \ - if (virTestRun(testLabel, api, &data) < 0) { \ - if (virTestGetDebug()) { \ - char *log; \ - if ((log = virTestLogContentAndReset()) && \ - strlen(log) > 0) \ - VIR_TEST_DEBUG("\n%s", log); \ - VIR_FREE(log); \ - } \ - ret = -1; \ - } \ - \ + virTestRunLog(&ret, testLabel, api, &data); \ } while (0) #define DO_TEST_COMPARE(arch, host, cpu, result) \ diff --git a/tests/testutils.c b/tests/testutils.c index 682fa142b5..5e9835ee89 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -180,6 +180,36 @@ virTestRun(const char *title, } +/* + * A wrapper for virTestRun that resets the log content before each run + * and sets ret to -1 on failure. On success, ret is untouched. + */ +void +virTestRunLog(int *ret, + const char *title, + int (*body)(const void *data), + const void *data) +{ + int rc; + + g_free(virTestLogContentAndReset()); + + rc = virTestRun(title, body, data); + + if (rc >= 0) + return; + + *ret = -1; + + if (virTestGetDebug()) { + g_autofree char *log = virTestLogContentAndReset(); + + if (strlen(log) > 0) + VIR_TEST_DEBUG("\n%s", log); + } +} + + /** * virTestLoadFile: * @file: name of the file to load diff --git a/tests/testutils.h b/tests/testutils.h index 6848323586..48de864131 100644 --- a/tests/testutils.h +++ b/tests/testutils.h @@ -40,6 +40,10 @@ extern virArch virTestHostArch; int virTestRun(const char *title, int (*body)(const void *data), const void *data); +void virTestRunLog(int *ret, + const char *title, + int (*body)(const void *data), + const void *data); int virTestLoadFile(const char *file, char **buf); char *virTestLoadFilePath(const char *p, ...) G_GNUC_NULL_TERMINATED; -- 2.31.1
This essentially reverts: commit ca5c8e1dc7801854d856cfc62f2054ae753a40c3 qemuxml2argvtest: Avoid conditions in test macro Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/qemustatusxml2xmltest.c | 3 +-- tests/qemuxml2argvtest.c | 10 ++-------- tests/qemuxml2xmltest.c | 9 +++------ tests/testutilsqemu.h | 1 - 4 files changed, 6 insertions(+), 17 deletions(-) diff --git a/tests/qemustatusxml2xmltest.c b/tests/qemustatusxml2xmltest.c index f6bde507a1..d58f4b69da 100644 --- a/tests/qemustatusxml2xmltest.c +++ b/tests/qemustatusxml2xmltest.c @@ -83,8 +83,7 @@ mymain(void) g_autoptr(virConnect) conn = NULL; struct testQemuConf testConf = { .capslatest = capslatest, .capscache = capscache, - .qapiSchemaCache = NULL, - .retptr = NULL }; + .qapiSchemaCache = NULL }; if (!capslatest) return EXIT_FAILURE; diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 4ce7ed5f00..00a85988d6 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -766,8 +766,6 @@ testCompareXMLToArgv(const void *data) goto cleanup; } - log = virTestLogContentAndReset(); - VIR_FREE(log); virResetLastError(); if (!(cmd = testCompareXMLToArgvCreateArgs(&driver, vm, migrateURI, info, @@ -826,9 +824,6 @@ testCompareXMLToArgv(const void *data) if (info->arch != VIR_ARCH_NONE && info->arch != VIR_ARCH_X86_64) qemuTestSetHostArch(&driver, VIR_ARCH_NONE); - if (ret < 0) - *info->conf->retptr = ret; - return ret; } @@ -856,8 +851,7 @@ mymain(void) g_autoptr(GHashTable) capscache = virHashNew(virObjectFreeHashData); struct testQemuConf testConf = { .capslatest = capslatest, .capscache = capscache, - .qapiSchemaCache = qapiSchemaCache, - .retptr = &ret }; + .qapiSchemaCache = qapiSchemaCache }; if (!capslatest) return EXIT_FAILURE; @@ -952,7 +946,7 @@ mymain(void) }; \ testQemuInfoSetArgs(&info, &testConf, __VA_ARGS__); \ testInfoSetPaths(&info, _suffix); \ - virTestRun("QEMU XML-2-ARGV " _name _suffix, testCompareXMLToArgv, &info); \ + virTestRunLog(&ret, "QEMU XML-2-ARGV " _name _suffix, testCompareXMLToArgv, &info); \ testQemuInfoClear(&info); \ } while (0) diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 0413a130c3..6d3526f91f 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -55,7 +55,6 @@ testXML2XMLActive(const void *opaque) info->infile, info->outfile, true, info->parseFlags, TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS) < 0) { - *info->conf->retptr = -1; return -1; } @@ -73,7 +72,6 @@ testXML2XMLInactive(const void *opaque) info->infile, info->outfile, false, info->parseFlags, TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS) < 0) { - *info->conf->retptr = -1; return -1; } @@ -118,8 +116,7 @@ mymain(void) g_autoptr(virConnect) conn = NULL; struct testQemuConf testConf = { .capslatest = capslatest, .capscache = capscache, - .qapiSchemaCache = NULL, - .retptr = &ret }; + .qapiSchemaCache = NULL }; if (!capslatest) return EXIT_FAILURE; @@ -168,12 +165,12 @@ mymain(void) \ if (when & WHEN_INACTIVE) { \ testInfoSetPaths(&info, suffix, WHEN_INACTIVE); \ - virTestRun("QEMU XML-2-XML-inactive " _name, testXML2XMLInactive, &info); \ + virTestRunLog(&ret, "QEMU XML-2-XML-inactive " _name, testXML2XMLInactive, &info); \ } \ \ if (when & WHEN_ACTIVE) { \ testInfoSetPaths(&info, suffix, WHEN_ACTIVE); \ - virTestRun("QEMU XML-2-XML-active " _name, testXML2XMLActive, &info); \ + virTestRunLog(&ret, "QEMU XML-2-XML-active " _name, testXML2XMLActive, &info); \ } \ testQemuInfoClear(&info); \ } while (0) diff --git a/tests/testutilsqemu.h b/tests/testutilsqemu.h index 64562cbd1c..d59fa53239 100644 --- a/tests/testutilsqemu.h +++ b/tests/testutilsqemu.h @@ -58,7 +58,6 @@ struct testQemuConf { GHashTable *capscache; GHashTable *capslatest; GHashTable *qapiSchemaCache; - int *retptr; }; struct testQemuArgs { -- 2.31.1
On 8/20/21 4:48 PM, Ján Tomko wrote:
Also improve the compilation times (unoptimized optimization routines take too long when there are too many branches in a function, which is the case of cputest's main) and switch qemuxml2* to use the same approach.
Ján Tomko (9): qemu: monitor: define cleanup function for qemuMonitorCPUModelInfo tests: use g_auto in cpuTestMakeQEMUCaps tests: cputest: use g_auto for virQEMUCaps tests: cputest: use g_auto for virCPUData tests: cputest: use g_auto for virCPUDef tests: cputest: use g_autofree tests: cputest: remove unnecessary labels tests: cputests: introduce and use virTestRunLog tests: qemuxml2*test: switch to virRunTestLog
src/qemu/qemu_monitor.h | 1 + tests/cputest.c | 239 +++++++++++----------------------- tests/qemustatusxml2xmltest.c | 3 +- tests/qemuxml2argvtest.c | 10 +- tests/qemuxml2xmltest.c | 9 +- tests/testutils.c | 30 +++++ tests/testutils.h | 4 + tests/testutilsqemu.h | 1 - 8 files changed, 114 insertions(+), 183 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Ján Tomko -
Michal Prívozník