On 01/10/2012 10:15 AM, Peter Krempa wrote:
On 01/09/2012 08:03 PM, Eric Blake wrote:
When disk snapshots were first implemented, libvirt blindly refused to allow an external snapshot destination that already exists, since qemu will blindly overwrite the contents of that file during the snapshot_blkdev monitor command, and we don't like a default of data loss by default. But VDSM has a scenario where NFS permissions are intentionally set so that the destination file can only be created by the management machine, and not the machine where the guest is running, so that libvirt will necessarily see the destination file already existing; adding a flag will allow VDSM to force the file reuse without libvirt complaining of possible data loss.
+ VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT = (1<< 5), /* reuse any existing + external files */ } virDomainSnapshotCreateFlags;
You added a new flag here
/* Take a snapshot of the current VM state */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1a7e816..9765a69 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10162,11 +10163,14 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
--- SNIP from top of this func. ------ virCheckFlags(VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE | VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT | VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA | VIR_DOMAIN_SNAPSHOT_CREATE_HALT | VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY, NULL); ----- </snip> -----------
But you don't check for the new flag here.
Oops - shows that I posted before testing. Now fixed.
Apart from that I think the code looks sane. ACK with that flag check fixed.
Thanks; pushed (after testing, this time). -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org