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

[Daniel P. Berrange]

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
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|