[libvirt] [PATCH 4/4] storage: Complete implementation volume by hash object

[Michal Privoznik]

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 at redhat.com>
> ---
>  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.

> +}
> +
> +


> +
> +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 ;-)

> +        goto cleanup;
> +
> +    data->count++;
> +
> + cleanup:
> +    virObjectUnlock(volobj);
> +    return 0;
> +}

Michal