[libvirt] [PATCH 3/4] conf: Introduce virDomainObjListAddObjLocked

[John Ferlan]

On 04/06/2018 08:56 AM, Michal Privoznik wrote:
> On 03/29/2018 02:34 PM, John Ferlan wrote:
>> Create a common helper to add an object to the locked domain
>> objlist hash tables and use it.
>>
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>>  src/conf/virdomainobjlist.c | 64 +++++++++++++++++++++++++++------------------
>>  1 file changed, 39 insertions(+), 25 deletions(-)
>>
>> diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c
>> index 765f46d5a..7022abe09 100644
>> --- a/src/conf/virdomainobjlist.c
>> +++ b/src/conf/virdomainobjlist.c
>> @@ -220,6 +220,42 @@ virDomainObjPtr virDomainObjListFindByName(virDomainObjListPtr doms,
>>  }
>>  
>>  
>> +/**
>> + * @doms: Domain object list pointer
>> + * @vm: Domain object to be added
>> + *
>> + * Upon entry @vm should have at least 1 ref and be locked.
>> + *
>> + * Add the @vm into the @doms->objs and @doms->objsName hash
>> + * tables.
>> + *
>> + * Returns 0 on success with 2 references and locked
>> + *        -1 on failure with 1 reference and locked
>> + */
>> +static int
>> +virDomainObjListAddObjLocked(virDomainObjListPtr doms,
>> +                             virDomainObjPtr vm)
>> +{
>> +    char uuidstr[VIR_UUID_STRING_BUFLEN];
>> +
>> +    virUUIDFormat(vm->def->uuid, uuidstr);
>> +    if (virHashAddEntry(doms->objs, uuidstr, vm) < 0)
>> +        return -1;
>> +
>> +    if (virHashAddEntry(doms->objsName, vm->def->name, vm) < 0) {
>> +        virObjectRef(vm);
>> +        virHashRemoveEntry(doms->objs, uuidstr);
>> +        return -1;
>> +    }
>> +
>> +    /* Since domain is in two hash tables, increment the
>> +     * reference counter */
>> +    virObjectRef(vm);
> 
> I think this virObjectRef() could be moved in between those two
> virHashAddEntry() calls and thus the later call to virObjectRef() can be
> dropped then.
> 

True; however, the purpose of the first two patches was to set the code
up to mirror each other and thus have this helper instead do the same
thing. The reality is the next step *is* to put a virObjectRef between
the two and thus have 2 virObjectRef calls to match the 2 Unref calls in
the *Remove function.  The next step also removes the comment leaving an
ObjectRef after each successful AddEntry.

Doing this means "some" drivers won't need the extra virObjectRef after
they call the Add function, while "other" drivers will need to use the
Remove and EndAPI after they call the Add function.

I've been trying to coordinate this effort with the FindBy{UUID|ID}[Ref]
effort as they are (in a way) strangely related.

Tks-

John