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

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
> 


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