[libvirt] [PATCH v5 2/3] nwfilter: Convert _virNWFilterObjList to use virObjectRWLockable

Stefan Berger stefanb at linux.vnet.ibm.com
Thu Feb 8 21:13:34 UTC 2018


On 02/08/2018 08:13 AM, Michal Privoznik wrote:
> On 02/06/2018 08:20 PM, John Ferlan wrote:
>> Implement the self locking object list for nwfilter object lists
>> that uses two hash tables to store the nwfilter object by UUID or
>> by Name.
>>
>> As part of this alter the uuid argument to virNWFilterObjLookupByUUID
>> to expect an already formatted uuidstr.
>>
>> Alter the existing list traversal code to implement the hash table
>> find/lookup/search functionality.
>>
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>>   src/conf/virnwfilterobj.c      | 405 ++++++++++++++++++++++++++++-------------
>>   src/conf/virnwfilterobj.h      |   2 +-
>>   src/nwfilter/nwfilter_driver.c |   5 +-
>>   3 files changed, 283 insertions(+), 129 deletions(-)
>
>> @@ -425,24 +483,26 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfilters,
>>           return NULL;
>>       }
>>   
>> -
>> -    /* Get a READ lock and immediately promote to WRITE while we adjust
>> -     * data within. */
>> -    if ((obj = virNWFilterObjListFindByName(nwfilters, def->name))) {
>> +    /* We're about to make some changes to objects on the list - so get the
>> +     * list READ lock in order to Find the object and WRITE lock the object
>> +     * since both paths would immediately promote it anyway */
>> +    virObjectRWLockRead(nwfilters);
>> +    if ((obj = virNWFilterObjListFindByNameLocked(nwfilters, def->name))) {
>> +        virObjectRWLockWrite(obj);
>> +        virObjectRWUnlock(nwfilters);
>>   
>>           objdef = obj->def;
>>           if (virNWFilterDefEqual(def, objdef)) {
>> -            virNWFilterObjPromoteToWrite(obj);
>>               virNWFilterDefFree(objdef);
>>               obj->def = def;
>>               virNWFilterObjDemoteFromWrite(obj);
>>               return obj;
>>           }
>>   
>> -        /* Set newDef and run the trigger with a demoted lock since it may need
>> -         * to grab a read lock on this object and promote before returning. */
>> -        virNWFilterObjPromoteToWrite(obj);
>>           obj->newDef = def;
>> +
>> +        /* Demote while the trigger runs since it may need to grab a read
>> +         * lock on this object and promote before returning. */
>>           virNWFilterObjDemoteFromWrite(obj);
>>   
>>           /* trigger the update on VMs referencing the filter */
>> @@ -462,39 +522,113 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfilters,
>>           return obj;
>>       }
>>   
>> +    /* Promote the nwfilters to add a new object */
>> +    virObjectRWUnlock(nwfilters);
>> +    virObjectRWLockWrite(nwfilters);
> How can this work? What if there's another thread waiting for the write
> lock (e.g. just to define an NWFilter)? The Unlock(nwfilters) wakes up
> the other thread, it does its job, unlocks the list waking us in turn.
> So we lock @nwfilters for writing and add something into the hash table
> - without all the checks (e.g. duplicity check) done earlier.

Could we keep the read lock and grab a 2nd lock as a write-lock? It 
wouldn't be a virObject call, though.

     Stefan




More information about the libvir-list mailing list