[libvirt] [PATCH] blockcopy: check dst = identical device

[Chun Yan Liu]

>>> On 7/31/2014 at 05:52 PM, in message <53DA11DB.FE6 : 102 : 21807>, Chun Yan Liu
wrote: 

>  
>>>> On 7/31/2014 at 11:35 AM, in message <53D9B993.4040806 at redhat.com>, Eric Blake 
> <eblake at redhat.com> wrote:  
> > On 07/30/2014 09:29 PM, Chun Yan Liu wrote:  
>>   
>> >> A better idea would be to rely on the volume lease manager - obtaining a   
>> >> lease should be impossible for an image already in use (and should even   
>> >> cover the case of copying 'base <- active' onto 'base', which your   
>> >> equality test wouldn't catch). I'm not sure why the lease manager is not   
>> >> already flagging this issue - are we still using the nop lease manager   
>> >> by default, and would the fcntl or sanlock lease manager do a better job?  
>> >   
>> > Besides the default lock is 'nop', currently lock manager is only used in:  
>> > VM start/stop and attach/detach disk, blockcopy not using it.  
>>   
>> But that's not true - the code IS trying to use it.  qemuDomainBlockCopy  
>> calls qemuDomainPrepareDiskChainElement(), which calls  
>> virDomainLockImageAttach(), and that should be the use of the lock  
>> manager.  Can you debug why it is not working when using something other  
>> than the 'nop' manager?  
>  
> Update: 
> 1. identical device with exactly the same spelled name. e.g. source is  
>     /dev/sda4, dest is /dev/sda4: 
>     * 'lockd' manager is working well with --reuse-external. 
>     * my previous testing result which makes the VM cannot be started  
>        successfully after doing blockcopy to same device and shutdown
>        VM and start VM again, because I didn't add --reuse-external
>        option. Then in code, it  considers the dest as a new created file,
>         when error happens, it unlinks the dest, which s actually 
>        also my source disk. 
> 2. identical device with not exactly the same spelling. e.g. source is  
>     /dev/sda4, dest is /dev///sda4, or a softlink: 
>     'lockd' manager is not working. Both hashtable check and fcntl can't  
>      give right result. 
>     * HashTable checks the resource in the locked list depending on the  
>        name. If name is not exactly the same, it will treat as 'not found in  
>        locked list'. 
>     * In fcntl stage, since fcntl is to one process, in the same process, do  
>        two times fcntl F_SETLK, both will succeed. Since everytime it's
>        virtlockd tries fcntl, so in this case, 1st time fcntl /dev/sda4, it
>        succeeds; 2nd time fcntl /dev///sda4, also succeeds. 
>        Not as we expected 'cannot get the lock'. 

About the 2nd point, 'lockd' manager cannot handle identical device but
different name (extra / in name or softlink), I didn't think of a quick fix.
Maybe still better to check src and dst name in earlier stage. Could call
realpath() to cover more cases. Any other ideas?

>>   
>> > To use it in blockcopy, maybe can refer to attach/detach disk: before doing  
>> > blockcopy, try AcquireResource; after blockcopy finish, try   
>> releaseResource.  
>>   
>> That should be what is already happening.  
>>   
>>   
>> --   
>> Eric Blake   eblake redhat com    +1-919-301-3266  
>> Libvirt virtualization library http://libvirt.org  
>>   
>>   
>