Re: [Qemu-devel] [PATCH v3 10/12] Dump: add qmp command "query-dump"

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2576 bytes --]

On 11/30/2015 04:32 AM, Peter Xu wrote:
> When dump-guest-memory is requested with detach flag, after its
> return, user could query its status using "query-dump" command (with
> no argument). The result for now contains:
> 
> - status: current dump status
> - written_bytes: bytes written in latest dump
> - total_bytes: bytes to write in latest dump
> 
>>From written_bytes and total_bytes, we could see how much work
> finished by calculating:
> 
>   100.0 * written_bytes / total_bytes (%)
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  dump.c           | 10 ++++++++++
>  qapi-schema.json | 29 +++++++++++++++++++++++++++++
>  qmp-commands.hx  | 26 +++++++++++++++++++++++++-
>  3 files changed, 64 insertions(+), 1 deletion(-)
> 
> diff --git a/dump.c b/dump.c
> index 56a2d7e..6596bc8 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -1675,6 +1675,16 @@ static void *dump_thread(void *data)
>      return NULL;
>  }
>  
> +DumpQueryResult *qmp_query_dump(Error **errp)
> +{
> +    DumpQueryResult *result = g_malloc0(sizeof(*result));

Might be nicer as g_new0(DumpQueryResult, 1). Markus has been switching
to g_new0 where a type name was already mentioned, although here you
used *result rather than a typename.


> +++ b/qapi-schema.json
> @@ -2157,6 +2157,35 @@
>    'data': [ 'none', 'active', 'completed', 'failed' ] }
>  
>  ##
> +# @DumpQueryResult
> +#
> +# The result format for 'query-dump'.
> +#
> +# @status: enum of @DumpStatus, which shows current dump status
> +#
> +# @written_bytes: bytes written in latest dump (uncompressed)
> +#
> +# @total_bytes: total bytes to be write in latest dump (uncompressed)

s/be write/written/

> +#
> +# Since 2.6
> +##
> +{ 'struct': 'DumpQueryResult',
> +  'data': { 'status': 'DumpStatus',
> +            'written_bytes': 'int',
> +            'total_bytes': 'int' } }

Prefer '-' over '_' in new QMP (as in 'total-bytes' rather than
'total_bytes').  Furthermore, QMP already defaults to bytes, so it would
be sufficient to name these merely 'written'/'total' or even
'complete'/'total'.

> +++ b/qmp-commands.hx
> @@ -881,7 +881,7 @@ EQMP
>      {
>          .name       = "query-dump-guest-memory-capability",
>          .args_type  = "",
> -    .mhandler.cmd_new = qmp_marshal_query_dump_guest_memory_capability,
> +        .mhandler.cmd_new = qmp_marshal_query_dump_guest_memory_capability,
>      },

Unrelated hunk.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]