[libvirt] [PATCH v2 07/11] qemu_domain: Allow qemuDomainObjListAdd to keep job upon return

[Nikolay Shirokovskiy]

On 05.06.2019 12:09, Michal Privoznik wrote:
> In some cases, caller of qemuDomainObjListAdd() tries to acquire
> MODIFY job after the call. Let's adjust qemuDomainObjListAdd() so
> that it will keep the job set upon return (if requested by
> caller).
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/qemu/qemu_domain.c    | 17 ++++++++++++++---
>  src/qemu/qemu_domain.h    |  1 +
>  src/qemu/qemu_driver.c    | 24 +++++++++++-------------
>  src/qemu/qemu_migration.c |  2 +-
>  4 files changed, 27 insertions(+), 17 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index f6b677c69e..b0b3fa5fd8 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -7061,6 +7061,7 @@ virDomainDefParserConfig virQEMUDriverDomainDefParserConfig = {
>   * @def: domain definition
>   * @oldDef: previous domain definition
>   * @live: whether @def is live definition
> + * @keepJob: whether to leave MODIFY job set on returned object
>   * @flags: an bitwise-OR of virDomainObjListAdd flags
>   *
>   * Add a domain onto the list of domain object and sets its
> @@ -7070,6 +7071,10 @@ virDomainDefParserConfig virQEMUDriverDomainDefParserConfig = {
>   * In addition to that, if definition of an existing domain is
>   * changed a MODIFY job is acquired prior to that.
>   *
> + * If @keepJob is true, then the MODIFY job is not ended upon
> + * successful return from this function. This might be handy if
> + * caller would try to acquire the job anyway.
> + *
>   * Returns: domain object pointer on success,
>   *          NULL otherwise.
>   */
> @@ -7078,9 +7083,11 @@ qemuDomainObjListAdd(virQEMUDriverPtr driver,
>                       virDomainDefPtr def,
>                       virDomainDefPtr *oldDef,
>                       bool live,
> +                     bool keepJob,
>                       unsigned int flags)
>  {
>      virDomainObjPtr vm = NULL;
> +    bool defSet = false;
>  
>      if (!(vm = virDomainObjListAdd(driver->domains, def, driver->xmlopt, flags)))
>          return NULL;
> @@ -7091,7 +7098,9 @@ qemuDomainObjListAdd(virQEMUDriverPtr driver,
>       * just set the definition without acquiring job. */
>      if (!vm->def) {
>          virDomainObjAssignDef(vm, def, live, oldDef);
> -        VIR_RETURN_PTR(vm);
> +        defSet = true;
> +        if (!keepJob)
> +            VIR_RETURN_PTR(vm);
>      }

Just realized. If we got in this branch and have @keepJob = true
and later fail to aqcuire job and remove vm entirely then we
free @def which is unexpected by callers.

Nikolay