[libvirt] [PATCHv9] snapshot: sanity check when reusing file for snapshot

The snapshot code when reusing an existing file had hard-to-read logic, as well as a missing sanity check: REUSE_EXT should require the destination to already be present. * src/qemu/qemu_driver.c (qemuDomainSnapshotDiskPrepare): Require destination on REUSE_EXT, rename variable for legibility. --- v9: separate out from larger series, as this one is ready now; provide diff with more context and a renamed variable. https://www.redhat.com/archives/libvir-list/2012-October/msg01107.html src/qemu/qemu_driver.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d1dc2b8..8af316f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10702,18 +10702,18 @@ qemuDomainSnapshotDiskPrepare(virDomainObjPtr vm, virDomainSnapshotDefPtr def, { int ret = -1; int i; bool found = false; bool active = virDomainObjIsActive(vm); struct stat st; - bool allow_reuse = (*flags & VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0; + bool reuse = (*flags & VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0; bool atomic = (*flags & VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC) != 0; int external = 0; qemuDomainObjPrivatePtr priv = vm->privateData; - if (allow_reuse && !qemuCapsGet(priv->caps, QEMU_CAPS_TRANSACTION)) { + if (reuse && !qemuCapsGet(priv->caps, QEMU_CAPS_TRANSACTION)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("reuse is not supported with this QEMU binary")); goto cleanup; } for (i = 0; i < def->ndisks; i++) { @@ -10756,14 +10756,19 @@ qemuDomainSnapshotDiskPrepare(virDomainObjPtr vm, virDomainSnapshotDefPtr def, if (stat(disk->file, &st) < 0) { if (errno != ENOENT) { virReportSystemError(errno, _("unable to stat for disk %s: %s"), disk->name, disk->file); goto cleanup; + } else if (reuse) { + virReportSystemError(errno, + _("missing existing file for disk %s: %s"), + disk->name, disk->file); + goto cleanup; } - } else if (!(S_ISBLK(st.st_mode) || !st.st_size || allow_reuse)) { + } else if (!S_ISBLK(st.st_mode) && st.st_size && !reuse) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("external snapshot file for disk %s already " "exists and is not a block device: %s"), disk->name, disk->file); goto cleanup; } -- 1.7.11.7

On 10/22/12 22:34, Eric Blake wrote:
The snapshot code when reusing an existing file had hard-to-read logic, as well as a missing sanity check: REUSE_EXT should require the destination to already be present.
* src/qemu/qemu_driver.c (qemuDomainSnapshotDiskPrepare): Require destination on REUSE_EXT, rename variable for legibility. ---
v9: separate out from larger series, as this one is ready now; provide diff with more context and a renamed variable. https://www.redhat.com/archives/libvir-list/2012-October/msg01107.html
src/qemu/qemu_driver.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
ACK Peter

On 10/22/2012 03:03 PM, Peter Krempa wrote:
On 10/22/12 22:34, Eric Blake wrote:
The snapshot code when reusing an existing file had hard-to-read logic, as well as a missing sanity check: REUSE_EXT should require the destination to already be present.
* src/qemu/qemu_driver.c (qemuDomainSnapshotDiskPrepare): Require destination on REUSE_EXT, rename variable for legibility. ---
v9: separate out from larger series, as this one is ready now; provide diff with more context and a renamed variable. https://www.redhat.com/archives/libvir-list/2012-October/msg01107.html
src/qemu/qemu_driver.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
ACK
Thanks; pushed. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake
-
Peter Krempa