On 01/10/2018 12:39 PM, John Ferlan wrote: > > > On 01/10/2018 04:16 AM, Michal Privoznik wrote: >> On 01/09/2018 09:05 PM, John Ferlan wrote: >>> Alter the volume logic to use the hash tables instead of forward >>> linked lists. There are three hash tables to allow for fast lookup >>> by name, target.path, and key. >>> >>> Modify the virStoragePoolObjAddVol to place the object in all 3 >>> tables if possible using self locking RWLock on the volumes object. >>> Conversely when removing the volume, it's a removal of the object >>> from the various hash tables. >>> >>> Implement functions to handle remote ForEach and Search Volume >>> type helpers. These are used by the disk backend in order to >>> facilitate adding a primary, extended, or logical partition. >>> >>> Implement the various VolDefFindBy* helpers as simple (and fast) >>> hash lookups. The NumOfVolumes, GetNames, and ListExport helpers >>> are all implemented using standard for each hash table calls. >>> >>> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> >>> --- >>> src/conf/virstorageobj.c | 420 +++++++++++++++++++++++++++++++++++------------ >>> 1 file changed, 311 insertions(+), 109 deletions(-) >>> >>> diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c >>> index 8a1c6f782..d92b2b2e3 100644 >>> --- a/src/conf/virstorageobj.c >>> +++ b/src/conf/virstorageobj.c >>> @@ -53,11 +53,6 @@ virStorageVolObjListDispose(void *opaque); >>> >> >>> >>> size_t >>> virStoragePoolObjGetVolumesCount(virStoragePoolObjPtr obj) >>> { >>> - return obj->volumes.count; >>> + return virHashSize(obj->volumes->objsKey); >> >> How come we don't need a read lock here? ... I think we should grab a >> read lock from obj->volumes just like you're doing in >> virStorageVolDefFindByKey() for instance. > > This doesn't traverse volumes->objs - it gets the size directly from the > hash table stored @objsKey. Exactly the reason why we need a lock here. What if another thread is already modifying the table? True, this is not that problematic since we don't really care if we get N or N+1 result here (moreover, this function is used only at one place and that is VIR_DEBUG, so not a big problem), but still I think we need a read lock here. > I'm not against adding an RWLock here, but > that's probably what the distinguishing factor was (I wrote the code 3 > months ago ;-) - it's just been waiting it's turn). Sure, no problem. Michal -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list