From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54786) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a3bb8-0006Mp-Fn for qemu-devel@nongnu.org; Mon, 30 Nov 2015 22:27:23 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a3bb5-0005q8-9a for qemu-devel@nongnu.org; Mon, 30 Nov 2015 22:27:22 -0500 Received: from mx1.redhat.com ([209.132.183.28]:35418) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a3bb5-0005ok-3b for qemu-devel@nongnu.org; Mon, 30 Nov 2015 22:27:19 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id 6F377537A for ; Tue, 1 Dec 2015 03:27:17 +0000 (UTC) Date: Tue, 1 Dec 2015 11:27:03 +0800 From: Peter Xu Message-ID: <20151201032701.GH21032@pxdev.xzpeter.org> References: <1448883140-20249-1-git-send-email-peterx@redhat.com> <1448883140-20249-9-git-send-email-peterx@redhat.com> <565CC9CF.7070401@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <565CC9CF.7070401@redhat.com> Subject: Re: [Qemu-devel] [PATCH v3 08/12] dump-guest-memory: add qmp event DUMP_COMPLETED List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: drjones@redhat.com, armbru@redhat.com, qemu-devel@nongnu.org, famz@redhat.com, pbonzini@redhat.com, lcapitulino@redhat.com, lersek@redhat.com 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 > > #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 >