Re: [PATCH] virtlockd: Don't report error if lockspace exists

[Daniel Henrique Barboza <danielhb413@xxxxxxxxx>]

On 6/30/21 8:17 PM, Jim Fehlig wrote:
> When the qemu or libxl driver is configured to use lockd and
> file_lockspace_dir is set, virtlockd emits an error when libvirtd
> is retarted
>
> May 25 15:44:31 virt81 virtlockd[7723]: Requested operation is not
> valid: Lockspace for path /data/libvirtd/lockspace already exists
>
> There is really no need to fail when the lockspace already exists,
> paricularly since the user is expected to create the lockspace
> specified in file_lockspace_dir. Failure to do so will prevent
> starting any domains
>
> virsh start test
> error: Failed to start domain 'test'
> error: Unable to open/create resource /data/libvirtd/lockspace/de22c4bf931e7c48b49e8ca64b477d44e78a51543e534df488b05ccd08ec5caa: No such file or directory
>
> Also, virLockManagerLockDaemonSetupLockspace already has logic to ignore
> the error. Since callers are not interested in the error, change
> virtlockd to not report or return an error when the specified lockspace
> already exists.
>
> Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@xxxxxxxxx>

> In response to my earlier query about this problem
>
> https://listman.redhat.com/archives/libvir-list/2021-May/msg00872.html
>
>   src/locking/lock_daemon_dispatch.c |  5 ++---
>   src/locking/lock_driver_lockd.c    | 11 ++---------
>   2 files changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/src/locking/lock_daemon_dispatch.c b/src/locking/lock_daemon_dispatch.c
> index e8b9832453..13e688f2e2 100644
> --- a/src/locking/lock_daemon_dispatch.c
> +++ b/src/locking/lock_daemon_dispatch.c
> @@ -405,9 +405,8 @@ virLockSpaceProtocolDispatchCreateLockSpace(virNetServer *server G_GNUC_UNUSED,
>       }
>   
>       if (virLockDaemonFindLockSpace(lockDaemon, args->path) != NULL) {
> -        virReportError(VIR_ERR_OPERATION_INVALID,
> -                       _("Lockspace for path %s already exists"),
> -                       args->path);
> +        VIR_DEBUG("Lockspace for path %s already exists", args->path);
> +        rv = 0;
>           goto cleanup;
>       }
>   
> diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
> index 3a7386af30..87afdbfb78 100644
> --- a/src/locking/lock_driver_lockd.c
> +++ b/src/locking/lock_driver_lockd.c
> @@ -281,15 +281,8 @@ static int virLockManagerLockDaemonSetupLockspace(const char *path)
>                                   VIR_LOCK_SPACE_PROTOCOL_PROC_CREATE_LOCKSPACE,
>                                   0, NULL, NULL, NULL,
>                                   (xdrproc_t)xdr_virLockSpaceProtocolCreateLockSpaceArgs, (char*)&args,
> -                                (xdrproc_t)xdr_void, NULL) < 0) {
> -        if (virGetLastErrorCode() == VIR_ERR_OPERATION_INVALID) {
> -            /* The lockspace already exists */
> -            virResetLastError();
> -            rv = 0;
> -        } else {
> -            goto cleanup;
> -        }
> -    }
> +                                (xdrproc_t)xdr_void, NULL) < 0)
> +        goto cleanup;
>   
>       rv = 0;
>