
On Fri, Jun 01, 2012 at 10:46:04AM +0200, Frido Roose wrote:
On Thursday 31 May 2012 at 10:44, Frido Roose wrote:
Hello,
I logged a bug about using virsh detach-disk cleaning up all sanlock resources for the domain instead of only the device in question.
After a quick look into the code, I think a new method similar to virLockManagerSanlockAddResource is needed in case of detaching a disk from the domain, like e.g. virLockManagerSanlockDelResource (…).
Now it looks like virLockManagerSanlockRelease is called, which releases all resources: if ((rv = sanlock_release(-1, priv->vm_pid, SANLK_REL_ALL, 0, NULL)) < 0) {
virsh detach-disk should then call virLockManagerSanlockDelResource for the given resource imo.
You are a little mixed up with the sanlock API naming here vs the hotplug hotunplug actions. The way sanlock works is that you do - Connect to sanlock - Register all the disks that will be operated on using virLockManagerSanlockAddResource - Invoke the acquire/inquire/release actions on sanlock In other words, using 'AddResource' is correct, even when releasing a resource/lease. The problem is that the usage of SANLK_REL_ALL is wrong. We should be using the res_args + res_count fields to pass the explicit list of disks, instead of releasing all of them. 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 :|