From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54254) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YoYei-0004Np-7z for qemu-devel@nongnu.org; Sat, 02 May 2015 10:44:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YoYee-0003cO-0i for qemu-devel@nongnu.org; Sat, 02 May 2015 10:44:35 -0400 Received: from cantor2.suse.de ([195.135.220.15]:41673 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YoYed-0003bd-Qs for qemu-devel@nongnu.org; Sat, 02 May 2015 10:44:31 -0400 Message-ID: <5544E2C9.40509@suse.de> Date: Sat, 02 May 2015 16:44:25 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1429975686-19494-1-git-send-email-ehabkost@redhat.com> <553BC973.2030506@suse.de> <20150427172320.GC25766@thinpad.lan.raisama.net> <20150429083802.06f102ea@redhat.com> <20150429125548.GF25766@thinpad.lan.raisama.net> <20150501161905.11253.15908@loki> <20150501162309.11253.4357@loki> In-Reply-To: <20150501162309.11253.4357@loki> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] QJSON: Use OBJECT_CHECK List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Roth , Eduardo Habkost , Luiz Capitulino Cc: qemu-devel@nongnu.org, =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= , Juan Quintela On 01.05.15 18:23, Michael Roth wrote: > Quoting Michael Roth (2015-05-01 11:19:05) >> Quoting Eduardo Habkost (2015-04-29 07:55:48) >>> On Wed, Apr 29, 2015 at 08:38:02AM -0400, Luiz Capitulino wrote: >>>> On Mon, 27 Apr 2015 14:23:20 -0300 >>>> Eduardo Habkost wrote: >>>> >>>>> On Sat, Apr 25, 2015 at 07:05:55PM +0200, Andreas F=C3=A4rber wrote= : >>>>>> Am 25.04.2015 um 17:28 schrieb Eduardo Habkost: >>>>>>> The QJSON code used casts to (QJSON*) directly, instead of OBJECT= _CHECK. >>>>>>> There were even some functions using object_dynamic_cast() calls >>>>>>> followed by assert(), which is exactly what OBJECT_CHECK does (by >>>>>>> calling object_dynamic_cast_assert()). >>>>>> >>>>>> Suggest s/OBJECT_CHECK/OBJECT_CHECK()/g everywhere for clarity. >>>> >>>> Everywhere? You mean, in other places? In this case someone has to >>>> post a different patch. >>> >>> Just in the commit message. >>> >>>> >>>>> I assume it can be fixed during commit by whoever is going to queue= it. >>>>> >>>>>> >>>>>>> >>>>>>> Signed-off-by: Eduardo Habkost >>>>>>> --- >>>>>>> qjson.c | 10 +++++----- >>>>>>> 1 file changed, 5 insertions(+), 5 deletions(-) >>>>>> >>>>>> Reviewed-by: Andreas F=C3=A4rber >>>>>> >>>>>> Wasn't aware QJSON is using QOM - assuming this will go through so= me >>>>>> QAPI/QMP tree. >>>>> >>>>> The only user of qjson.c right now is migration code. Should it go = through >>>>> the migration tree? >>>> >>>> It could be, but I can take it if nobody does. >>> >>> Thanks! >>> >>>> >>>>> Also, why do we have two JSON writers in QEMU? And why do they have >>>>> exactly the same name? >>>> >>>> Not sure I got it, which writers? >>> >>> qjson.c and qobject/qjson.c:to_json(). >> >> I'm guessing it's to avoid the need to build up a QObject throughout >> the migration code, as opposed to just serializing metadata/vmstate >> fields directly to string. >> >> Does make me wonder though why we don't just use visit_type_{int,etc}(= ) >> interfaces to build up the QObject through a QMPOutputVisitor, then fe= ed >> the resulting QObject through the existing qobject/qjson.c code. >=20 > I guess that would affect downtime. Don't imagine it would by much > though. A JSONOutputVisitor that simply wraps JSONMessageParser > and avoids the QObject intermediate might be another option. Well, there were a number of reasons why I didn't want to use the QObject json writer. The biggest one is complexity. We're really trying to do something incredibly trivial, namely writing json string data continuously. Allocating all that memory (potentially running oom) didn't seem incredibly appealing to me. Alex