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

[Michal Privoznik]

On 5/28/19 6:11 PM, 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 in case of updating exiting domain.
> 
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
> ---
> 
> diff from v1 [1]:
> - pay a bit of attention to the problem
> 
> Reproducer in the Michal's reply to the [1].
> 
> [1] https://www.redhat.com/archives/libvir-list/2019-May/msg00804.html
> 
>   src/qemu/qemu_driver.c | 37 +++++++++++++++++++++++++++++++++++--
>   1 file changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 42b1ce2..2dd4442 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7620,6 +7620,7 @@ qemuDomainDefineXMLFlags(virConnectPtr conn,
>       virDomainDefPtr def = NULL;
>       virDomainDefPtr oldDef = NULL;
>       virDomainObjPtr vm = NULL;
> +    virDomainObjPtr exvm = NULL;
>       virDomainPtr dom = NULL;
>       virObjectEventPtr event = NULL;
>       virQEMUDriverConfigPtr cfg;
> @@ -7647,10 +7648,34 @@ qemuDomainDefineXMLFlags(virConnectPtr conn,
>       if (virDomainDefineXMLFlagsEnsureACL(conn, def) < 0)
>           goto cleanup;
>   
> +    if ((exvm = virDomainObjListFindByUUID(driver->domains, def->uuid))) {
> +        if (qemuDomainObjBeginJob(driver, exvm, QEMU_JOB_MODIFY) < 0) {
> +            virDomainObjEndAPI(&exvm);
> +            goto cleanup;
> +        }
> +        virObjectUnlock(exvm);
> +    }
> +
>       if (!(vm = virDomainObjListAdd(driver->domains, def,
>                                      driver->xmlopt,
> -                                   0, &oldDef)))
> +                                   0, &oldDef))) {
> +        if (exvm)
> +            virObjectLock(exvm);
>           goto cleanup;
> +    }
> +

Almost. There are other places where virDomainObjListAdd() is called and 
they require the same threatment. I'm posting a different approach.

Michal