Re: [libvirt] [PATCH v4 3/4] nwfilter: Convert _virNWFilterObjList to use virObjectRWLockable

>> >> @@ -391,8 +449,11 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr >> nwfilters, >>   { >>       virNWFilterObjPtr obj; >>       virNWFilterDefPtr objdef; >> +    char uuidstr[VIR_UUID_STRING_BUFLEN]; >> >> -    if ((obj = virNWFilterObjListFindByUUID(nwfilters, def->uuid))) { >> +    virUUIDFormat(def->uuid, uuidstr); >> + >> +    if ((obj = virNWFilterObjListFindByUUID(nwfilters, uuidstr))) { >>           objdef = obj->def; >> >>           if (STRNEQ(def->name, objdef->name)) { >> @@ -406,10 +467,7 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr >> nwfilters, >>           virNWFilterObjEndAPI(&obj); >>       } else { >>           if ((obj = virNWFilterObjListFindByName(nwfilters, >> def->name))) { >> -            char uuidstr[VIR_UUID_STRING_BUFLEN]; >> - >>               objdef = obj->def; >> -            virUUIDFormat(objdef->uuid, uuidstr); >>               virReportError(VIR_ERR_OPERATION_FAILED, >>                              _("filter '%s' already exists with uuid >> %s"), >>                              def->name, uuidstr); >> @@ -424,11 +482,13 @@ >> 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))) { >> -        virNWFilterObjPromoteToWrite(obj); >> +    /* 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 it >> +     * while we adjust data within. */ >> +    virObjectRWLockRead(nwfilters); >> +    if ((obj = virNWFilterObjListFindByNameLocked(nwfilters, >> def->name))) { >> +        virObjectRWUnlock(nwfilters); >> +        virObjectRWLockWrite(obj); >> >>           objdef = obj->def; >>           if (virNWFilterDefEqual(def, objdef)) { > > I think you should have left the above PromoteToWrite(obj) rather than > doing a virObjectRWLockWrite(obj) now and would then adapt the code here > as mentioned in review of 2/4. > Hmm... true... I think that's because originally I had done the list patch before the object patch... So it's probably one of those merge things... Good catch.