On 7/29/2014 at 11:47 PM, in message <53D7C1F6.4030705@redhat.com>, Eric Blake <eblake@redhat.com> wrote: 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@suse.com> --- 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?
According to code, it's still using 'nop' by default. Code: if (!(qemu_driver->lockManager = virLockManagerPluginNew(cfg->lockManagerName ? cfg->lockManagerName : "nop", "qemu", cfg->configBaseDir, 0))) goto error; Saw Daniel's virlockd patch series, the last patch: "Change the default QEMU lock manager to 'lockd'" is ACKed. But this patch is not committed. All other patches in that series have been committed. http://www.redhat.com/archives/libvir-list/2012-December/msg00734.html
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org