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. 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).
>
>> +}
>> +
>> +
>
>
>> +
>> +static int
>> +virStoragePoolObjNumOfVolumesCb(void *payload,
>> + const void *name ATTRIBUTE_UNUSED,
>> + void *opaque)
>> +{
>> + virStorageVolObjPtr volobj = payload;
>> + struct _virStorageVolObjCountData *data = opaque;
>> +
>> + virObjectLock(volobj);
>> +
>> + if (data->filter && !data->filter(data->conn, data->pooldef,
>> + volobj->voldef))
>
> If you'd break the line after logical and it would look nicer ;-)
>
Sure, np. Same for GetNamesCb and ListExportCb too
TKs -
John
>> + goto cleanup;
>> +
>> + data->count++;
>> +
>> + cleanup:
>> + virObjectUnlock(volobj);
>> + return 0;
>> +}
>
> Michal
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
