[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 08/12] dump-guest-memory: add qmp event DUMP_
From: |
Peter Xu |
Subject: |
Re: [Qemu-devel] [PATCH v3 08/12] dump-guest-memory: add qmp event DUMP_COMPLETED |
Date: |
Tue, 1 Dec 2015 11:27:03 +0800 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Mon, Nov 30, 2015 at 03:12:31PM -0700, Eric Blake wrote:
> On 11/30/2015 04:32 AM, Peter Xu wrote:
> > +Example:
> > +
> > +{ "event": "DUMP_COMPLETED",
> > + "data": {} }
>
> Please keep this file sorted. The insertion should be between
> DEVICE_TRAY_MOVED and GUEST_PANICKED.
Sorry for the incaution to miss that. Will fix in v4.
>
> > diff --git a/dump.c b/dump.c
> > index 14fd41f..43f565d 100644
> > --- a/dump.c
> > +++ b/dump.c
> > @@ -25,6 +25,7 @@
> > #include "qapi/error.h"
> > #include "qapi/qmp/qerror.h"
> > #include "qmp-commands.h"
> > +#include "qapi-event.h"
> >
> > #include <zlib.h>
> > #ifdef CONFIG_LZO
> > @@ -1633,8 +1634,11 @@ static void *dump_thread(void *data)
> > dump_process(s, &err);
> >
> > if (err) {
> > - /* TODO: notify user the error */
> > + qapi_event_send_dump_completed(true, error_get_pretty(err),
> > + &error_abort);
> > error_free(err);
> > + } else {
> > + qapi_event_send_dump_completed(false, NULL, &error_abort);
> > }
>
> Hmmm. I wonder if error_get_pretty() should be improved to return NULL
> when there is no error. Then we could write:
>
> qapi_event_send_dump_completed(!!err, error_get_pretty(err),
> &error_abort);
> error_free(err);
>
> But that doesn't affect the correctness of your patch.
After seeing that improving error_get_pretty() is simple (and also
will not break the other callers), I'd like to take your advice,
which is clean enough. Thanks.
>
> > return NULL;
> > }
> > diff --git a/qapi/event.json b/qapi/event.json
> > index f0cef01..c46214b 100644
> > --- a/qapi/event.json
> > +++ 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
> > +#
> > +# Since: 2.6
>
> Missing documentation of 'error'.
Added.
Thanks!
Peter
>
> > +##
> > +{ 'event': 'DUMP_COMPLETED' ,
> > + 'data': { '*error': 'str' } }
> >
>
> --
> Eric Blake eblake redhat com +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
- Re: [Qemu-devel] [PATCH v3 03/12] dump-guest-memory: using static DumpState, add DumpStatus, (continued)
- [Qemu-devel] [PATCH v3 04/12] dump-guest-memory: add dump_in_progress() helper function, Peter Xu, 2015/11/30
- [Qemu-devel] [PATCH v3 05/12] dump-guest-memory: introduce dump_process() helper function., Peter Xu, 2015/11/30
- [Qemu-devel] [PATCH v3 06/12] dump-guest-memory: disable dump when in INMIGRATE state, Peter Xu, 2015/11/30
- [Qemu-devel] [PATCH v3 07/12] dump-guest-memory: add "detach" support, Peter Xu, 2015/11/30
- [Qemu-devel] [PATCH v3 08/12] dump-guest-memory: add qmp event DUMP_COMPLETED, Peter Xu, 2015/11/30
- [Qemu-devel] [PATCH v3 09/12] DumpState: adding total_size and written_size fields, Peter Xu, 2015/11/30
- [Qemu-devel] [PATCH v3 10/12] Dump: add qmp command "query-dump", Peter Xu, 2015/11/30
- [Qemu-devel] [PATCH v3 11/12] Dump: add hmp command "info dump", Peter Xu, 2015/11/30
- [Qemu-devel] [PATCH v3 12/12] Dump: enhance the documentations., Peter Xu, 2015/11/30