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

On 03/13/2018 03:44 AM, Marc Hartmayer wrote: [...] Hmm... The only place where vm = NULL && !virDomainObjIsActive is during bhyveDomainUndefine... And yes, that one needs an Unref mainly because of how virDomainObjListRemove operates. (NB: You added a comment under bhyveDomainLookupByUUID which confused me at first). Long story short, for now any virDomainObjListRemove called in the error path of a virDomainObjListAdd would need to set vm = NULL, but other consumers would need to call virObjectUnref (e.g. bhyveDomainUndefine and bhyveDomainDestroy) since ListRemove returns w/ 1 ref and unlocked. Of course I have the "proper handling" of ListRemove in later patches in part of the series that hasn't been posted, so in my mind it's already there ;-) Well sort of ;-) The other half of the series is virDomainObjListAdd will return the correct number of refs. Currently it returns an object with 2 refs and locked - it should be returning with 3 refs (1 for object alloc, 1 for insertion into doms->objs, and 1 for insertion into doms->objsName). Thus after getting a vm from *Add, one should also use the virDomainObjEndAPI to reduce the refcnt by 1. When virDomainObjListRemove is called it will reduce refcnt by 1 for each call to virHashRemoveEntry (e.g. by 2) and unlock before returning with 1 ref (which is essentially the proper way). Note that "some" drivers will return from Add *and* call virObjectRef which does effectively what "should" be done in Add which is where I'm trying to get. John I have added in my local branch a virObjectUnref(vm) for Undefine and Destroy after ListRemove is called and before vm = NULL;