Pavel Hrdina <phrd...@redhat.com> writes:
> HMP command "savevm" now takes extra optional force parameter to specify
> whether replace existing snapshot or not. It also returns information
> about created snapshot.
It doesn't return anything, it prints :)
> Signed-off-by: Pavel Hrdina <phrd...@redhat.com>
> Reviewed-by: Eric Blake <ebl...@redhat.com>
> ---
> hmp-commands.hx | 16 ++++++++--------
> hmp.c | 18 +++++++++++++++++-
> 2 files changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 382b87d..9719cc0 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -307,19 +307,19 @@ ETEXI
>
> {
> .name = "savevm",
> - .args_type = "name:s?",
> - .params = "[tag|id]",
> - .help = "save a VM snapshot. If no tag or id are provided, a
> new snapshot is created",
> + .args_type = "force:-f,name:s?",
> + .params = "[-f] [tag|id]",
> + .help = "save a VM snapshot, to replace existing snapshot use
> force flag",
Humor me: semicolon instead of comma.
> .mhandler.cmd = hmp_vm_snapshot_save,
> },
>
> STEXI
> -@item savevm [@var{tag}|@var{id}]
> +@item savevm [@var{-f}] @var{tag}|@var{id}
> @findex savevm
> -Create a snapshot of the whole virtual machine. If @var{tag} is
> -provided, it is used as human readable identifier. If there is already
> -a snapshot with the same @var{tag} or @var{id}, it is replaced. More info at
> -@ref{vm_snapshots}.
> +Create a snapshot of the whole virtual machine. Parameter "name" is optional.
Huh? There is no parameter "name".
> +If @var{tag} is provided, it is used as human readable identifier. If there
> is
> +already a snapshot with the same @var{tag} or @var{id}, @var{-f} flag needs
> to
> +be specified. More info at @ref{vm_snapshots}.
Suggest "needs to be specified to replace it".
> ETEXI
>
> {
Since you're touching the texinfo in this patch anyway, suggest to move
the markup fix from PATCH 07 here. Only if you respin anyway.
> diff --git a/hmp.c b/hmp.c
> index b38b6ce..151e48b 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1437,10 +1437,26 @@ void hmp_chardev_remove(Monitor *mon, const QDict
> *qdict)
> void hmp_vm_snapshot_save(Monitor *mon, const QDict *qdict)
> {
> const char *name = qdict_get_try_str(qdict, "name");
> + bool force = qdict_get_try_bool(qdict, "force", 0);
> Error *err = NULL;
> SnapshotInfo *info = NULL;
>
> - info = qmp_vm_snapshot_save(!!name, name, true, true, &err);
> + info = qmp_vm_snapshot_save(!!name, name, !!force, force, &err);
> +
> + if (info) {
> + char buf[256];
> + QEMUSnapshotInfo sn = {
> + .vm_state_size = info->vm_state_size,
> + .date_sec = info->date_sec,
> + .date_nsec = info->date_nsec,
> + .vm_clock_nsec = info->vm_clock_sec * 1000000000 +
> + info->vm_clock_nsec,
> + };
> + pstrcpy(sn.id_str, sizeof(sn.id_str), info->id);
> + pstrcpy(sn.name, sizeof(sn.name), info->name);
Need to copy because QAPI type SnapshotInfo duplicates QEMUSnapshotInfo.
Cleaning that up is outside the scope of this series.
> + monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf),
> &sn));
Printing this makes lots of sense :)
Stick in a print of bdrv_snapshot_dump(..., NULL) to get a nice header,
like we do elsewhere?
> + }
> +
> qapi_free_SnapshotInfo(info);
> hmp_handle_error(mon, &err);
> }
