[libvirt] [PATCH] qemu: aquire job in qemuDomainDefineXMLFlags

[Peter Krempa]

On Tue, May 28, 2019 at 14:34:18 +0300, Nikolay Shirokovskiy wrote:
> The function updates vm->newDef and frees previous pesistent definition
> which can be used by in different thread by some API that unlocks domain
> to communicate to qemu. So we have an access to freed memory in that
> thread. Let's acquire job condition when changing persistent xml.
> 
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
> ---
> 
> A bug example (some downstream version of libvirt, nevertheless):
> 
>     (gdb) bt
>     #0  0x00007f3bb77a0533 in virDomainDefHasVcpusOffline (def=def at entry=0x7f3b7802a830) at conf/domain_conf.c:1637
>     #1  0x00007f3bb77cb32f in virDomainCpuDefFormat (def=0x7f3b7802a830, buf=0x7f3ba7308940) at conf/domain_conf.c:27503
>     #2  virDomainDefFormatInternal (def=def at entry=0x7f3b7802a830, caps=0x7f3b70000ae0, flags=3, flags at entry=1, 
>         buf=buf at entry=0x7f3ba7308940, xmlopt=xmlopt at entry=0x0) at conf/domain_conf.c:27850
>     #3  0x00007f3bb77ced46 in virDomainDefFormat (def=def at entry=0x7f3b7802a830, caps=<optimized out>, 
>         flags=flags at entry=1) at conf/domain_conf.c:28564
>     #4  0x00007f3bb77d23f9 in virDomainSaveConfig (configDir=0x7f3b981227e0 "/etc/libvirt/qemu", caps=<optimized out>, 
>         def=0x7f3b7802a830) at conf/domain_conf.c:28776
>     #5  0x00007f3ba086b617 in qemuDomainSetMemoryStatsPeriod (dom=<optimized out>, period=30, flags=<optimized out>)
>         at qemu/qemu_driver.c:4839
>     #6  0x00007f3bb7905613 in virDomainSetMemoryStatsPeriod (domain=domain at entry=0x7f3b74006200, period=30, flags=3)
>         at libvirt-domain.c:2087
> 
>     (gdb) f 5
>     #5  0x00007f3ba086b617 in qemuDomainSetMemoryStatsPeriod (dom=<optimized out>, period=30, flags=<optimized out>)
>         at qemu/qemu_driver.c:4839
>     4839            ret = virDomainSaveConfig(cfg->configDir, driver->caps, persistentDef);
> 
>     (gdb) p vm->def
>     $8 = (virDomainDefPtr) 0x7f3b8400c6f0
>     (gdb) p vm->newDef
>     $9 = (virDomainDefPtr) 0x7f3b7001de20
>     (gdb) p persistentDef
>     $10 = (virDomainDefPtr) 0x7f3b7802a830
> 
>  src/qemu/qemu_driver.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 42b1ce2..683dcaa 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7626,6 +7626,7 @@ qemuDomainDefineXMLFlags(virConnectPtr conn,
>      virCapsPtr caps = NULL;
>      unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
>                                 VIR_DOMAIN_DEF_PARSE_ABI_UPDATE;
> +    bool job = false;
>  
>      virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL);
>  
> @@ -7647,6 +7648,10 @@ qemuDomainDefineXMLFlags(virConnectPtr conn,
>      if (virDomainDefineXMLFlagsEnsureACL(conn, def) < 0)
>          goto cleanup;
>  
> +    if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
> +        goto cleanup;
> +    job = true;

How is this supposed to work? 'vm' is NULL ...

> +
>      if (!(vm = virDomainObjListAdd(driver->domains, def,

... until this assignment.

>                                     driver->xmlopt,
>                                     0, &oldDef)))
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190528/8c7aee8e/attachment-0001.sig>