[libvirt] [PATCH v3 15/28] locking: Introduce virLockManagerClearResources

[John Ferlan]

On 08/27/2018 04:08 AM, Michal Privoznik wrote:
> This is a counterpart to virLockManagerAddResource. It is going
> to be handy when using one lock manager to lock multiple files
> step by step.

OK, sure, but knowing what the purpose is now would perhaps be more a
more useful commit message.

> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/libvirt_private.syms        |  1 +
>  src/locking/lock_driver.h       |  5 +++++
>  src/locking/lock_driver_lockd.c | 27 ++++++++++++++++++++++++++-
>  src/locking/lock_driver_nop.c   |  6 ++++++
>  src/locking/lock_manager.c      | 12 ++++++++++++
>  src/locking/lock_manager.h      |  3 +++
>  6 files changed, 53 insertions(+), 1 deletion(-)
> 

Not supported for lock_driver_sanlock.c... It doesn't seem it would be
difficult to handle, but yeah, that's not our problem and if we're
calling it old technology, then I'm fine with not changing.

> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 47ea35f864..42f15f117e 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1293,6 +1293,7 @@ virDomainLockProcessStart;
>  # locking/lock_manager.h
>  virLockManagerAcquire;
>  virLockManagerAddResource;
> +virLockManagerClearResources;
>  virLockManagerFree;
>  virLockManagerInquire;
>  virLockManagerNew;
> diff --git a/src/locking/lock_driver.h b/src/locking/lock_driver.h
> index 9be0abcfba..59c4c3aac7 100644
> --- a/src/locking/lock_driver.h
> +++ b/src/locking/lock_driver.h
> @@ -228,6 +228,10 @@ typedef int (*virLockDriverAddResource)(virLockManagerPtr man,
>                                          virLockManagerParamPtr params,
>                                          unsigned int flags);
>  
> +

No verbose documentation on this?  The others have it and this needs it.

> +typedef int (*virLockDriverClearResource)(virLockManagerPtr mgr,
> +                                          unsigned int flags);
> +
>  /**
>   * virLockDriverAcquire:
>   * @manager: the lock manager context
> @@ -313,6 +317,7 @@ struct _virLockDriver {
>      virLockDriverFree drvFree;
>  
>      virLockDriverAddResource drvAddResource;
> +    virLockDriverClearResource drvClearResources;
>  
>      virLockDriverAcquire drvAcquire;
>      virLockDriverRelease drvRelease;
> diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
> index d7cb183d7a..4883e89ac6 100644
> --- a/src/locking/lock_driver_lockd.c
> +++ b/src/locking/lock_driver_lockd.c
> @@ -402,8 +402,9 @@ static int virLockManagerLockDaemonDeinit(void)
>      return 0;
>  }
>  
> +
>  static void
> -virLockManagerLockDaemonPrivateFree(virLockManagerLockDaemonPrivatePtr priv)
> +virLockManagerLockDaemonFreeResources(virLockManagerLockDaemonPrivatePtr priv)
>  {
>      size_t i;
>  
> @@ -415,6 +416,17 @@ virLockManagerLockDaemonPrivateFree(virLockManagerLockDaemonPrivatePtr priv)
>          VIR_FREE(priv->resources[i].name);
>      }
>      VIR_FREE(priv->resources);
> +    priv->nresources = 0;
> +}
> +
> +
> +static void
> +virLockManagerLockDaemonPrivateFree(virLockManagerLockDaemonPrivatePtr priv)
> +{
> +    if (!priv)
> +        return;
> +
> +    virLockManagerLockDaemonFreeResources(priv);


Theoretically could have been done in a separate patch, but then there's
this thing about overkill...;-)

>  
>      switch (priv->type) {
>      case VIR_LOCK_MANAGER_OBJECT_TYPE_DOMAIN:
> @@ -733,6 +745,18 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
>  }
>  
>  
> +static int virLockManagerLockDaemonClearResources(virLockManagerPtr lock,
> +                                                  unsigned int flags)

static int
virLock...

Consistency with new API's vs. consistency with existing API's... Tough
one not to say something about.

With suggested amendments,

Reviewed-by: John Ferlan <jferlan at redhat.com>

John

[...]