Re: [Qemu-devel] RFC: Full introspection support for QMP

[Kevin Wolf]

Am 23.05.2013 um 15:52 hat Anthony Liguori geschrieben:
> Kevin Wolf <address@hidden> writes:
> 
> > Am 23.05.2013 um 14:08 hat Anthony Liguori geschrieben:
> >> Kevin Wolf <address@hidden> writes:
> >> 
> >> 
> >> There are two things here: the schema and the generated code.  The
> >> generated code can and should live in the module.
> >> 
> >> But the schema always stays the same.
> >> 
> >> Think of the schema like kernel headers.  The kernel headers are always
> >> fixed regardless of what kernel modules are loaded or how the kernel is
> >> configured.
> >> 
> >> > then we don't need introspection at all. There's no user for it then.
> >> 
> >> Introspection is not the right approach to feature discovery.  The
> >> schema does answer the question of what features are enabled, it just
> >> answers the question of what the signature of the methods are.
> >
> > You can see it like this. Then, as I said, it's totally useless,
> 
> It's not totally useless.  It's impossible to write a reasonable python
> client for QMP because arguments are not ordered (they are always
> named).  So you can't have:
> 
> qmp.block_commit('ide0-hd0', ...)
> 
> You have to have:
> 
> qmp.block_commit(device='ide0-hd0', ...)
> 
> Which is a little less nice.  The schema introspection solves this but I
> agree, it's not a critical problem to solve.

Well, yes, totally useless may be exaggerated. But this cosmetic problem
of a Python client is an argument as weak as (or even weaker than) the
needs of your beloved QMP client C library: Both of them are purely
hypothetical, they don't exist and nobody plans to actually implement
them.

If this was the only valid reason for schema introspection, I suspect
developmen would stop immediately.

> > because nobody has ever asked this question. The context in which
> > libvirt wants to use it is feature discovery. If we don't support
> > that, then there's no reason to provide introspection at all.
> >
> > libvirt already knows how to use features. It must know it, just parsing
> > the schema doesn't automagically give you libvirt code, so someone must
> > have coded the libvirt side of things. The interesting part is whether a
> > given interface is available on this specific qemu binary.
> 
> Take 'drive-mirror' as an example.  The format parameter is a string.
> You can't tell what valid arguments are for this parameter with
> introspection.
> 
> You can certainly say that we can make this an enum, and do
> introspection on the enums, but that only works because it's a string.
> 
> The same thing can apply to integer arguments.  If a new value becomes
> valid that previously was rejected, then libvirt probably needs to be
> able to figure that out.

I'm not sure how often this actually happens and if we really need to
consider it. But in theory we could still add versioning information to
fields if we use Eric's proposal and have a struct for each field.

> >> > It also makes the schema totally useless. If you can't use it to tell
> >> > which commands this qemu can execute and which it can't,
> >> 
> >> query-commands serves that purpose.
> >
> > It solves a subset of this problem. Optional fields can be added as
> > arguments or to returned structs, enums can be extended, and so far
> > we're having a hard time making use of it because the client can't
> > discover it.
> 
> Ack.
> 
> >> > We can have hundreds of individual query commands like you suggest
> >> > (query-qcow2-creation-option-values, yay!) or we do the modularity
> >> > thing and the schema introspection properly and make it dynamic. I
> >> > prefer the latter.
> >> 
> >> Let's consider a real example.  It sounds like you have something in
> >> mind, can you be more specific?
> >
> > Not a very specific one, it's just that the more I discuss about things
> > like blockdev-add, the more I get the impression that there is an awful
> > lot of information to query. Each image format can provide different
> > options, for creating images and for opening them, and some of them may
> > be enums that could be extended and whose values must be queried etc.
> 
> But is this specific to blockdev-add?  The supported image formats are
> global and apply to multiple commands.

Sure. There's no reason why enums or the option structs can't be used in
the definition of multiple commands. They are simply named QAPI types
and as such aren't specific to blockdev-add, it's only one user.

> Also, do we need to expose which formats are read-only vs. read-write?
> That wouldn't be part of the schema I would think.

No, probably not. That would be describing capabilities of the object
created by blockdev-add, which is neither an argument nor a return
value. So we wouldn't describe it in the blockdev-add schema or any
types referenced by it.

But do we need this information before creating the block device? It
seems unlikely, but I honestly don't know.

> > Schema introspection allows you to have one single way to check for all
> > optional fields, enum values, union branches. The other way is to have a
> > separate command for each of them.
> 
> Yes, my only worry is that the schema probably doesn't contain all of
> the information that something like libvirt needs.

I think it would already be great if it contained most of the
information. We can always add separate query-* commands if we have to,
but having a uniform way to query the majority of features is certainly
more attractive than using only query-*.

> > Basically instead of query-enum(Qcow2PreallocationType) you get
> > query-qcow2-prellocation-types(). You can imagine how the set of
> > commands grows once you start doing things this way.
> >
> > When I discussed with Eric (who is one of the few actual consumers of
> > the API), he preferred introspection of a dynamic schema as well,
> > compared to many separate query-* commands.
> 
> I'm not opposed to schema introspection even if it's used for this type
> of use-case.
> 
> But I don't think it's a complete solution.  We should think about what
> the use-cases are and make sure we have a solution that addresses them.
> 
> None of that should hold back schema introspection...

Agreed.

With Eric's proposal that involves postprocessing qapi-schema.json for a
more explicit wire format, we actually gain much flexibility to add more
detailed descriptions of fields if needed, and probably also get quite
close to a dynamic schema implementation-wise.

Kevin