From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47691) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1biPgR-0003pr-0S for qemu-devel@nongnu.org; Fri, 09 Sep 2016 13:33:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1biPgP-0005tx-1z for qemu-devel@nongnu.org; Fri, 09 Sep 2016 13:33:46 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60384) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1biPgO-0005tr-Q1 for qemu-devel@nongnu.org; Fri, 09 Sep 2016 13:33:44 -0400 Date: Fri, 9 Sep 2016 18:33:40 +0100 From: "Daniel P. Berrange" Message-ID: <20160909173340.GN25802@redhat.com> Reply-To: "Daniel P. Berrange" References: <1473157086-12062-1-git-send-email-dgilbert@redhat.com> <20160906130843.GI10095@redhat.com> <20160906133349.GH2041@work-vm> <87wpilm4ck.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <87wpilm4ck.fsf@dusky.pond.sub.org> Subject: Re: [Qemu-devel] [PATCH v2] qom: Implement qom-get HMP command List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: "Dr. David Alan Gilbert" , kwolf@redhat.com, qemu-devel@nongnu.org, arei.gonglei@huawei.com, pbonzini@redhat.com, lcapitulino@redhat.com, afaerber@suse.de On Fri, Sep 09, 2016 at 06:21:15PM +0200, Markus Armbruster wrote: > "Dr. David Alan Gilbert" writes: > > > * Daniel P. Berrange (berrange@redhat.com) wrote: > >> IIUC, you switched because string-output-visitor could not handle > >> complex types. > >> > >> I have previously written a text-output-visitor that could do > >> this correctly, since we have this exact same requirement for > >> 'qemu-info info' to print out the extra-block specific data > >> in human friendly format for the LUKS driver. > >> > >> https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg01727.html > > > > How close to going in is it? It looks from the comments that Eric > > is thinking about fixing string-output-visitor. > > I'm not clear why you'd want a text-output-visitor and a string-output-vistior. > > Me neither. > > In theory, we need two output visitors producing text, one for > machine-readable output (JSON), and the one for human-readable output. > > For the latter, we use the string output visitor. > > For the former, we first use the (badly named) QMP output visitor to > produce a QObject, which is then converted to JSON text by > qobject_to_json() or qobject_to_json_pretty(). > > Each of the two output visitors comes with a matching input visitor that > reads the same format. > > To read JSON text, we first parse it into a QObject with > json_message_parser_init() & friends, which we then pass to the QMP > input visitor. > > If I read Dan's cover letter correctly, the proposed text output visitor > competes with the string output visitor. Why not enhance or replace it? I guess my original posting was not clear enough about the rational for the separate visitor. Looking closely at the output format for the existing string output visitor and comparing to what's proposed for my text output visitor, you'll see they are completely different output formats. They both happen to create strings, but that's pretty much where the similarity ends. So while we could squish the functionality into one visitor class, it wouldn't really let us share any code - the visitor would just end up taking different codepaths for each style. Having these two styles mixed will then make it harder to understand the logic behind either of them. There is a specific place where code sharing is useful though - the formatting of a uint64 in sized notatation ie "1.24 GB", and for that I'm creating a new shared helper function in util/cutils since we've already got 2 copies of that logic ! Anyway, lets continue this debate on the patch series which actualy proposes the text-output-visitor, which I'm re-posting in a few minutes with greater explanation of the new format which should hopefully clarify why it is justified. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|