Re: [Qemu-devel] [PATCH v2] qom: Implement qom-get HMP command

["Daniel P. Berrange" <berrange@redhat.com>]

On Fri, Sep 09, 2016 at 06:21:15PM +0200, Markus Armbruster wrote:
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> 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 :|