Bug 826704

Summary: virsh detach-disk cleans up all sanlock resources for the domain pid instead of specified disk device only
Product: [Community] Virtualization Tools Reporter: Frido Roose <fr.roose>
Component: libvirtAssignee: Libvirt Maintainers <libvirt-maint>
Status: CLOSED NEXTRELEASE QA Contact:
Severity: medium Docs Contact:
Priority: unspecified    
Version: unspecifiedCC: bart, dyasny, eblake, fr.roose
Target Milestone: ---   
Target Release: ---   
Hardware: x86_64   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-08-13 16:36:17 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
Only release specific sanlock resource when hot-dettaching a disk none

Description Frido Roose 2012-05-30 19:49:24 UTC 
Description of problem:
virsh detach-disk cleans up all sanlock resources for the kvm domain

Version-Release number of selected component (if applicable):
sanlock-1.8-2.1.el6.x86_64
libvirt-lock-sanlock-0.9.4-23.el6_2.7.x86_64

How reproducible:
Always, using the steps below

Steps to Reproduce:
1. virsh start <domain>
2. sanlock client status | grep <pid>
3. virsh attach-disk <domain> /dev/disk/by-id/wwn-0xxx... vdb --persistent
4. sanlock client status
=> new resource is registered within lock space __LIBVIRT__DISKS__
5. virsh detach-disk <domain> vdb --persistent
6. sanlock client status
  
Actual results:
All sanlock resources (like the boot disk device and others) are also removed from the lockspace, as reported with sanlock client status

Expected results:
Only the detached device should be released from the lockspace for the pid of the domain

Additional info:

# virsh domblklist vmor01
Target Source
------------------------------------------------
vda /var/lib/libvirt/images/vmor01.img
vdb /dev/disk/by-id/wwn-0x600a0b80002695880000cc4c4f865710
vdc /dev/disk/by-id/wwn-0x600a0b80002695880000cc514f865a4d

# sanlock client status
daemon 5c8dbdb2-41b2-431a-a2e6-8384dc9cf476.pan
p -1 listener
p 22761 vmor01
p -1 status
s __LIBVIRT__DISKS__:14:/var/lib/libvirt/sanlock/__LIBVIRT__DISKS__:0
r __LIBVIRT__DISKS__:d540f8af975019b0d41c4c65e4955072:/var/lib/libvirt/sanlock/d540f8af975019b0d41c4c65e4955072:0:1 p 22761
r __LIBVIRT__DISKS__:571a282fc5e8cbb42167ce9ec1d41a95:/var/lib/libvirt/sanlock/571a282fc5e8cbb42167ce9ec1d41a95:0:1 p 22761
r __LIBVIRT__DISKS__:8a078688f5beec6983baef51d64453de:/var/lib/libvirt/sanlock/8a078688f5beec6983baef51d64453de:0:1 p 22761

# virsh detach-disk vmor01 vdb --persistent
Disk detached successfully

# virsh domblklist vmor01
Target Source
------------------------------------------------
vda /var/lib/libvirt/images/vmor01.img
vdc /dev/disk/by-id/wwn-0x600a0b80002695880000cc514f865a4d

# sanlock client status
daemon 5c8dbdb2-41b2-431a-a2e6-8384dc9cf476.pan
p -1 listener
p 22761 vmor01
p -1 status
s __LIBVIRT__DISKS__:14:/var/lib/libvirt/sanlock/__LIBVIRT__DISKS__:0
Comment 1 Frido Roose 2012-05-31 10:18:57 UTC 
Some more debug output while virsh detach-disk:
log_filters="1:locking"
log_outputs="1:file:/var/log/libvirt/libvirtd-debug.log"

2012-05-31 10:03:39.029+0000: 1728: debug : virDomainLockManagerNew:123 : plugin=0x7f55f8014500 dom=0x7f55f80102b0 withResources=0
2012-05-31 10:03:39.029+0000: 1728: debug : virLockManagerNew:291 : plugin=0x7f55f8014500 type=0 nparams=4 params=0x7f56056bd8b0 flags=0
2012-05-31 10:03:39.029+0000: 1728: debug : virLockManagerLogParams:98 :   key=uuid type=uuid value=c0ee1dfa-4056-f3f9-9f8c-f10e974b59f0
2012-05-31 10:03:39.029+0000: 1728: debug : virLockManagerLogParams:94 :   key=name type=string value=vmor01
2012-05-31 10:03:39.029+0000: 1728: debug : virLockManagerLogParams:82 :   key=id type=uint value=3
2012-05-31 10:03:39.029+0000: 1728: debug : virLockManagerLogParams:82 :   key=pid type=uint value=24861
2012-05-31 10:03:39.029+0000: 1728: debug : virDomainLockManagerAddDisk:86 : Add disk /dev/disk/by-id/wwn-0x600a0b80002695880000cc4c4f865710
2012-05-31 10:03:39.029+0000: 1728: debug : virLockManagerAddResource:320 : lock=0x7f55f00d8fb0 type=0 name=/dev/disk/by-id/wwn-0x600a0b80002695880000cc4c4f865710 nparams=0 params=(nil) flags=0
2012-05-31 10:03:39.029+0000: 1728: debug : virLockManagerRelease:352 : lock=0x7f55f00d8fb0 state=(nil) flags=0
2012-05-31 10:03:39.059+0000: 1728: debug : virLockManagerFree:374 : lock=0x7f55f00d8fb0

In virLockManagerSanlockRelease:
sanlock_release(-1, priv->vm_pid, SANLK_REL_ALL, 0, NULL)

Only the specified resource should be released.
Comment 2 Frido Roose 2012-07-12 07:54:40 UTC 
Created attachment 597726 [details]
Only release specific sanlock resource when hot-dettaching a disk

Call sanlock_release() with the specific resource instead of always passing the SANLK_REL_ALL flag.
Comment 3 Frido Roose 2012-07-12 08:00:59 UTC 
Comment on attachment 597726 [details]
Only release specific sanlock resource when hot-dettaching a disk

Call sanlock_release() with the specific resource instead of always passing the
SANLK_REL_ALL flag.
Comment 4 Eric Blake 2012-08-13 16:36:17 UTC 
Will be in the 0.10.0 release.

commit a58cb1ac40b1af53af881c3d1901240cc07853bc
Author: Frido Roose <frido.roose>
Date:   Mon Aug 13 12:54:38 2012 +0200

    Release only specific sanlock resource
    
    This is a patch for bug 826704
    All sanlock resources get released when hot-dettaching a disk from the domain   because virLockManagerSanlockRelease uses the wrong function parameters/flags.  With the patch only the resources that should be released are cleaned up.
    
    Signed-off-by: Frido Roose <frido.roose>
Comment 5 Bart Verwilst 2012-10-09 21:29:22 UTC 
Is this something that can be backported to RHEL 6.3? Since this effectively makes the cluster able to mount blockdevices multiple times, data corruption can become a factor..