[PATCH] testCompareXMLToArgvValidateSchema: Construct @vm from scratch

Michal Privoznik posted 1 patch 4 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1cf03c733ddae24aa5f423bbeb89b98ad9b5f6f6.1590087503.git.mprivozn@redhat.com
Test syntax-check failed
tests/qemuxml2argvtest.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)

[PATCH] testCompareXMLToArgvValidateSchema: Construct @vm from scratch

Posted by Michal Privoznik 4 days ago
Currently, the @vm is passed in as an argument and
testCompareXMLToArgvCreateArgs() is called over it which means
under the hood qemuProcessPrepareDomain() is called. But at the
point where ValidateSchema() is called, the domain object is
already 'prepared', i.e. all device aliases are assigned and so
on. But our code is not prepared to 'prepare' a domain twice - it
simply overwrites all the pointers leading to a memory leak.

Fortunately, this is only the problem of this test.

Resolve this by constructing the domain object from scratch.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 tests/qemuxml2argvtest.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 4f613e8f1a..3103cac884 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -482,16 +482,17 @@ testCompareXMLToArgvCreateArgs(virQEMUDriverPtr drv,
 
 static int
 testCompareXMLToArgvValidateSchema(virQEMUDriverPtr drv,
-                                   virDomainObjPtr vm,
                                    const char *migrateURI,
                                    struct testQemuInfo *info,
                                    unsigned int flags)
 {
     VIR_AUTOSTRINGLIST args = NULL;
+    g_autoptr(virDomainObj) vm = NULL;
     size_t nargs = 0;
     size_t i;
     g_autoptr(virHashTable) schema = NULL;
     g_autoptr(virCommand) cmd = NULL;
+    unsigned int parseFlags = info->parseFlags;
 
     if (info->schemafile)
         schema = testQEMUSchemaLoad(info->schemafile);
@@ -504,6 +505,15 @@ testCompareXMLToArgvValidateSchema(virQEMUDriverPtr drv,
     if (!schema)
         return 0;
 
+    if (!(vm = virDomainObjNew(driver.xmlopt)))
+        return -1;
+
+    parseFlags |= VIR_DOMAIN_DEF_PARSE_INACTIVE;
+    if (!(vm->def = virDomainDefParseFile(info->infile,
+                                          driver.xmlopt,
+                                          NULL, parseFlags)))
+        return -1;
+
     if (!(cmd = testCompareXMLToArgvCreateArgs(drv, vm, migrateURI, info, flags,
                                                true)))
         return -1;
@@ -651,7 +661,7 @@ testCompareXMLToArgv(const void *data)
         goto cleanup;
     }
 
-    if (testCompareXMLToArgvValidateSchema(&driver, vm, migrateURI, info, flags) < 0)
+    if (testCompareXMLToArgvValidateSchema(&driver, migrateURI, info, flags) < 0)
         goto cleanup;
 
     if (!(actualargv = virCommandToString(cmd, false)))
-- 
2.26.2

Re: [PATCH] testCompareXMLToArgvValidateSchema: Construct @vm from scratch

Posted by Peter Krempa 4 days ago
On Thu, May 21, 2020 at 20:58:23 +0200, Michal Privoznik wrote:
> Currently, the @vm is passed in as an argument and
> testCompareXMLToArgvCreateArgs() is called over it which means
> under the hood qemuProcessPrepareDomain() is called. But at the
> point where ValidateSchema() is called, the domain object is
> already 'prepared', i.e. all device aliases are assigned and so

Not completely though. First pass consumes some things.

> on. But our code is not prepared to 'prepare' a domain twice - it
> simply overwrites all the pointers leading to a memory leak.
> 
> Fortunately, this is only the problem of this test.

I wanted to prevent parsing it twice, but that shouldn't be a problem
for now. Definitely it's lesser problem until we stop loading the
schema for every single test run.

> Resolve this by constructing the domain object from scratch.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  tests/qemuxml2argvtest.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)

Reviewed-by: Peter Krempa <pkrempa@redhat.com>