[libvirt] Re: [PATCH 07/12] Domain Events - remote driver

[Ben Guthro]

OK - it looks like my EventImpl was passing along the wrong bits.

I'll look into the "token" scheme suggested in an earlier email, and get this ready for re-submission.

Would you prefer a new patch series, as before - or another patch that modifies the prior series?




Daniel P. Berrange wrote on 10/20/2008 10:59 AM:
> On Mon, Oct 20, 2008 at 10:48:38AM -0400, Ben Guthro wrote:
>>> Just discovered one tiny problem here - need to check 'event' to see
>>> if the POLLHUP or POLLERR flags are set, and unregister the callback.
>>> Otherwise if you kill the server, the client will just spin on POLLHUP
>>> or ERR  forever. Something like this ought todo the trick
>>>
>>>     if (event & (POLLERR | POLLHUP)) {
>>>          virEventRemoveHandle(fd);
>>>          return;
>>>     }
>>>
>> I've been looking over the rest of your changes.
>> Generally, I agree all these suggestions are good ones...except for the code above
>>
>> With this code in, I run the following test
>> 1. start libvirtd
>> 2. begin to monitor events with event-test
>> 3. virsh create foo.xml
>>
>> At this point, the event-test app encounters a HUP, or ERR, and stops
>> monitoring for events - it will only ever get the "Started" event
> 
> That's a little odd - I'm not sure why 'event-test' would be getting
> a HUP/ERR when 'virsh' starts a domain.
> 
> 'event-test' should only get a HUP/ERR if it looses its socket connection
> to the libvirtd server. Once you've lost the connection like this, the 
> entire virConnectPtr is non-operative, and the client app needs to 
> create a new virConnectPtr to re-connect. So removing the FD from the
> event loop shouldn't result in us loosing any events - we're already
> doomed there.to 
> 
>> I handle this in the event-test poll loop via
>>
>> if ( pfd.revents & POLLHUP ) {
>>             DEBUG0("Reset by peer");
>>             return -1;
>>         }
> 
> The integration into the event loop though is only something the app
> will have general control off - eg, in the example libvirt-glib code
> I posted its totally opaque to the app.
> 
>> Is it not a reasonable restriction to require the client app to handle a Hangup?
> 
> Once the socket is dead, all subsequent libvirt call made on that 
> virConnectPtr object will throw errors, which the app should see. 
> Though if they're only ever waiting for events, and not making any
> other calls, they'd not see it. Perhaps we could do with an explicit
> callback for connection disconnect.
> 
> 
> Daniel