[libvirt] [RFC] making (newly public) EventImpl interface moreconsistent

[David Lively]

Ugh.  I see what you mean.  It seems more complicated than that, though.
For one thing, the AvahiWatch/AvahiPoll stuff is sufficiently abstract
that it doesn't mention DBusConnection (or any other DBus type, AFAIK),
so there's no indication that *any* DBusConnection is being used (though
I trust that there is).  Turning on AVAHI_DEBUG, I see both my
(node_device_hal.c) callbacks and the avahi callbacks being called (but
at different times, and with different fds and event sets).  Makes me
wonder whether the avahi library is using dbus_bus_get_private() to get
its own DBusConnection that it doesn't have to worry about sharing with
others.

I tried changing the node_device_hal code to use a private dbus
connection, and see exactly the same behavior (i.e., dbus immediately
registers the same fd twice, with different event sets and enable
state).  Regardless, I like the idea of simply using this private
connection (as you suggested but rejected), just for the sake of
simplicity.  (Also, what if Avahi's been configured out?)

But ... back to the original problem.  Immediately after calling
dbus_set_watch_functions (and *before* initializing the Avahi stuff), I
see two callbacks to watch_add with the same fd.  Looking at the glib
watch functions, I see watches on GIOChannels are added with
g_io_add_watch(), which indeed doesn't specify what happens if the same
GIOChannel is added twice.  But GIOChannels aren't fds.  Presumably, one
could create two different GIOChannels with the same fd (via
g_io_channel_unix_new) and then register watches on each of them.  So I
remain convinced that we need to support the registration of the same fd
multiple times ...

Dave

[Caveat: Okay, so I've never actually *used* glib ... I'm going from the
docs here ...]


On Wed, 2008-11-05 at 16:49 +0000, Daniel P. Berrange wrote:
> On Wed, Nov 05, 2008 at 11:26:07AM -0500, David Lively wrote:
> > On Wed, 2008-11-05 at 11:51 +0000, Daniel P. Berrange wrote:
> I think the problem here is that the existing Avahi mdns code in the
> libvirtd daemon is also already using DBus, and has already setup the
> DBus system bus instances to integrate with the libvirtd event loop.
> 
> By default, there is a single DBus system bus connection in the app
> which is shared amongst all users. The DBus API spec for its watch 
> functions mandates that the application only setup watches once per
> connection, so having both the Avahi and HAL/DevKit code in libvirt 
> call dbus_connection_set_watch_functions() against the shared connection
> is in fact a bug. For added fun, PolicyKit code in the daemon also makes
> use of DBus, but doesn't (yet) need callbacks so hasn't done mainloop
> integration.
> 
> So the flaw here is that the individual drivers should not all be
> attempting to setup integration with the shared dbus system bus
> connection. Either each driver should use a private dbus system bus
> conenction, or the main daemon startup code should take responsibility
> for integrating the shared connection instance into its main loop.
> 
> I'd be inclined to go for the latter. For simplicitly, I think you
> ought to simply be able to remove the dbus_connection_set_watch_functions
> call from the driver, and rely on fact that Avahi code has already done
> this.
> 
> Daniel