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

Jim Fehlig posted 1 patch 2 years, 9 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20210630231758.22927-1-jfehlig@suse.com
src/locking/lock_daemon_dispatch.c |  5 ++---
src/locking/lock_driver_lockd.c    | 11 ++---------
2 files changed, 4 insertions(+), 12 deletions(-)
[PATCH] virtlockd: Don't report error if lockspace exists
Posted by Jim Fehlig 2 years, 9 months ago
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@suse.com>
---

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;
 
-- 
2.31.1


Re: [PATCH] virtlockd: Don't report error if lockspace exists
Posted by Daniel Henrique Barboza 2 years, 9 months ago

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@suse.com>
> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

> 
> 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;
>   
>