[libvirt PATCH 3/7] qemuxml2argvtest: Unlock virDomainObj before disposal

[Peter Krempa]

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 at 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)