[libvirt] [PATCH v3 18/28] lock_manager: Allow disabling configFile for virLockManagerPluginNew

[John Ferlan]

On 08/27/2018 04:08 AM, Michal Privoznik wrote:
> In some cases we might want to not load the lock driver config.
> Alter virLockManagerPluginNew() and the lock drivers to cope with
> this fact.

No current cases, but sometime in the future the requirement that a
configFile exists will be removed.... Waiting with baited fingers to see
how it's replaced ;-)

> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/locking/lock_driver_lockd.c   | 4 +++-
>  src/locking/lock_driver_sanlock.c | 4 +++-
>  src/locking/lock_manager.c        | 8 ++++++--
>  3 files changed, 12 insertions(+), 4 deletions(-)
> 

I note that virLockDriverInit in src/locking/lock_driver.h doesn't even
document @configFile (or seem to care if it was NULL). In any a
modification there to describe the argument should be added.

> diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
> index aec768b0df..c0598e6987 100644
> --- a/src/locking/lock_driver_lockd.c
> +++ b/src/locking/lock_driver_lockd.c
> @@ -370,8 +370,10 @@ static int virLockManagerLockDaemonInit(unsigned int version,
>      driver->requireLeaseForDisks = true;
>      driver->autoDiskLease = true;
>  
> -    if (virLockManagerLockDaemonLoadConfig(configFile) < 0)
> +    if (configFile &&
> +        virLockManagerLockDaemonLoadConfig(configFile) < 0) {
>          goto error;
> +    }
>  
>      if (driver->autoDiskLease) {
>          if (driver->fileLockSpaceDir &&
> diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c
> index 9393e7d9a2..66953c70d5 100644
> --- a/src/locking/lock_driver_sanlock.c
> +++ b/src/locking/lock_driver_sanlock.c
> @@ -450,8 +450,10 @@ static int virLockManagerSanlockInit(unsigned int version,
>          goto error;
>      }
>  
> -    if (virLockManagerSanlockLoadConfig(driver, configFile) < 0)
> +    if (configFile &&
> +        virLockManagerSanlockLoadConfig(driver, configFile) < 0) {
>          goto error;
> +    }
>  
>      if (driver->autoDiskLease && !driver->hostID) {
>          virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> diff --git a/src/locking/lock_manager.c b/src/locking/lock_manager.c
> index 30a0fd996e..c2ff7afb70 100644
> --- a/src/locking/lock_manager.c
> +++ b/src/locking/lock_manager.c
> @@ -105,6 +105,8 @@ static void virLockManagerLogParams(size_t nparams,
>  /**
>   * virLockManagerPluginNew:
>   * @name: the name of the plugin
> + * @driverName: the hypervisor driver that loads the plugin
> + * @configDir: path to dir where config files are stored
>   * @flag: optional plugin flags
>   *
>   * Attempt to load the plugin $(libdir)/libvirt/lock-driver/@name.so
> @@ -132,9 +134,11 @@ virLockManagerPluginPtr virLockManagerPluginNew(const char *name,
>      VIR_DEBUG("name=%s driverName=%s configDir=%s flags=0x%x",
>                name, driverName, configDir, flags);

You'll need to add NULLSTR around @driverName and @configDir then,
right?  If they can be NULL?

With those adjustments,

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

John

>  
> -    if (virAsprintf(&configFile, "%s/%s-%s.conf",
> -                    configDir, driverName, name) < 0)
> +    if (driverName && configDir &&
> +        virAsprintf(&configFile, "%s/%s-%s.conf",
> +                    configDir, driverName, name) < 0) {
>          return NULL;
> +    }
>  
>      if (STREQ(name, "nop")) {
>          driver = &virLockDriverNop;
>