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 <pet...@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).
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
>
