Re: [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2)

[Luiz Capitulino]

On Wed, 16 Mar 2011 13:32:50 -0500
Anthony Liguori <address@hidden> wrote:

> On 03/16/2011 01:09 PM, Luiz Capitulino wrote:
> >> This is a shift in our direction but the good news is that the practical
> >> impact is small.  But I don't think there's a lot of value of focusing
> >> on non-C consumers because any non-C consumer is capable of consuming a
> >> good C interface (but the inverse is not true).
> > I disagree. To access a C interface from a high-level language you usually
> > have to write bindings. Using something like QMP instead of writing bindings
> > is a lot easier.
> 
> I think you're confusing multiple points.
> 
> A C friendly interface has the following properties:
> 
> 1) arguments have a well defined order
> 2) variant arrays are not used
> 3) variant types are not used
> 4) structures only have items added to the end of them with some sort of 
> way to determine at run time whether the argument is present
> 5) functions don't increase their number of arguments
> 
> An API that follows these rules is a Good Library interface in C.  It 
> also happens to be a Good Library interface in Python and just about 
> every other language.
> 
> You can design interfaces in Python that rely on variant arrays or 
> types, or that add keyword values to arguments, but the absence of those 
> does not make a Bad Library in Python.
> 
> This has nothing to do with the need for bindings.  

I mentioned bindings because you put so much emphasis on C consumers
that sometimes I wonder if all you want is libqmp and libqmp only.

> The need for 
> bindings is entirely based on whether the RPC being used is 
> self-describing.  JSON is.
> 
> That said, I think we made a critical mistake in QMP that practically 
> means that we need bindings for QMP.  There is no argument ordering.  

I'm sorry? Critical mistake? Didn't _we_ consciously choose a dictionary
for this?

> So 
> I can write a Python class that does:
> 
> import qmp
> 
> qmp.eject_device(device='ide0-hd0')
> 
> But I cannot write a library today that does:
> 
> qmp.eject_device('ide0-hd0')
> 
> Without using a library that has knowledge of the QMP schema.  This is 
> somewhat unfortunate and I've been thinking about adding another code 
> generation mode in qmp-gen.py to generate wrappers that would enable the 
> more natural usage in Python.

I don't get it. Having knowledge of the schema is needed anyway, isn't it?

> > Also, what's the problem with C consumers using QMP? Libvirt is C, and it
> > does it just fine.
> 
> I was going to cut and paste libvirt's code that handles query-pci to 
> show you how complex it is to use vs. just using libqmp

Does this suggest you think libvirt should switch from QMP to libqmp?

And, if you don't want to focus on non-C consumers, why having QMP at all?
(I'm *not* saying I'm ok in dropping it).

> but it turns out 
> libvirt doesn't implement query-pci in QMP mode and falls back to the 
> human monitor.  I think that might be as good of a way to show my point 
> though :-)

This comment assumes two things: 1. query-pci is good in its current form,
2. libvirt doesn't use it because it's complex.

We already had changes proposal to query-pci and I can't tell why libvirt
doesn't use it. If it's because it's complex, then we need to know why it's
complex: is it the command or is it accessing JSON from C?
 
> >> But the same sort of compatibility considerations apply to using QMP
> >> within QEMU.  If you add a new field to a function call, we need to
> >> modify any internal usage of the API.
> > What's the problem of doing this?
> 
> If we have a bunch of code in QEMU that relies on doing
> 
> qmp_set_vnc_password(password, &err);
> 
> And then we add a new parameter to set_vnc_password(), we now need to 
> touch ever place we call this.
>
> Good external interfaces also happen to be good internal interfaces and 
> encourage better modularity within QEMU itself.  It also means that for 
> subsystems that we can convert to be largely implemented in terms of 
> QMP, we have the option to move them into an external process which is 
> good from a security PoV.

I agree with the security PoV, but moving things to a separate process
is not an immediate step. IOW, there's no need to impose strict compatibilities
rules for something that's going to be internal for some (unknown) time.

> >> If we add a field to a structure,
> >> as long as we use feature flags (we do), only the places that care to
> >> set that field need to worry about it.
> > Why do we need this in an internal interface?
> 
> It's about eating our own dog food.  It helps us ensure that we're 
> really providing high quality external interfaces.

We'll be eating our food just fine by using the internal interface
as an internal interface.

> 
> Regards,
> 
> Anthony Liguori
> 
>