[PATCH v2] virnodedeviceobj: Don't unlock virNodeDeviceObj in virNodeDeviceObjListRemove()
Daniel Henrique Barboza
danielhb413 at gmail.com
Wed Feb 2 20:16:50 UTC 2022
On 2/2/22 06:00, 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 at redhat.com>
> ---
Reviewed-by: Daniel Henrique Barboza <danielhb413 at gmail.com>
>
> v2 of:
>
> https://listman.redhat.com/archives/libvir-list/2022-February/msg00036.html
>
> diff to v1:
> - Fixed test driver which worked around broken logic
>
> Note, there is similar problem with virNodeDeviceObjListForEachRemove()
> which removes objects from the list, even without lock, but that happens
> only with mdevs and I have none to test with.
>
> src/conf/virnodedeviceobj.c | 2 +-
> src/test/test_driver.c | 4 +---
> 2 files changed, 2 insertions(+), 4 deletions(-)
>
> 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);
> }
>
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index 9e1fc65972..f900123941 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -6810,7 +6810,7 @@ testDestroyVport(testDriver *privconn,
> 0);
>
> virNodeDeviceObjListRemove(privconn->devs, obj);
> - virObjectUnref(obj);
> + virNodeDeviceObjEndAPI(&obj);
>
> virObjectEventStateQueue(privconn->eventState, event);
> return 0;
> @@ -7797,8 +7797,6 @@ testNodeDeviceDestroy(virNodeDevicePtr dev)
>
> virObjectLock(obj);
> virNodeDeviceObjListRemove(driver->devs, obj);
> - virObjectUnref(obj);
> - obj = NULL;
>
> cleanup:
> virNodeDeviceObjEndAPI(&obj);
More information about the libvir-list
mailing list