Re: [Qemu-devel] [PATCH] full introspection support for QMP

[Kevin Wolf]

Am 03.07.2013 um 17:59 hat Anthony Liguori geschrieben:
> Kevin Wolf <address@hidden> writes:
> 
> > Am 02.07.2013 um 19:06 hat Anthony Liguori geschrieben:
> >> Eric Blake <address@hidden> writes:
> >> > On 07/02/2013 08:51 AM, Anthony Liguori wrote:
> >> >> Amos Kong <address@hidden> writes:
> >> >> 
> >> >>> Introduces new monitor command to query QMP schema information,
> >> >>> the return data is a nested dict/list, it contains the useful
> >> >>> metadata.
> >> >>>
> >> >>> we can add events definations to qapi-schema.json, then it can
> >> >>> also be queried.
> >> >>>
> >> >>> Signed-off-by: Amos Kong <address@hidden>
> >> >> 
> >> >> Maybe I'm being too meta here, but why not just return qapi-schema.json
> >> >> as a string and call it as day?
> >
> > I know you don't agree with this, but as I mentioned several times
> > before, I think the schema as returned by the introspection functions
> > shouldn't contain what a qemu of this version _could_ in theory provide,
> > but what this specific build actually _does_ provide. It shouldn't
> > include things that are compiled out.
> 
> I really don't disagree with you here.  I just don't like having two
> formats for the schema.

So you agree that we have to postprocess at least in the sense that we
leave out things that aren't available?

In this case, I think you already have most of the postprocessing code
(and this diffstat of this patch seems to show that it's not that much
code anyway), so code size isn't a valid point any more. Then we can
concentrate on getting the optimal wire format and do whatever is needed
to implement it.

> >> > I've also been the one arguing that the additional complexity (an array 
> >> > of
> >> > {"name":"str","type":"str","optional":bool"}) is better for libvirt in
> >> > that the JSON is then well-suited for scanning (it is easier to scan
> >> > through an array where the key is a constant "name", and looking for the
> >> > value that we are interested in, than it is to scan through a dictionary
> >> > where the keys of the dictionary are the names we are interested in).
> >> > That is, the JSON in qapi-schema.json is a nice compact representation
> >> > that works for humans, but may be a bit TOO compact for handling via
> >> > machines.
> >> 
> >> But adding a bunch of code to do JSON translation just adds a bunch of
> >> additional complexity.
> >> 
> >> One reasonable compromise would be:
> >> 
> >> { "command": "foo", "arguments": { "name": "str", "id": "int" },
> >>                     "optional": { "bar": "bool" } }
> >
> > This assumes that optional vs. mandatory is the only property we ever
> > want to describe for fields. Eric's approach is much more future-proof.
> > Let's keep the format of qapi-schema.json an implementation detail that
> > we can change and extend when necessary.
> 
> It's always possible to add another argument that describes additional
> information.
> 
> For instance:
> 
> { "command": "foo",
>   "arguments": { "name": "str", "id": "int" },
>   "optional": { "bar": "bool" },
>   "defaults": { "bar": false } }
> 
> That doesn't mean I think exposing defaults is good, but rather that
> it's still possible to do this in a compact form.

Yeah, it's possible, but it feels kind of backwards to have the
properties on the top level and repeat the field names for each property
that they have.

How does it work for nested structs? There you don't have the
"arguments" substructure, so you'd have to have "optional" as a child of
all the other fields or something like that. It becomes ugly quite
quickly.

Kevin