Re: [Qemu-devel] [PATCH v2 4/8] dump-guest-memory: add qmp event DUMP_COMPLETED

[Eric Blake]

On 11/26/2015 07:48 PM, Peter Xu wrote:
> To get aligned with QMP interface, one new QMP event DUMP_COMPLETED
> is added. It is used when user specified "detach" in dump, and
> triggered when the dump finishes. Error message will be appended to
> this event if the dump has failed.

Why not emit the new event unconditionally, instead of only when detach
was specified?

> 
> Signed-off-by: Peter Xu <address@hidden>
> ---
>  docs/qmp-events.txt | 12 ++++++++++++
>  dump.c              | 12 +++++++++++-
>  qapi/event.json     | 10 ++++++++++
>  3 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/qmp-events.txt b/docs/qmp-events.txt
> index d2f1ce4..fe494f9 100644
> --- a/docs/qmp-events.txt
> +++ b/docs/qmp-events.txt
> @@ -674,3 +674,15 @@ Note: If action is "reset", "shutdown", or "pause" the 
> WATCHDOG event is
>  followed respectively by the RESET, SHUTDOWN, or STOP events.
>  
>  Note: this event is rate-limited.
> +
> +DUMP_COMPLETED
> +--------------
> +
> +Emitted when the guest has finished one memory dump.
> +
> +Data: None.

Wrong - you have 'msg' as data.  Except that Paolo is right, 'msg'
should be optional, and only present on error.  You should also document
that the contents of 'msg' are for human consumption and should not be
machine-parsed (basically, only the presence of absence of 'msg' is
useful for machines).

> +
> +Example:
> +
> +{ "event": "DUMP_COMPLETED",
> +  "data": { "msg": "Dump completed successfully" } }
> diff --git a/dump.c b/dump.c

> +++ b/qapi/event.json
> @@ -356,3 +356,13 @@
>  ##
>  { 'event': 'MEM_UNPLUG_ERROR',
>    'data': { 'device': 'str', 'msg': 'str' } }
> +
> +##
> +# @DUMP_COMPLETED
> +#
> +# Emitted when background dump has completed
> +#

Missing documentation of 'msg', which should be optional.

> +# Since: 2.6
> +##
> +{ 'event': 'DUMP_COMPLETED' ,
> +  'data': { 'msg': 'str' } }
> 

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

signature.asc
Description: OpenPGP digital signature