qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Anthony Liguori
Subject: Re: [Qemu-devel] Re: [RFC] qapi: events in QMP
Date: Mon, 14 Feb 2011 15:14:00 -0600
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.15) Gecko/20101027 Lightning/1.0b1 Thunderbird/3.0.10

On 02/14/2011 06:32 AM, Kevin Wolf wrote:
Am 14.02.2011 13:03, schrieb Anthony Liguori:
On 02/14/2011 03:50 AM, Kevin Wolf wrote:
Am 13.02.2011 19:08, schrieb Anthony Liguori:
Proposal for events in QAPI

For QAPI, I'd like to model events on the notion of signals and
slots[2].  A client would explicitly connect to a signal through a QMP
interface which would result in a slot being added that then generates
an event.  Internally in QEMU, we could also connect slots to the same
signals.  Since we don't have an object API in QMP, we'd use a pair of
connect/disconnect functions that had enough information to identify the
signal.

Example:

We would define QEVENT_BLOCK_IO_EVENT as:

# qmp-schema.json
{ 'BlockIOEvent': {'device': 'str', 'action': 'str', 'operation': 'str'} }

The 'Event' suffix is used to determine that the type is an event and
not a structure.  This would generate the following structures for QEMU:

typedef void (BlockIOEventFunc)(const char *device, const char *action,
const char *operation, void *opaque);

Why is an event not a structure? For one it makes things inconsistent
(you have this 'Event' suffix magic), and it's not even convenient. The
parameter list of the BlockIOEventFunc might become very long. At the
moment you have three const char* there and I think it's only going to
grow - who is supposed to remember the right order of arguments?

So why not make the event a struct and have a typedef void
(BlockIOEventFunc)(BlockIOEvent *evt)?

A signal is a function call.  You can pass a structure as a parameter is
you so desire but the natural thing to do is pass position arguments.

If you've got a ton of signal arguments, it's probably an indication
that you're doing something wrong.
Yes. For example, you're taking tons of independent arguments for
something that is logically a single entity, namely a block error event. ;-)

I'm almost sure that we'll want to add more things to this specific
event, for example a more detailed error description (Luiz once
suggested using errno here, but seems it hasn't made its way into
upstream). And I would be surprised if we never wanted to add more
information to other events, too.

You can pass structures as part of events.  For instance:

{ 'BlockIOEventInfo': { 'action': 'str', 'operation': 'str' } }
{ 'BlockIOEvent': { 'data': 'BlockIOEventInfo' } }

Which generates:

typedef struct BlockIOEventInfo
{
     const char *action;
     const char *operation;
} BlockIOEventInfo;

typedef void (BlockIOEventFunc)(BlockIOEventInfo *data, void *opaque);

There are some cases where this might make sense. For instance, device hotplug events might want to pass quite a bit of device info in the event.

It's just like designing any normal API. Sometimes you need to pass structs and sometimes adding more args is the right thing to do.

   From a protocol perspective, events look like this today:

{ "event": "BlockIOError", "data": { "device": "ide1-cd0", "operation":
"read", "action": "report" } }

The only change to the protocol I'm proposing is adding a "tag" field
which would give us:

{ "event": "BlockIOError", tag: "event023234", "data": { "device":
"ide1-cd0", "operation": "read", "action": "report" } }
Right, I was more referring to this schema thing. I didn't quite
understand yet if/why functions and events are the same thing from that
perspective.

They seem to be some kind of function that is called from within qemu,
gets its arguments from the event_connect call and returns its return
value to the QMP client.

Is that about right?

Close. It's a function that's called from within QEMU whereas the arguments to the function end up being sent as the payload for the event.

The arguments to connect allow the client to connect to very specific signals. For example, each BlockDriverState may have it's own BlockIOEvent signal. So:

struct BlockDriverState {
     // ...
     BlockIOEvent ioevent;
};

And then the various places would do:

block_io_event_signal(&bs->ioevent, "report", "read");

The connect operation is used to figure out which bs->ioevent to connect to. So it would look like:

block_io_event_connect("ide0-hd0", my_io_event_handler, &mystate);

We could eliminate the need for individual connect/disconnect operations if we had a global object model where you could describe objects in a unified hierarchy. For instance:

signal_connect("/blockdev/ide0-hd0", my_ioevent_handler, &mystate);

But this is a pretty big change that I fear is overengineering.

Another Example

Here's an example of BlockDeviceEvent, the successor to BlockIOEvent.
It makes use of signal accessor arguments and QAPI enumeration support:

So are we going to drop BlockIOEvent (or any other event that will be
extended) or will both old and new version continue to exist and we
always signal both of them?
Both have to exist.  We can't drop old events without breaking backwards
compatibility.
Right. So a second event doesn't sound like something we'd love to
have... Maybe extending the existing ones with optional arguments would
be better?

See the other thread about using optional arguments to extend. I think it's extremely hard to write a client if we use this model to do extensions. It basically forces you to work with variant types instead of native types.

Regards,

Anthony Liguori

Kevin





reply via email to

[Prev in Thread] Current Thread [Next in Thread]