qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Peter Xu <peterx@redhat.com>, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v2 4/8] dump-guest-memory: add qmp event DUMP_COMPLETED
Date: Mon, 30 Nov 2015 11:18:48 -0700	[thread overview]
Message-ID: <565C9308.3090901@redhat.com> (raw)
In-Reply-To: <1448592497-2462-5-git-send-email-peterx@redhat.com>

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

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 <peterx@redhat.com>
> ---
>  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


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

  parent reply	other threads:[~2015-11-30 18:19 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-27  2:48 [Qemu-devel] [PATCH v2 0/8] Add basic "detach" support for dump-guest-memory Peter Xu
2015-11-27  2:48 ` [Qemu-devel] [PATCH v2 1/8] dump-guest-memory: cleanup: removing dump_{error|cleanup}() Peter Xu
2015-11-27  4:28   ` Fam Zheng
2015-11-27  6:51     ` Peter Xu
2015-11-27  2:48 ` [Qemu-devel] [PATCH v2 2/8] dump-guest-memory: add "detach" flag for QMP/HMP interfaces Peter Xu
2015-11-27  4:31   ` Fam Zheng
2015-11-27  6:05     ` Peter Xu
2015-11-30 18:21   ` Eric Blake
2015-12-01  1:28     ` Peter Xu
2015-11-27  2:48 ` [Qemu-devel] [PATCH v2 3/8] dump-guest-memory: add basic "detach" support Peter Xu
2015-11-27  5:14   ` Fam Zheng
2015-11-27  5:20     ` Fam Zheng
2015-11-27  6:27     ` Peter Xu
2015-11-27 10:14       ` Paolo Bonzini
2015-11-27 11:03         ` Peter Xu
2015-11-28  5:51     ` Peter Xu
2015-11-30  1:48       ` Fam Zheng
2015-11-27 10:31   ` Paolo Bonzini
2015-11-27 11:26     ` Peter Xu
2015-11-27 11:52       ` Paolo Bonzini
2015-11-27  2:48 ` [Qemu-devel] [PATCH v2 4/8] dump-guest-memory: add qmp event DUMP_COMPLETED Peter Xu
2015-11-27  5:19   ` Fam Zheng
2015-11-27  6:43     ` Peter Xu
2015-11-27 10:15   ` Paolo Bonzini
2015-11-27 11:29     ` Peter Xu
2015-11-30 18:18   ` Eric Blake [this message]
2015-12-01  1:52     ` Peter Xu
2015-11-27  2:48 ` [Qemu-devel] [PATCH v2 5/8] dump-query: add "dump-query" command to query dump status Peter Xu
2015-11-27  5:25   ` Fam Zheng
2015-11-27  7:03     ` Peter Xu
2015-11-27 10:17       ` Paolo Bonzini
2015-11-27 11:33         ` Peter Xu
2015-11-30 18:27   ` Eric Blake
2015-12-01  2:03     ` Peter Xu
2015-11-27  2:48 ` [Qemu-devel] [PATCH v2 6/8] dump-query: implement "status" of "dump-query" command Peter Xu
2015-11-27 10:22   ` Paolo Bonzini
2015-11-27 11:42     ` Peter Xu
2015-11-27 11:53       ` Paolo Bonzini
2015-11-27  2:48 ` [Qemu-devel] [PATCH v2 7/8] DumpState: adding total_size and written_size fields Peter Xu
2015-11-27  2:48 ` [Qemu-devel] [PATCH v2 8/8] dump-query: make the percentage accurate Peter Xu
2015-11-27  2:59 ` [Qemu-devel] [PATCH v2 0/8] Add basic "detach" support for dump-guest-memory Peter Xu
2015-11-27  5:28 ` Fam Zheng
2015-11-27  6:53   ` Peter Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=565C9308.3090901@redhat.com \
    --to=eblake@redhat.com \
    --cc=peterx@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).