[v1] virnodedeviceobj: Don't unlock virNodeDeviceObj in virNodeDeviceObjListRemove()

[PATCH] virnodedeviceobj: Don't unlock virNodeDeviceObj in virNodeDeviceObjListRemove()

Michal Privoznik posted 1 patch 2 years, 3 months ago

Diff against v2
Download mbox

Test syntax-check failed

Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/5b3bad6f05ddc46ed5c808677aa34b2e22b7af8c.1643726626.git.mprivozn@redhat.com

There is a newer version of this series

src/conf/virnodedeviceobj.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Expand all Fold all

[PATCH] virnodedeviceobj: Don't unlock virNodeDeviceObj in virNodeDeviceObjListRemove()

Posted by Michal Privoznik 2 years, 3 months ago

When virNodeDeviceObjListRemove() is called, the passed
virNodeDeviceObj is removed from internal list of node devices
and then unrefed and unlocked. While the former is warranted (the
object was refed at the beginning of the function) the unlock is
not. In fact, it's wrong from conceptual POV. We still want
threads working on the object tu mutually exclude each other.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/conf/virnodedeviceobj.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
index 2e4ef2df3c..7a560349d4 100644
--- a/src/conf/virnodedeviceobj.c
+++ b/src/conf/virnodedeviceobj.c
@@ -524,7 +524,7 @@ virNodeDeviceObjListRemove(virNodeDeviceObjList *devs,
     virObjectRWLockWrite(devs);
     virObjectLock(obj);
     virNodeDeviceObjListRemoveLocked(devs, obj);
-    virNodeDeviceObjEndAPI(&obj);
+    virObjectUnref(obj);
     virObjectRWUnlock(devs);
 }
 
-- 
2.34.1

Re: [PATCH] virnodedeviceobj: Don't unlock virNodeDeviceObj in virNodeDeviceObjListRemove()

Posted by Michal Prívozník 2 years, 3 months ago

On 2/1/22 15:43, Michal Privoznik wrote:
> When virNodeDeviceObjListRemove() is called, the passed
> virNodeDeviceObj is removed from internal list of node devices
> and then unrefed and unlocked. While the former is warranted (the
> object was refed at the beginning of the function) the unlock is
> not. In fact, it's wrong from conceptual POV. We still want
> threads working on the object tu mutually exclude each other.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/conf/virnodedeviceobj.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
> index 2e4ef2df3c..7a560349d4 100644
> --- a/src/conf/virnodedeviceobj.c
> +++ b/src/conf/virnodedeviceobj.c
> @@ -524,7 +524,7 @@ virNodeDeviceObjListRemove(virNodeDeviceObjList *devs,
>      virObjectRWLockWrite(devs);
>      virObjectLock(obj);
>      virNodeDeviceObjListRemoveLocked(devs, obj);
> -    virNodeDeviceObjEndAPI(&obj);
> +    virObjectUnref(obj);
>      virObjectRWUnlock(devs);
>  }
>  

Self-NAK, I need to squash in two more bits.

Michal