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

[Kevin Wolf <kwolf@redhat.com>]

Am 17.03.2011 14:28, schrieb Anthony Liguori:
> On 03/17/2011 08:15 AM, Kevin Wolf wrote:
>> Am 17.03.2011 13:46, schrieb Anthony Liguori:
>>> On 03/17/2011 07:21 AM, Kevin Wolf wrote:
>>>>>> Another detail is that, event extension is more important than command
>>>>>> extension, because it's probably going to happen. I think it would be very
>>>>>> bad to add new events just because we wanted to add a new field.
>>>>> The way this is typically handled is that signals tend to pass
>>>>> structures instead of lots of fields.  For instance, most of the GDK
>>>>> events just pass a structure for the event (like GdkButtonEvent).
>>>> Can we do that with existing events or would we break the external
>>>> interface because we'd have to nest everything one level deeper?
>>> We have to introduce new versions of existing events anyway so we can
>>> make sure to nest the structures appropriately.  I think BLOCK_IO_ERROR
>>> is the only one that isn't doing this today FWIW.
>> But then we must always send both events in order to maintain
>> compatibility, right? That sucks.
> 
> No, it's more complicated than that unfortunately.  The old events are 
> "broadcast events".  The new event/signal model requires explicit 
> registration.  There is a capabilities negotiation feature that let's us 
> disable broadcast events.
> 
> So from the wire perspective, a newer client will never see/care about 
> broadcast events.

Right, it can disable them (i.e. some events are registered by default
and there's a command/capability that unregisters all events). But
what's the problem if the client re-enables one of these events by
registering for it?

Adding new events means that we need to have code to generate (that's
what I meant when I said "send") both events for a single action.
Especially if we happen to do it again in the future, this is going to
get really ugly.

>> If I understand right, the problem with the current events isn't even on
>> the protocol level, which would be visible externally,
> 
> No, the problem with the old events is that they aren't 
> registered/maskable.  So even if you don't care about BLOCK_IO_ERROR, 
> you're getting the notification.  Plus, we'd like to add the ability to 
> add a tag to events when we register them.

What's the problem with registering them? If you want to stop client
from doing this you must introduce a special case for obsolete events
that cannot be registered. Do we gain anything from this?

> The other problem is that events are all global today.  BLOCK_IO_ERROR 
> is a good example of this.  It's really an error that's specific to a 
> block device and it passes the name of the block device that it's 
> specific to as an argument.  But if we have a masking mechanism it could 
> only globally enable/disable BLOCK_IO_ERROR for all devices.
> 
> It would be much nicer to be able to enable the event for specific block 
> devices.  This requires some protocol visible changes.  I'm still 
> writing up these changes but hope to have something for review soon.

I wonder if the old, more generic event couldn't be generated
automatically if the more specific signal is triggered in the block code.

>>   but just that it
>> doesn't map to the C interface in the way you like.
> 
> I think I've maybe been using "C interface" to much.  The current event 
> wire protocol doesn't map to any client interface well.

If you mean their broadcast style, that's not really related to nesting
or struct vs. argument.

Kevin