[libvirt] [PATCH] util: make use of VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE in virLockSpaceAcquireResource

When create external disk snapshot with virtlock enabled, libvirtd will hang if flag VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE is missed in virLockSpaceAcquireResource. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1191901 Signed-off-by: Shanzhi Yu <shyu@redhat.com> --- src/util/virlockspace.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c index 2366a74..25b4433 100644 --- a/src/util/virlockspace.c +++ b/src/util/virlockspace.c @@ -626,8 +626,10 @@ int virLockSpaceAcquireResource(virLockSpacePtr lockspace, virMutexLock(&lockspace->lock); if ((res = virHashLookup(lockspace->resources, resname))) { - if ((res->flags & VIR_LOCK_SPACE_ACQUIRE_SHARED) && - (flags & VIR_LOCK_SPACE_ACQUIRE_SHARED)) { + if (((res->flags & VIR_LOCK_SPACE_ACQUIRE_SHARED) && + (flags & VIR_LOCK_SPACE_ACQUIRE_SHARED)) || + ((res->flags & VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE) && + (flags & VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE))){ if (VIR_EXPAND_N(res->owners, res->nOwners, 1) < 0) goto cleanup; -- 1.8.3.1

On Thu, Feb 12, 2015 at 07:12:57PM +0800, Shanzhi Yu wrote:
When create external disk snapshot with virtlock enabled, libvirtd will hang if flag VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE is missed in virLockSpaceAcquireResource.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1191901 Signed-off-by: Shanzhi Yu <shyu@redhat.com> --- src/util/virlockspace.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c index 2366a74..25b4433 100644 --- a/src/util/virlockspace.c +++ b/src/util/virlockspace.c @@ -626,8 +626,10 @@ int virLockSpaceAcquireResource(virLockSpacePtr lockspace, virMutexLock(&lockspace->lock);
if ((res = virHashLookup(lockspace->resources, resname))) { - if ((res->flags & VIR_LOCK_SPACE_ACQUIRE_SHARED) && - (flags & VIR_LOCK_SPACE_ACQUIRE_SHARED)) { + if (((res->flags & VIR_LOCK_SPACE_ACQUIRE_SHARED) && + (flags & VIR_LOCK_SPACE_ACQUIRE_SHARED)) || + ((res->flags & VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE) && + (flags & VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE))){
No, this is wrong. If virHashLookup returns a non-NULL entry it indicates that the lock is already held. It is not valid to ignore this error if 'autocreate' flag is set, as that would allow multiple VMs to own the same disk which is exactly the scenario we are trying to prevent. Regards, 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 :|

On 02/12/2015 07:17 PM, Daniel P. Berrange wrote:
On Thu, Feb 12, 2015 at 07:12:57PM +0800, Shanzhi Yu wrote:
When create external disk snapshot with virtlock enabled, libvirtd will hang if flag VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE is missed in virLockSpaceAcquireResource.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1191901 Signed-off-by: Shanzhi Yu <shyu@redhat.com> --- src/util/virlockspace.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c index 2366a74..25b4433 100644 --- a/src/util/virlockspace.c +++ b/src/util/virlockspace.c @@ -626,8 +626,10 @@ int virLockSpaceAcquireResource(virLockSpacePtr lockspace, virMutexLock(&lockspace->lock);
if ((res = virHashLookup(lockspace->resources, resname))) { - if ((res->flags & VIR_LOCK_SPACE_ACQUIRE_SHARED) && - (flags & VIR_LOCK_SPACE_ACQUIRE_SHARED)) { + if (((res->flags & VIR_LOCK_SPACE_ACQUIRE_SHARED) && + (flags & VIR_LOCK_SPACE_ACQUIRE_SHARED)) || + ((res->flags & VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE) && + (flags & VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE))){ No, this is wrong. If virHashLookup returns a non-NULL entry it indicates that the lock is already held. It is not valid to ignore this error if 'autocreate' flag is set, as that would allow multiple VMs to own the same disk which is exactly the scenario we are trying to prevent.
Yes. It's really wrong way. Please ignore this.
Regards, Daniel
-- Regards shyu
participants (2)
-
Daniel P. Berrange
-
Shanzhi Yu