Re: [libvirt PATCH 3/7] qemuxml2argvtest: Unlock virDomainObj before disposal
On Thu, Aug 05, 2021 at 15:08:47 +0200, Tim Wiederhake wrote:
virDomainObj contains a mutex. Destroying a locked mutex results in
undefined behavior.
I presume all of the unlocks you've added are purely based on success
code paths you've observed, right?
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
tests/qemuxml2argvtest.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index b552f5deed..4276f062a2 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
Specifically, we have:
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainObj, virObjectUnref);
and then in this function:
g_autoptr(virDomainObj) vm = NULL;
@@ -562,8 +562,9 @@ testCompareXMLToArgvValidateSchema(virQEMUDriver
*drv,
if (virBitmapParse("0-3", &priv->autoNodeset, 4) < 0)
return -1;
So in all other cases that hit the automatic cleanup path before
virDomainObjEndAPI (yes, they would result in test suite failure), such
as in the above call to virBitmapParse, or rather the
'virDomainDefParseFile' call before it (which can fail easily on XML
errors) will still hit the problem.
So it seems that we either will rely on the fact that the undefined
behaviour is sane in our case, or we need a more systemic approach than
unlocking the few cases, to fix this properly.
- if (!(cmd = testCompareXMLToArgvCreateArgs(drv, vm, migrateURI, info, flags,
- true)))
+ cmd = testCompareXMLToArgvCreateArgs(drv, vm, migrateURI, info, flags, true);
+ virDomainObjEndAPI(&vm);
+ if (!cmd)
return -1;
if (virCommandGetArgList(cmd, &args, &nargs) < 0)