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.
https://bugzilla.redhat.com/show_bug.cgi?id=767104
* include/libvirt/libvirt.h.in (virDomainSnapshotCreateFlags): Add
VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT.
* src/libvirt.c (virDomainSnapshotCreateXML): Document it. Add
note about partial failure.
* tools/virsh.c (cmdSnapshotCreate, cmdSnapshotCreateAs): Add new
flag.
* tools/virsh.pod (snapshot-create, snapshot-create-as): Document
it.
* src/qemu/qemu_driver.c (qemuDomainSnapshotDiskPrepare)
(qemuDomainSnapshotCreateXML): Implement the new flag.
---
I'm not sure I have the best name for the proposed flag; alternative
suggestions are welcome.
I couldn't come up with anything better.
include/libvirt/libvirt.h.in | 2 ++
src/libvirt.c | 13 +++++++++++++
src/qemu/qemu_driver.c | 10 +++++++---
tools/virsh.c | 8 +++++++-
tools/virsh.pod | 16 +++++++++++++---
5 files changed, 42 insertions(+), 7 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index ad6fcce..0b564cf 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2990,6 +2990,8 @@ typedef enum {
after snapshot */
VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY = (1<< 4), /* disk snapshot, not
system checkpoint */
+ 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.
Running virsh with the new flag:
virsh # snapshot-create 1 ../test.xml --reuse-external
error: invalid argument: qemuDomainSnapshotCreateXML: unsupported flags
(0x20)
goto cleanup;
if (flags& VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) {
+ bool allow_reuse;
+
+ allow_reuse = (flags& VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0;
if (virDomainSnapshotAlignDisks(def,
VIR_DOMAIN_DISK_SNAPSHOT_EXTERNAL,
false)< 0)
Apart from that I think the code looks sane. ACK with that flag check fixed.
Peter
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list