Re: [libvirt] [PATCH] snapshot: allow reuse of existing files in disk snapshot

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.

Apart from that I think the code looks sane. ACK with that flag check fixed.