[libvirt] [PATCH 2/4] storage: Introduce virStoragePoolObjListForEach

[John Ferlan]

On 11/23/2017 04:42 AM, Erik Skultety wrote:
> On Thu, Nov 16, 2017 at 11:58:03AM -0500, John Ferlan wrote:
>> Create an API to walk the pools->objs[] list in order to perform a
>> callback function for each element of the objs array that doesn't care
>> about whether the action succeeds or fails as the desire is to run the
>> code over every element in the array rather than fail as soon as or if
>> one fails.
>>
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>>  src/conf/virstorageobj.c     |  29 +++++++++++
>>  src/conf/virstorageobj.h     |   9 ++++
>>  src/libvirt_private.syms     |   1 +
>>  src/storage/storage_driver.c | 115 +++++++++++++++++++++++--------------------
>>  4 files changed, 100 insertions(+), 54 deletions(-)
>>
>> diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
>> index 2ca8453139..3cae34d970 100644
>> --- a/src/conf/virstorageobj.c
>> +++ b/src/conf/virstorageobj.c
>> @@ -230,6 +230,35 @@ virStoragePoolObjListFree(virStoragePoolObjListPtr pools)
>>  }
>>
>>
>> +/**
>> + * virStoragePoolObjListForEach
>> + * @pools: Pointer to pools object
>> + * @iter: Callback iteration helper
>> + * @opaque: Opaque data to use as argument to helper
>> + *
>> + * For each object in @pools, call the @iter helper using @opaque as
>> + * an argument.  This function doesn't care whether the @iter fails or
>> + * not as it's being used for Autostart and UpdateAllState callers
>> + * that want to iterate over all the @pools objects not stopping if
>> + * one happens to fail.
>> + */
>> +void
>> +virStoragePoolObjListForEach(virStoragePoolObjListPtr pools,
>> +                             virStoragePoolObjListIterator iter,
>> +                             const void *opaque)
>> +{
>> +    size_t i;
>> +    virStoragePoolObjPtr obj;
>> +
>> +    for (i = 0; i < pools->count; i++) {
>> +        obj = pools->objs[i];
>> +        virStoragePoolObjLock(obj);
>> +        iter(obj, opaque);
>> +        virStoragePoolObjUnlock(obj);
>> +    }
>> +}
>> +
>> +
>>  void
>>  virStoragePoolObjRemove(virStoragePoolObjListPtr pools,
>>                          virStoragePoolObjPtr obj)
>> diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
>> index a4d7186d12..c84877694e 100644
>> --- a/src/conf/virstorageobj.h
>> +++ b/src/conf/virstorageobj.h
>> @@ -226,6 +226,15 @@ virStoragePoolObjFree(virStoragePoolObjPtr obj);
>>  void
>>  virStoragePoolObjListFree(virStoragePoolObjListPtr pools);
>>
>> +typedef void
>> +(*virStoragePoolObjListIterator)(virStoragePoolObjPtr obj,
>> +                                 const void *opaque);
>> +
>> +void
>> +virStoragePoolObjListForEach(virStoragePoolObjListPtr pools,
>> +                             virStoragePoolObjListIterator iter,
>> +                             const void *opaque);
>> +
>>  void
>>  virStoragePoolObjRemove(virStoragePoolObjListPtr pools,
>>                          virStoragePoolObjPtr obj);
>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>> index 57ba8f4038..62f423649a 100644
>> --- a/src/libvirt_private.syms
>> +++ b/src/libvirt_private.syms
>> @@ -1089,6 +1089,7 @@ virStoragePoolObjIsActive;
>>  virStoragePoolObjIsAutostart;
>>  virStoragePoolObjIsDuplicate;
>>  virStoragePoolObjListExport;
>> +virStoragePoolObjListForEach;
>>  virStoragePoolObjListFree;
>>  virStoragePoolObjLoadAllConfigs;
>>  virStoragePoolObjLoadAllState;
>> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
>> index 7cc3c518b4..db327a875a 100644
>> --- a/src/storage/storage_driver.c
>> +++ b/src/storage/storage_driver.c
>> @@ -157,24 +157,75 @@ storagePoolUpdateState(virStoragePoolObjPtr obj)
>>      return;
>>  }
>>
>> +
>> +
>> +static void
>> +storagePoolUpdateAllStateCallback(virStoragePoolObjPtr obj,
>> +                                  const void *opaque ATTRIBUTE_UNUSED)
>> +{
>> +    storagePoolUpdateState(obj);
>> +}
> 
> May I suggest converting storagePoolUpdateState to
> storagePoolUpdateStateCallback? I this particular case I find the additional
> level of wrapping functions unnecessary.
> 
> Reviewed-by: Erik Skultety <eskultet at redhat.com>
> 

Hmm..  right... I looked ahead in the other series to see if there was a
reason for this and I couldn't find one (it's been a few weeks since I
made the changes so I couldn't remember). At some point in time
(shortly) there's a conversion to use hash tables too (it's really close).

Tks -

John

(