[libvirt] [PATCH 3/4] qemu: Don't return success if creation of snapshot save file fails

[Jiri Denemark]

On Thu, Jan 17, 2013 at 14:12:04 +0100, Peter Krempa wrote:
> When the snapshot definition can't be saved, the
> qemuDomainSnapshotCreate function succeeded without filling some of the
> fields in the internal definition.
> 
> This patch removes the snapshot and returns failure if the XML file
> cannot be written.
> ---
>  src/qemu/qemu_driver.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index d52cf24..b264fc8 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -11593,8 +11593,14 @@ cleanup:
>          if (snapshot && !(flags & VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA)) {
>              if (qemuDomainSnapshotWriteMetadata(vm, snap,
>                                                  driver->snapshotDir) < 0) {
> -                VIR_WARN("unable to save metadata for snapshot %s",
> -                         snap->def->name);
> +                /* if writing of metadata fails, error out rather than trying
> +                 * to silently carry on  without completing the snapshot */
> +                virDomainSnapshotFree(snapshot);
> +                snapshot = NULL;
> +                virReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("unable to save metadata for snapshot %s"),
> +                               snap->def->name);
> +                virDomainSnapshotObjListRemove(vm->snapshots, snap);
>              } else {
>                  if (update_current)
>                      vm->current_snapshot = snap;

ACK, I think this approach is better. We will still leave the snapshot
data behind (which could be enhanced in the future when libvirt knows
how to delete external snapshots) but at least we don't pretend
everything was ok and surprise users when such snapshot would disappear
after restarting libvirtd.

Jirka