qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Anthony Liguori <anthony@codemonkey.ws>
To: Luiz Capitulino <lcapitulino@redhat.com>
Cc: qemu-devel@nongnu.org, Avi Kivity <avi@redhat.com>,
	Markus Armbruster <armbru@redhat.com>,
	Adam Litke <agl@us.ibm.com>
Subject: Re: [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2)
Date: Wed, 16 Mar 2011 15:00:10 -0500	[thread overview]
Message-ID: <4D8116CA.60207@codemonkey.ws> (raw)
In-Reply-To: <20110316162703.21f03bd8@doriath>

On 03/16/2011 02:27 PM, Luiz Capitulino wrote:
>
>> 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 transports (libqmp and QMP) are less important than the interface 
itself.  My emphasis is on the API, not the transports or 
materialization of it.

The terminology gets confusing and I probably am not consistent enough 
in how I use it.

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

Yes, we did.  In fact, I'm fairly sure that Avi and/or I strongly 
advocated it.

But hindsight is always 20/20 and if our goal is to have an API that 
doesn't require special support in Python beyond a simple transport 
class, using dictionaries and not allowing unnamed positional parameters 
was a mistake.

But we makes lots of mistakes.  That's part of the development process.

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

If we allowed positional arguments, it wouldn't be needed.  For 
instance, with xmlrpclib in python, you can do:

srv = ServerProxy('http://localhost:8000/RPC2')
srv.eject_device('ide0-hd0')

And it Just Works.  The transport doesn't have to know anything about 
the schema because the server does all of the parameter validation.

We could fix this by adding another way of specifying unnamed parameters 
in QMP.  Something like:

import qmp

srv = qmp.ServerProxy('/tmp/qmp.sock')
srv.eject_device('ide0-hd0')

Becomes:

{ 'execute': 'eject_device', 'ordered_arguments': [ 'ide0-hd0' ], 
arguments: {} }

I'm not sure it's really worth it though.  I don't think it's a terrible 
thing for us to generate a Python consumable schema.  I don't think it's 
all that important for us to worry about QMP as an RPC mechanism being 
consumable outside of QEMU which means that we can (and should) be 
providing high level language libraries for 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?

I think they eventually should, yes.  Why duplicate all of the 
marshalling and unmarshalling code unnecessarily especially when dealing 
with complex data structures?

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

No matter what, we need an RPC.  You can't drop QMP because something 
needs to take it's place.

The question is whether we expect projects to write directly to the 
protocol or use libraries provided by use to handle it.  I don't think 
it's practical to expect everyone to write directly to the protocol 
(particularly when dealing with C).

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

I think it's one of our better commands actually.

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

It returns a complex data structure.  There's nothing wrong with that, 
but it's very hard to interface with at the JSON level.  Is there really 
any doubt about that?

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

What I struggle with it what it means for you to say that libqmp is an 
internal interface.  libqmp is a C library that implements QMP.  QEMU 
would never use libqmp directly (it doesn't need to).  If it cannot 
maintain a stable ABI, than that means it's impossible to write a QMP 
client that maintains a stable C ABI unless you totally redefine the 
interfaces.

Is that really desirable?

Regards,

Anthony Liguori

>> Regards,
>>
>> Anthony Liguori
>>
>>
>

  reply	other threads:[~2011-03-16 20:00 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-11 23:05 [Qemu-devel] [PATCH 00/15] QAPI Round 1 (core code generator) (v2) Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 01/15] qapi: add code generator for qmp-types (v2) Anthony Liguori
2011-03-11 23:12   ` [Qemu-devel] " Anthony Liguori
2011-03-12 11:29   ` [Qemu-devel] " Blue Swirl
2011-03-12 15:00     ` Anthony Liguori
2011-03-18 14:18       ` Luiz Capitulino
2011-03-18 14:14   ` [Qemu-devel] " Luiz Capitulino
2011-03-11 23:05 ` [Qemu-devel] [PATCH 02/15] qapi: add code generator for type marshallers Anthony Liguori
2011-03-18 15:13   ` [Qemu-devel] " Luiz Capitulino
2011-03-11 23:05 ` [Qemu-devel] [PATCH 03/15] qapi: add core QMP server support (v2) Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 04/15] qapi: add signal support to core QMP server Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 05/15] qapi: add QAPI module type Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 06/15] qapi: add code generators for QMP command marshaling Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 07/15] qapi: add query-version QMP command Anthony Liguori
2011-03-12 11:19   ` Blue Swirl
2011-03-12 15:06     ` Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 08/15] qapi: add new QMP server that uses CharDriverState (v2) Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 09/15] vl: add a new -qmp2 option to expose experimental QMP server Anthony Liguori
2011-03-11 23:14   ` [Qemu-devel] " Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 10/15] qapi: add QMP quit command Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 11/15] qapi: add QMP qmp_capabilities command Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 12/15] qapi: add QMP put-event command Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 13/15] qapi: add code generator for libqmp (v2) Anthony Liguori
2011-03-12 11:10   ` Blue Swirl
2011-03-12 14:53     ` Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 14/15] qapi: add test-libqmp Anthony Liguori
2011-03-12 11:23   ` Blue Swirl
2011-03-12 14:59     ` Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 15/15] qapi: generate HTML report for test-libqmp Anthony Liguori
2011-03-16 14:34 ` [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2) Luiz Capitulino
2011-03-16 14:49   ` Paolo Bonzini
2011-03-16 15:00     ` Luiz Capitulino
2011-03-16 16:06       ` Anthony Liguori
2011-03-16 16:03     ` Anthony Liguori
2011-03-16 16:31       ` Paolo Bonzini
2011-03-16 18:06         ` Anthony Liguori
2011-03-16 15:59   ` Anthony Liguori
2011-03-16 18:09     ` Luiz Capitulino
2011-03-16 18:32       ` Anthony Liguori
2011-03-16 19:27         ` Luiz Capitulino
2011-03-16 20:00           ` Anthony Liguori [this message]
2011-03-18 14:10             ` Luiz Capitulino
2011-03-18 14:22               ` Anthony Liguori
2011-03-17 12:21     ` Kevin Wolf
2011-03-17 12:46       ` Anthony Liguori
2011-03-17 13:15         ` Kevin Wolf
2011-03-17 13:28           ` Anthony Liguori
2011-03-17 14:04             ` Kevin Wolf
2011-03-17 15:49               ` Anthony Liguori

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4D8116CA.60207@codemonkey.ws \
    --to=anthony@codemonkey.ws \
    --cc=agl@us.ibm.com \
    --cc=armbru@redhat.com \
    --cc=avi@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).