[libvirt] [PATCH 0/3] tests: schema: Try to improve debugability
Peter Krempa (3): tests: schema: Simplify memory handling using g_autofree tests: utils: Introduce helper for dispatching libvirt errors tests: virschema: Propagate errors from directory traversal in testSchemaDir tests/testutils.c | 29 +++++++++++++++++++++++++---- tests/testutils.h | 1 + tests/virschematest.c | 34 +++++++++++++--------------------- 3 files changed, 39 insertions(+), 25 deletions(-) -- 2.23.0
Refactor various functions to avoid multiple freeing function calls. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/virschematest.c | 26 +++++++------------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/tests/virschematest.c b/tests/virschematest.c index 5d092d8c8e..8a2322b3bd 100644 --- a/tests/virschematest.c +++ b/tests/virschematest.c @@ -72,8 +72,6 @@ testSchemaDir(const char *schema, struct dirent *ent; int ret = 0; int rc; - char *test_name = NULL; - char *xml_path = NULL; struct testSchemaData data = { .validator = validator, }; @@ -82,6 +80,9 @@ testSchemaDir(const char *schema, return -1; while ((rc = virDirRead(dir, &ent, dir_path)) > 0) { + g_autofree char *test_name = NULL; + g_autofree char *xml_path = NULL; + if (!virStringHasSuffix(ent->d_name, ".xml")) continue; if (ent->d_name[0] == '.') @@ -94,16 +95,11 @@ testSchemaDir(const char *schema, data.xml_path = xml_path; if (virTestRun(test_name, testSchemaFile, &data) < 0) ret = -1; - - VIR_FREE(test_name); - VIR_FREE(xml_path); } if (rc < 0) ret = -1; - VIR_FREE(test_name); - VIR_FREE(xml_path); VIR_DIR_CLOSE(dir); return ret; } @@ -114,19 +110,16 @@ testSchemaDirs(const char *schema, virXMLValidatorPtr validator, ...) { va_list args; int ret = 0; - char *dir_path = NULL; const char *dir; va_start(args, validator); while ((dir = va_arg(args, char *))) { - dir_path = g_strdup_printf("%s/%s", abs_srcdir, dir); + g_autofree char *dir_path = g_strdup_printf("%s/%s", abs_srcdir, dir); if (testSchemaDir(schema, validator, dir_path) < 0) ret = -1; - VIR_FREE(dir_path); } - VIR_FREE(dir_path); va_end(args); return ret; } @@ -136,20 +129,15 @@ static int testSchemaGrammar(const void *opaque) { struct testSchemaData *data = (struct testSchemaData *) opaque; - char *schema_path; - int ret = -1; + g_autofree char *schema_path = NULL; schema_path = g_strdup_printf("%s/docs/schemas/%s", abs_top_srcdir, data->schema); if (!(data->validator = virXMLValidatorInit(schema_path))) - goto cleanup; - - ret = 0; + return -1; - cleanup: - VIR_FREE(schema_path); - return ret; + return 0; } -- 2.23.0
In cases when we call a libvirt helper which reports an error the error would be hidden unless libvirt library debug is on. This produces a lot of output and is hard to debug. The helper provides a way to dispatch the libvirt error in specific cases sice we do already dispatch it in case when virTestRun is used. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/testutils.c | 29 +++++++++++++++++++++++++---- tests/testutils.h | 1 + 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/tests/testutils.c b/tests/testutils.c index f5d8487736..d9c0d5abbe 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -81,6 +81,30 @@ virTestGetFlag(const char *name) } +/** + * virTestPropagateLibvirtError: + * + * In cases when a libvirt utility function which reports libvirt errors is + * used in the test suite outside of the virTestRun call and the failure of such + * a function would cause an test failure the error message reported by that + * function will not be propagated to the user as the error callback is not + * invoked. + * + * In cases when the error message may be beneficial in debugging this helper + * provides means to dispatch the errors including invocation of the error + * callback. + */ +void +virTestPropagateLibvirtError(void) +{ + if (virGetLastErrorCode() == VIR_ERR_OK) + return; + + if (virTestGetVerbose() || virTestGetDebug()) + virDispatchError(NULL); +} + + /* * Runs test * @@ -112,10 +136,7 @@ virTestRun(const char *title, virResetLastError(); ret = body(data); - if (virGetLastErrorCode()) { - if (virTestGetVerbose() || virTestGetDebug()) - virDispatchError(NULL); - } + virTestPropagateLibvirtError(); if (virTestGetVerbose()) { if (ret == 0) diff --git a/tests/testutils.h b/tests/testutils.h index 0088251dca..76090c5522 100644 --- a/tests/testutils.h +++ b/tests/testutils.h @@ -84,6 +84,7 @@ unsigned int virTestGetDebug(void); unsigned int virTestGetVerbose(void); unsigned int virTestGetExpensive(void); unsigned int virTestGetRegenerate(void); +void virTestPropagateLibvirtError(void); #define VIR_TEST_DEBUG(fmt, ...) \ do { \ -- 2.23.0
testSchemaDir is a helper which invokes the schema test using virTestRun on all schema files. Since the function itself is not called inside virTestRun any helper function call is not dispatched to the user and thus it's hard to debug the test. Propagate errors from the directory traversal. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/virschematest.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/virschematest.c b/tests/virschematest.c index 8a2322b3bd..df50ef1717 100644 --- a/tests/virschematest.c +++ b/tests/virschematest.c @@ -76,8 +76,10 @@ testSchemaDir(const char *schema, .validator = validator, }; - if (virDirOpen(&dir, dir_path) < 0) + if (virDirOpen(&dir, dir_path) < 0) { + virTestPropagateLibvirtError(); return -1; + } while ((rc = virDirRead(dir, &ent, dir_path)) > 0) { g_autofree char *test_name = NULL; @@ -97,8 +99,10 @@ testSchemaDir(const char *schema, ret = -1; } - if (rc < 0) + if (rc < 0) { + virTestPropagateLibvirtError(); ret = -1; + } VIR_DIR_CLOSE(dir); return ret; -- 2.23.0
On 11/19/19 12:08 PM, Peter Krempa wrote:
Peter Krempa (3): tests: schema: Simplify memory handling using g_autofree tests: utils: Introduce helper for dispatching libvirt errors tests: virschema: Propagate errors from directory traversal in testSchemaDir
tests/testutils.c | 29 +++++++++++++++++++++++++---- tests/testutils.h | 1 + tests/virschematest.c | 34 +++++++++++++--------------------- 3 files changed, 39 insertions(+), 25 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Michal Privoznik -
Peter Krempa