[libvirt] [PATCH] Fix release of resources with lockd plugin
From: "Daniel P. Berrange" <berrange@redhat.com> The lockd plugin for the lock manager was not correctly handling the release of resource locks. This meant that during migration, or when pausing a VM, the locks would not get released. This in turn made it impossible to resume the domain, or finish migration --- src/locking/lock_driver_lockd.c | 38 ++++++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c index 28a4f15..4d0cd74 100644 --- a/src/locking/lock_driver_lockd.c +++ b/src/locking/lock_driver_lockd.c @@ -774,10 +774,11 @@ static int virLockManagerLockDaemonRelease(virLockManagerPtr lock, virNetClientPtr client = NULL; virNetClientProgramPtr program = NULL; int counter = 0; - virLockSpaceProtocolReleaseResourceArgs args; int rv = -1; + size_t i; + virLockManagerLockDaemonPrivatePtr priv = lock->privateData; - memset(&args, 0, sizeof(args)); + virCheckFlags(0, -1); if (state) *state = NULL; @@ -785,16 +786,29 @@ static int virLockManagerLockDaemonRelease(virLockManagerPtr lock, if (!(client = virLockManagerLockDaemonConnect(lock, &program, &counter))) goto cleanup; - args.flags = flags; - - if (virNetClientProgramCall(program, - client, - counter++, - VIR_LOCK_SPACE_PROTOCOL_PROC_RELEASE_RESOURCE, - 0, NULL, NULL, NULL, - (xdrproc_t)xdr_virLockSpaceProtocolReleaseResourceArgs, &args, - (xdrproc_t)xdr_void, NULL) < 0) - goto cleanup; + for (i = 0 ; i < priv->nresources ; i++) { + virLockSpaceProtocolReleaseResourceArgs args; + + memset(&args, 0, sizeof(args)); + + if (priv->resources[i].lockspace) + args.path = priv->resources[i].lockspace; + args.name = priv->resources[i].name; + args.flags = priv->resources[i].flags; + + args.flags &= + ~(VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_SHARED | + VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_AUTOCREATE); + + if (virNetClientProgramCall(program, + client, + counter++, + VIR_LOCK_SPACE_PROTOCOL_PROC_RELEASE_RESOURCE, + 0, NULL, NULL, NULL, + (xdrproc_t)xdr_virLockSpaceProtocolReleaseResourceArgs, &args, + (xdrproc_t)xdr_void, NULL) < 0) + goto cleanup; + } rv = 0; -- 1.8.2.1
On 05/03/2013 06:15 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The lockd plugin for the lock manager was not correctly handling the release of resource locks. This meant that during migration, or when pausing a VM, the locks would not get released. This in turn made it impossible to resume the domain, or finish migration --- src/locking/lock_driver_lockd.c | 38 ++++++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 12 deletions(-)
diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
- - if (virNetClientProgramCall(program, - client, - counter++, - VIR_LOCK_SPACE_PROTOCOL_PROC_RELEASE_RESOURCE, - 0, NULL, NULL, NULL, - (xdrproc_t)xdr_virLockSpaceProtocolReleaseResourceArgs, &args, - (xdrproc_t)xdr_void, NULL) < 0)
So the old code released the first resource,
- goto cleanup; + for (i = 0 ; i < priv->nresources ; i++) {
but the new code loops over all resources. Yeah, I could see how that causes a problem if more than one resource is held. ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Fri, May 03, 2013 at 06:34:38AM -0600, Eric Blake wrote:
On 05/03/2013 06:15 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The lockd plugin for the lock manager was not correctly handling the release of resource locks. This meant that during migration, or when pausing a VM, the locks would not get released. This in turn made it impossible to resume the domain, or finish migration --- src/locking/lock_driver_lockd.c | 38 ++++++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 12 deletions(-)
diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
- - if (virNetClientProgramCall(program, - client, - counter++, - VIR_LOCK_SPACE_PROTOCOL_PROC_RELEASE_RESOURCE, - 0, NULL, NULL, NULL, - (xdrproc_t)xdr_virLockSpaceProtocolReleaseResourceArgs, &args, - (xdrproc_t)xdr_void, NULL) < 0)
So the old code released the first resource,
- goto cleanup; + for (i = 0 ; i < priv->nresources ; i++) {
but the new code loops over all resources. Yeah, I could see how that causes a problem if more than one resource is held.
Actually the old code didn't release any resources since it failed to fill in the resname field. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (2)
-
Daniel P. Berrange -
Eric Blake