[v4,11/11] savevm: add force parameter to HMP command and return snapshot info

HMP command "savevm" now takes extra optional force parameter to specify
whether replace existing snapshot or not. It also returns information
about created snapshot.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 hmp-commands.hx | 16 ++++++++--------
 hmp.c           | 18 +++++++++++++++++-
 2 files changed, 25 insertions(+), 9 deletions(-)

Pavel Hrdina <phrdina@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 <phrdina@redhat.com>
> Reviewed-by: Eric Blake <eblake@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);
>  }