[libvirt] [PATCH 01/20] bhyve: Use virDomainObjListFindBy{UUID|ID}Ref

[Marc Hartmayer]

On Fri, Mar 09, 2018 at 05:47 PM +0100, John Ferlan <jferlan at redhat.com> wrote:
> For bhyveDomObjFromDomain, bhyveDomainLookupByUUID, and
> bhyveDomainLookupByID let's return a locked and referenced
> @vm object so that callers can then use the common and more
> consistent virDomainObjEndAPI in order to handle cleanup rather
> than needing to know that the returned object is locked and
> calling virObjectUnlock.
>
> The LookupByName already returns the ref counted and locked object,
> so this will make things more consistent.
>
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---


>  src/bhyve/bhyve_driver.c | 58 ++++++++++++++++++------------------------------
>  1 file changed, 21 insertions(+), 37 deletions(-)
>
> diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
> index 849d3abcd..79963570c 100644
> --- a/src/bhyve/bhyve_driver.c
> +++ b/src/bhyve/bhyve_driver.c
> @@ -168,7 +168,7 @@ bhyveDomObjFromDomain(virDomainPtr domain)
>      bhyveConnPtr privconn = domain->conn->privateData;
>      char uuidstr[VIR_UUID_STRING_BUFLEN];
>
> -    vm = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
> +    vm = virDomainObjListFindByUUIDRef(privconn->domains, domain->uuid);
>      if (!vm) {
>          virUUIDFormat(domain->uuid, uuidstr);
>          virReportError(VIR_ERR_NO_DOMAIN,
> @@ -312,8 +312,7 @@ bhyveDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info)
>      ret = 0;
>
>   cleanup:
> -    if (vm)
> -        virObjectUnlock(vm);
> +    virDomainObjEndAPI(&vm);
>      return ret;
>  }
>
> @@ -338,8 +337,7 @@ bhyveDomainGetState(virDomainPtr domain,
>      ret = 0;
>
>   cleanup:
> -    if (vm)
> -        virObjectUnlock(vm);
> +    virDomainObjEndAPI(&vm);
>      return ret;
>  }
>
> @@ -359,8 +357,7 @@ bhyveDomainGetAutostart(virDomainPtr domain, int *autostart)
>      ret = 0;
>
>   cleanup:
> -    if (vm)
> -        virObjectUnlock(vm);
> +    virDomainObjEndAPI(&vm);
>      return ret;
>  }
>
> @@ -423,8 +420,7 @@ bhyveDomainSetAutostart(virDomainPtr domain, int autostart)
>   cleanup:
>      VIR_FREE(configFile);
>      VIR_FREE(autostartLink);
> -    if (vm)
> -        virObjectUnlock(vm);
> +    virDomainObjEndAPI(&vm);
>      return ret;
>  }
>
> @@ -443,8 +439,7 @@ bhyveDomainIsActive(virDomainPtr domain)
>      ret = virDomainObjIsActive(obj);
>
>   cleanup:
> -    if (obj)
> -        virObjectUnlock(obj);
> +    virDomainObjEndAPI(&obj);
>      return ret;
>  }
>
> @@ -463,8 +458,7 @@ bhyveDomainIsPersistent(virDomainPtr domain)
>      ret = obj->persistent;
>
>   cleanup:
> -    if (obj)
> -        virObjectUnlock(obj);
> +    virDomainObjEndAPI(&obj);
>      return ret;
>  }
>
> @@ -484,8 +478,7 @@ bhyveDomainGetOSType(virDomainPtr dom)
>          goto cleanup;
>
>   cleanup:
> -    if (vm)
> -        virObjectUnlock(vm);
> +    virDomainObjEndAPI(&vm);
>      return ret;
>  }
>
> @@ -512,8 +505,7 @@ bhyveDomainGetXMLDesc(virDomainPtr domain, unsigned int flags)
>
>      virObjectUnref(caps);
>   cleanup:
> -    if (vm)
> -        virObjectUnlock(vm);
> +    virDomainObjEndAPI(&vm);
>      return ret;
>  }
>
> @@ -630,8 +622,7 @@ bhyveDomainUndefine(virDomainPtr domain)
>      ret = 0;
>
>   cleanup:
> -    if (vm)
> -        virObjectUnlock(vm);
> +    virDomainObjEndAPI(&vm);
>      if (event)
>          virObjectEventStateQueue(privconn->domainEventState, event);
>      return ret;
> @@ -803,7 +794,7 @@ bhyveDomainLookupByUUID(virConnectPtr conn,
>      virDomainObjPtr vm;
>      virDomainPtr dom = NULL;
>
> -    vm = virDomainObjListFindByUUID(privconn->domains, uuid);
> +    vm = virDomainObjListFindByUUIDRef(privconn->domains, uuid);
>
>      if (!vm) {
>          char uuidstr[VIR_UUID_STRING_BUFLEN];
> @@ -819,8 +810,7 @@ bhyveDomainLookupByUUID(virConnectPtr conn,
>      dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
>
>   cleanup:
> -    if (vm)
> -        virObjectUnlock(vm);
> +    virDomainObjEndAPI(&vm);
>      return dom;
>  }

Is there now no memory leak (as there is a unref missing) when @vm is
set to NULL if virDomainObjIsActive(vm) returns false? Similar cases are
bhyveDomainCreateXML, bhyveDomainDefineXMLFlags, and
bhyveDomainDestroy.

But maybe I'm just missing something :)

[…snip]

--
Beste Grüße / Kind regards
   Marc Hartmayer

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294