I'll answer for Dave, while I'm looking at this.

As far as I know, Dave is of the opinion that we are just "getting lucky" using the APIs as we are, and remains convinced that his suggested change is necessary here.

He (and I) remain worried that release of the EventImpl API without this API change could get us into trouble in the future, as we would have to support the released API that has different semantics than DBus, which we were supposed to be modeled closely to.

You had sounded convinced it was not necessary the last we heard though...and ultimatley we don't have checkin permissions...so we'll go with whatever you guys decide.

I'll let Dave follow up with any additional examples of where this would get us in trouble...


-----Original Message-----
From: Daniel P. Berrange [mailto:berrange@redhat.com]
Sent: Fri 11/14/2008 6:27 AM
To: Dave Lively
Cc: Ben Guthro; libvir-list
Subject: Re: [libvirt] [RFC] making (newly public) EventImpl interface moreconsistent

On Wed, Nov 05, 2008 at 05:32:36PM -0500, David Lively wrote:
> 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 ...

Did you ever get to the bottom of this problem. If not, then I think
the only safe thing todo for this release is to change the signature
of AddHandle as you suggest, so we explicitly track FDs using an
integer ID that's independant of the FD number.

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 :|