Re: [libvirt] Re: [PATCH 04/12] Domain Events - rpc changes
This is very similar to what I had in the original patch,
where on the server side, we just increment/decrement a callback_registered counter,
then keep the list of callbacks/opaque data on the client.
The server would then send one rpc to each connected client, whose job it would be
to multiplex this out to all registered callbacks.
There would still be a "one-for-one" for register/deregister, but this scheme
has the following advantages for efficency:
a.) Fewer RPC calls (less data on the wire - one-to-many for events firing)
b.) Less RPC data passed on register/deregister (no need for cb/user_data)
c.) Code is simpler - no need to invent yet another data structure, and list for tracking
tokens
Daniel - what are your thoughts on this?
This is similar to what I had originally implemented, but I'm not sure if you objected
to this part of it, or not...
Ben
David Lively wrote on 10/20/2008 04:05 PM:
On Sun, 2008-10-19 at 20:22 +0100, Daniel P. Berrange wrote:
> On Fri, Oct 17, 2008 at 11:58:15AM -0400, Ben Guthro wrote:
>> Changes to the RPC protocol
>>
>> +struct remote_domain_event_ret {
>> + remote_nonnull_domain dom;
>> + int event;
>> + unsigned long int callback;
>> + unsigned long int user_data;
>> +};
> Using a 'unsigned long int' field to transmit the raw pointer feels a little
> wrong to me. Could we have the client side pass a simple integer 'token'
when
> registering / unregistering, and have that 'token' passed back by the server
> in the actual event. The client could use this token to lookup the callback
> and user_data.
Hold on. We can (and IMO should) quite easily avoid both this lookup
and the passing of the callback pointer to the server:
Suppose we have the same client registered for two different domain
event callbacks. In the current patch, the server will send two RPCs
per event, one for each callback (which the client then unmarshals,
casts, and calls).
But what if we sent just one RPC per event (& per client) and had the
client walk its list of callbacks (which we'll need to track on the
client side anyway, if we're not sending the data over the wire)? We
*always* make *all* the callbacks on the list, so there's no point in
making individual RPCs to fire off each callback individually. This
gets rid of the need to send callback/user_data over the wire, and also
doesn't require tokenization (which is all just extra overhead in this
case).
remote_domain_events_register/deregister_args structs will go away.
remoteDispatchhDomainEventsRegister/Deregister will simply inc/dec a
value, sending events only when the value is >0.
While I'm not sure I've described this very well, I feel pretty strongly
that it's the right way to go. If my explanation isn't clear, please
get back to me ...
Dave