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

>> On 7/29/2014 at 11:47 PM, in message <53D7C1F6.4030705(a)redhat.com&gt;, Eric Blake

On 07/29/2014 07:37 AM, Eric Blake wrote: > On 07/29/2014 03:59 AM, Chunyan Liu wrote: >> Check whether dst is the same device as source, if yes, report >> error and exit. >> >> Currently if dst is the same device as source, blockcopy is still >> going and qemu 'drive-mirror' is executed. Considering that: >> a). blockcopy to the same device is meaningless. b.) result is >> unexpected. (tested with block device whose source path is /dev/sdaX, >> after blockcopy, shutdown VM and then create VM from xml again, the >> VM cannot be started.) This case should not be allowed. >> >> Signed-off-by: Chunyan Liu <cyliu(a)suse.com&gt; >> --- >> src/qemu/qemu_driver.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c >> index 704ba39..87a3790 100644 >> --- a/src/qemu/qemu_driver.c >> +++ b/src/qemu/qemu_driver.c >> @@ -15309,6 +15309,13 @@ qemuDomainBlockCopy(virDomainObjPtr vm, >> } >> >> /* Prepare the destination file. */ >> + if (STREQ(disk->src->path, dest)) { > > STREQ is too weak (consider symlinks, or even "a/b" vs. "a//b"). It > will catch some cases, but not all. > > I don't know that we can reliably detect all possible ways the user can > shoot themselves in the foot, so I'm not sure this patch is a good idea. 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?

-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org