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

2015-02-12 Thread Shanzhi Yu 
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 s...@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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2015-02-12 Thread Daniel P. Berrange 
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 s...@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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2015-02-12 Thread Shanzhi Yu 


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 s...@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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list