[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 4/8] dump-guest-memory: add qmp event DUMP_CO
From: |
Peter Xu |
Subject: |
Re: [Qemu-devel] [PATCH v2 4/8] dump-guest-memory: add qmp event DUMP_COMPLETED |
Date: |
Tue, 1 Dec 2015 09:52:40 +0800 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Mon, Nov 30, 2015 at 11:18:48AM -0700, Eric Blake wrote:
> 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?
I was just trying to make sure that dump will work exactly as it is
when detach is not used. Yes, I think I could emit it
unconditionally if libvirt does not complain about it. :)
>
> >
> > 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).
I will make sure v4 contains these suggestions. Thanks.
>
> > +
> > +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.
Sorry I missed that. Thanks to point out!
Peter
>
> > +# Since: 2.6
> > +##
> > +{ 'event': 'DUMP_COMPLETED' ,
> > + 'data': { 'msg': 'str' } }
> >
>
> --
> Eric Blake eblake redhat com +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
- Re: [Qemu-devel] [PATCH v2 3/8] dump-guest-memory: add basic "detach" support., (continued)
[Qemu-devel] [PATCH v2 4/8] dump-guest-memory: add qmp event DUMP_COMPLETED, Peter Xu, 2015/11/26
[Qemu-devel] [PATCH v2 5/8] dump-query: add "dump-query" command to query dump status, Peter Xu, 2015/11/26
Re: [Qemu-devel] [PATCH v2 5/8] dump-query: add "dump-query" command to query dump status, Eric Blake, 2015/11/30
[Qemu-devel] [PATCH v2 6/8] dump-query: implement "status" of "dump-query" command., Peter Xu, 2015/11/26