[libvirt] [PATCH v1 02/11] virStoragePoolObjListForEach: Grab a reference for pool object

Michal Privoznik posted 11 patches 5 years, 6 months ago
[libvirt] [PATCH v1 02/11] virStoragePoolObjListForEach: Grab a reference for pool object
Posted by Michal Privoznik 5 years, 6 months ago
Turns out there's one callback that might remove a storage pool
during its run: storagePoolUpdateAllState() call
storagePoolUpdateStateCallback() which may call
virStoragePoolUpdateInactive() which in turn may call
virStoragePoolObjRemove(). Problem is that the
UpdateStateCallback() sees a storage pool object with just two
references: one for each hash table holding the object. If the
function ends up calling ObjRemove() then upon removing the
object from hash tables those references are gone and thus any
subsequent call touching the object is invalid.

The solution to this problem is to grab reference for the object
we are running iterator with.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/conf/virstorageobj.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 6af4a1a22d..286f55fb0c 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -411,9 +411,13 @@ virStoragePoolObjListForEachCb(void *payload,
     virStoragePoolObjPtr obj = payload;
     struct _virStoragePoolObjListForEachData *data = opaque;
 
+    /* Grab a reference so that we don't rely only on references grabbed by
+     * hash table earlier. Remember, an iterator can remove object from the
+     * hash table. */
+    virObjectRef(obj);
     virObjectLock(obj);
     data->iter(obj, data->opaque);
-    virObjectUnlock(obj);
+    virStoragePoolObjEndAPI(&obj);
 
     return 0;
 }
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 02/11] virStoragePoolObjListForEach: Grab a reference for pool object
Posted by Ján Tomko 5 years, 3 months ago
On Fri, May 24, 2019 at 04:35:38PM +0200, Michal Privoznik wrote:
>Turns out there's one callback that might remove a storage pool
>during its run: storagePoolUpdateAllState() call
>storagePoolUpdateStateCallback() which may call
>virStoragePoolUpdateInactive() which in turn may call
>virStoragePoolObjRemove(). Problem is that the
>UpdateStateCallback() sees a storage pool object with just two
>references: one for each hash table holding the object. If the
>function ends up calling ObjRemove() then upon removing the
>object from hash tables those references are gone and thus any
>subsequent call touching the object is invalid.
>
>The solution to this problem is to grab reference for the object
>we are running iterator with.
>
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
> src/conf/virstorageobj.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list