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

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(a)redhat.com&gt; > --- > 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(a)redhat.com&gt;