Re: [libvirt PATCH 09/11] storage: Statically initialize mutex
On Thu, 2022-02-17 at 13:15 +0000, Daniel P. Berrangé wrote:
On Thu, Feb 17, 2022 at 02:01:01PM +0100, Tim Wiederhake wrote:
Really should be giving an explanation of why this change
is required.
Added locally to commit message:
Making the mutex static and independent of the lifetime of the
driver object allows for simplification of mutex handling during
the object's initialization and cleanup using VIR_LOCK_GUARD's
unlock-on-scope-exit behavior.
Regards,
Tim
> Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
> ---
> src/conf/virstorageobj.h | 2 --
> src/storage/storage_driver.c | 11 ++++-------
> 2 files changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
> index 523bdec244..ad6005f153 100644
> --- a/src/conf/virstorageobj.h
> +++ b/src/conf/virstorageobj.h
> @@ -31,8 +31,6 @@ typedef struct _virStoragePoolObjList
> virStoragePoolObjList;
>
> typedef struct _virStorageDriverState virStorageDriverState;
> struct _virStorageDriverState {
> - virMutex lock;
> -
> /* pid file FD, ensures two copies of the driver can't use the
> same root */
> int lockFD;
>
> diff --git a/src/storage/storage_driver.c
> b/src/storage/storage_driver.c
> index 97e0d9b3a0..05675a5539 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -57,6 +57,8 @@ VIR_LOG_INIT("storage.storage_driver");
>
> static virStorageDriverState *driver;
>
> +static virMutex mutex = VIR_MUTEX_INITIALIZER;
> +
> static int storageStateCleanup(void);
>
> typedef struct _virStorageVolStreamInfo virStorageVolStreamInfo;
> @@ -67,11 +69,11 @@ struct _virStorageVolStreamInfo {
>
> static void storageDriverLock(void)
> {
> - virMutexLock(&driver->lock);
> + virMutexLock(&mutex);
> }
> static void storageDriverUnlock(void)
> {
> - virMutexUnlock(&driver->lock);
> + virMutexUnlock(&mutex);
> }
>
>
> @@ -270,10 +272,6 @@ storageStateInitialize(bool privileged,
> driver = g_new0(virStorageDriverState, 1);
>
> driver->lockFD = -1;
> - if (virMutexInit(&driver->lock) < 0) {
> - VIR_FREE(driver);
> - return VIR_DRV_STATE_INIT_ERROR;
> - }
> storageDriverLock();
>
> if (!(driver->pools = virStoragePoolObjListNew()))
> @@ -392,7 +390,6 @@ storageStateCleanup(void)
> VIR_FREE(driver->autostartDir);
> VIR_FREE(driver->stateDir);
> storageDriverUnlock();
> - virMutexDestroy(&driver->lock);
> VIR_FREE(driver);
>
> return 0;
> --
> 2.31.1
>
Regards,
Daniel