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

[Markus Armbruster]

"Daniel P. Berrange" <address@hidden> writes:

> On Fri, Sep 09, 2016 at 06:21:15PM +0200, Markus Armbruster wrote:
>> "Dr. David Alan Gilbert" <address@hidden> writes:
>> 
>> > * Daniel P. Berrange (address@hidden) 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.

Begs the question why we need different output formats, but...

> 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.

... I agree it should be discussed there, not here.