On Fri, Mar 09, 2018 at 05:47 PM +0100, John Ferlan <jferlan@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---
> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
