On 03/28/2013 10:47 AM, Pavel Hrdina wrote: > QMP command "vm-snapshot-save" has also extra optional force parameter > and name parameter isn't optional anymore. It also returns information > about created snapshot.
Commit message is a bit stale from an earlier version of the series,
since name is still optional in this patch.
> +++ b/qapi-schema.json
> @@ -3453,3 +3453,25 @@
> # Since: 1.5
> ##
> { 'command': 'query-tpm', 'returns': ['TPMInfo'] }
> +
> +##
> +# @vm-snapshot-save:
> +#
> +# Create a snapshot of the whole virtual machine. If tag is provided as
> @name,
> +# it is used as human readable identifier. If there is already a snapshot
> +# with the same tag or id, the force argument needs to be true to replace it.
> +#
> +# The VM is automatically stopped and resumed and saving a snapshot can take
> +# a long time.
> +#
> +# @name: #optional tag of new snapshot or tag|id of existing snapshot
> +#
> +# @force: #optional specify whether existing snapshot is replaced or not,
> +# default is false
> +#
> +# Returns: Nothing on success
Comment is stale, now that you return the SnapshotInfo of the
newly-created snapshot.
> +#
> +# Since: 1.5
> +##
> +{ 'command': 'vm-snapshot-save', 'data': {'*name': 'str', '*force': 'bool'},
> + 'returns': 'SnapshotInfo' }
> +++ b/savevm.c
> @@ -2249,17 +2249,18 @@ static int del_existing_snapshots(const char *name,
> Error **errp)
> return 0;
> }
>
> -void do_savevm(Monitor *mon, const QDict *qdict)
> +SnapshotInfo *qmp_vm_snapshot_save(bool has_name, const char *name,
> + bool has_force, bool force, Error **errp)
Indentation is off by one (the two "bool has_" should be aligned).
> @@ -2271,16 +2272,16 @@ void do_savevm(Monitor *mon, const QDict *qdict)
> }
>
> if (!bdrv_can_snapshot(bs)) {
> - monitor_printf(mon, "Device '%s' is writable but does not
> support snapshots.\n",
> - bdrv_get_device_name(bs));
> - return;
> + error_setg(errp, "Device '%s' is writable but does not support "
> + "snapshots.", bdrv_get_device_name(bs));
Missed a removal of a trailing '.' in the error message.
> + return NULL;
> }
> }
>
> bs = bdrv_snapshots();
> if (!bs) {
> - monitor_printf(mon, "No block device can accept snapshots\n");
> - return;
> + error_setg(errp, "No block device can accept snapshots.");
And completely added a trailing '.' here.
> + if (has_force && force) {
> + pstrcpy(sn->name, sizeof(sn->name), old_sn->name);
> + pstrcpy(sn->id_str, sizeof(sn->id_str), old_sn->id_str);
> +
> + /* Delete old snapshots of the same name */
> + if (del_existing_snapshots(name, &local_err) < 0) {
> + error_propagate(errp, local_err);
> + goto the_end;
> + }
> + } else {
> + error_setg(errp, "Snapshot '%s' exists. For override use "
> + "'force' parameter.", name);
New message with trailing '.'; maybe:
"cannot override existing snapshot '%s' without 'force' parameter"
> /* save the VM state */
> f = qemu_fopen_bdrv(bs, 1);
> if (!f) {
> - monitor_printf(mon, "Could not open VM state file\n");
> + error_setg(errp, "Failed to open '%s' file.",
> bdrv_get_device_name(bs));
don't add a trailing '.'
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
