Re: [Qemu-devel] Re: [RFC] qapi: events in QMP

[Anthony Liguori <anthony@codemonkey.ws>]

On 02/14/2011 12:34 PM, Luiz Capitulino wrote:
> On Mon, 14 Feb 2011 08:39:11 -0600
> Anthony Liguori<anthony@codemonkey.ws>  wrote:
>
>    
>> On 02/14/2011 06:45 AM, Luiz Capitulino wrote:
>>      
>>> So the question is: how does the schema based design support extending
>>> commands or events? Does it require adding new commands/events?
>>>
>>>        
>> Well, let me ask you, how do we do that today?
>>
>> Let's say that I want to add a new parameter to the `change' function so
>> that I can include a salt parameter as part of the password.
>>
>> The way we'd do this today is by checking for the 'salt' parameter in
>> qdict, and if it's not present, use a random salt or something like that.
>>      
> You likely want to do what you did before. Of course that you have to
> consider if what you're doing is extending an existing command or badly
> overloading it (like change is today), in this case you'll want to add
> a new command instead.
>
> But yes, the use-case here is extending an existing command.
>
>    
>> However, if I'm a QMP client, how can I tell whether you're going to
>> ignore my salt parameter or actually use it?  Nothing in QMP tells me
>> this today.  If I set the salt parameter in the `change' command, I'll
>> just get a success message.
>>      
> I'm sorry?
>
> { "execute": "change", "arguments": { "device": "vnc", "target": "password", "arg": "1234", "salt": "r1" } }
> {"error": {"class": "InvalidParameter", "desc": "Invalid parameter 'salt'", "data": {"name": "salt"}}}
>    

So I'm supposed to execute the command, and if execution fails, drop the 
new parameter?  If we add a few optional parameters, does that mean I 
have to try every possible combination of parameters?

>    
>> Even if we expose a schema, but leave things as-is, having to parse the
>> schema as part of a function call is pretty horrible,
>>      
> That's a client implementation detail, they are not required to do it
> as part of a function call.
>
> But let me ask, if we don't expose a schema, how will clients be able to
> query available commands/events and their parameters?
>    

We need to expose the schema, I'm not saying we shouldn't.  But we don't 
today.

You're arguing that we should extend commands by adding new parameters.  
I'm saying that's a bad interface.  If we need to change a command, we 
should introduce a new command.  It's a well understood mechanism for 
maintaining compatibility (just about every C library does exactly this).

>> particularly if
>> distros do silly things like backport some optional parameters and not
>> others.  If those optional parameters are deeply nested in a structure,
>> it's even worse.
>>      
> Why would they do this? I mean, if distros (or anyone else shipping qemu)
> goes that deep on changing the wire protocol they are on their own, why
> would we want to solve this problem?
>    

It's not at all unreasonable for a distro to backport a new QMP 
command.  If all modifications are discrete commands, compatibility is 
easy to preserve, however if a distro does backporting and we end up 
with a frankenstein command, compatibility will be an issue.

>> OTOH, if we introduce a new command to set the password with a salt, it
>> becomes very easy for the client to support.  The do something as simple as:
>>
>> if qmp.has_command("vnc-set-password-with-salt"):
>>       qmp.vnc_set_password_with_salt('foobar', 'X*')
>> else:
>>       window.set_weak_security_icon(True)
>>       qmp.vnc_set_password('foobar')
>>
>> Now you could answer, hey, we can add capabilities then those
>> capabilities can quickly get out of hand.
>>      
> Adding one command per new argument has its problems too and it's even
> worse with events, as clients will have to be changed to handle a
> new event just because of a parameter addition.
>    

Yes, but it's an extremely well understood way to design compatible APIs.

> Look, although I did _not_ check any code yet, your description of the QAPI
> looks really exciting. I'm not against it, what bothers me though is this
> number of small limitations we're imposing to the wire protocol.
>
> Why don't we make libqmp internal only? This way we're free to change it
> whatever we want.
>    

libqmp is a test of how easy it is to use QMP from an external 
application.  If we can't keep libqmp stable, then that means tools like 
libvirt will always have a hard time using QMP.

Proper C support is important.  We cannot make it impossible to write a 
useful C client API.

Regards,

Anthony Liguori