[PATCH 0/2] storage: Fix returning of locked objects from 'virStoragePoolObjListSearch'
Patch 1 improves chances to reproduce the bug. https://bugzilla.redhat.com/show_bug.cgi?id=2222210 Peter Krempa (2): file reproducer storage: Fix returning of locked objects from 'virStoragePoolObjListSearch' src/conf/virstorageobj.c | 7 ++++++- src/storage/storage_util.c | 2 ++ 2 files changed, 8 insertions(+), 1 deletion(-) -- 2.41.0
To reproduce the issue run many instances of 'virsh vol-info /some/path/to/a/storage/pool' --- src/storage/storage_util.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index d39aa92eae..a478a85a05 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -2289,6 +2289,8 @@ virStorageBackendVolRefreshLocal(virStoragePoolObj *pool G_GNUC_UNUSED, { int ret; + usleep(5000); + /* Refresh allocation / capacity / permissions info in case its changed */ if ((ret = virStorageBackendUpdateVolInfo(vol, false, VIR_STORAGE_VOL_FS_OPEN_FLAGS, -- 2.41.0
'virStoragePoolObjListSearch' explicitly documents that it's returning a pointer to a locked and ref'd pool that maches the lookup function. This was not the case as in commit 0c4b391e2a9 (released in libvirt-8.3.0) the code was accidentally converted to use 'VIR_LOCK_GUARD' which auto-unlocked it when leaving the scope, even when the code was originally "leaking" the lock. Revert the corresponding conversion and add a comment that this function is intentionally leaking a locked object. Fixes: 0c4b391e2a9 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2221851 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/virstorageobj.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index 7010e97d61..59fa5da372 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -454,11 +454,16 @@ virStoragePoolObjListSearchCb(const void *payload, virStoragePoolObj *obj = (virStoragePoolObj *) payload; struct _virStoragePoolObjListSearchData *data = (struct _virStoragePoolObjListSearchData *)opaque; - VIR_LOCK_GUARD lock = virObjectLockGuard(obj); + virObjectLock(obj); + + /* If we find the matching pool object we must return while the object is + * locked as the caller wants to return a locked object. */ if (data->searcher(obj, data->opaque)) return 1; + virObjectUnlock(obj); + return 0; } -- 2.41.0
On a Tuesday in 2023, Peter Krempa wrote:
'virStoragePoolObjListSearch' explicitly documents that it's returning a pointer to a locked and ref'd pool that maches the lookup function.
This was not the case as in commit 0c4b391e2a9 (released in libvirt-8.3.0) the code was accidentally converted to use 'VIR_LOCK_GUARD' which auto-unlocked it when leaving the scope, even when the code was originally "leaking" the lock.
Revert the corresponding conversion and add a comment that this function is intentionally leaking a locked object.
Fixes: 0c4b391e2a9 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2221851 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/virstorageobj.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa