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@xxxxxxxxxx +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list