RFC: Addon singleton support

[David Zeuthen]

On Tue, 2007-06-05 at 12:26 +0100, Rob Taylor wrote:
> >  1. when killing the s-addon, the runner dies
> > 
> >  2. when removing the last device [1] for an s-addon the runner also
> >     dies; might be related to item 1.
> >
> 
> Ah, yes, looks like I failed to update run_exited, I'll fix up the
> patch.. (and thanks for the pointer how to test for the last device
> removed case..)

Cool, I figured it was an easy fix.

> >  5. do you think it's necessary to 
> 
> to...?

Ah, I moved that below. Sorry for the confusion.

> > Do we really want that? I mean; s-addons are supposed to be used by
> > devices not permanently attached; so it may just happen that the last
> > device who needs an s-addon is removed; then we should kill the s-addon
> > just like we do for normal addon's.
> 
> The problem is that its hald-runner's job to kill addons, so to have
> hald-runner kill an s-addon would require the addon to communicate
> directly with it. IMHO this would just be adding more complexity to
> hald-runner than I'd like. The other option is to have hald tell
> hald-runner that the singleton is no longer needed with a KillSingleton
> method. I didn't go this way for fear of race conditions, but I think
> that would be possible, sending KillSingleton after the s-addon has
> responded to DeviceRemove.
> 
> However, either way it's adding more complexity to hald and hald-runner,
> and I don't really see the need as we need a s-addon to be well behaved
> anyhow.

Well, there are very real use cases here; you may be running expensive
(expensive as both CPU utilization / number of wakeups (cf. powertop)
and memory footprint) code only when one or more devices are attached -
for example, applications of this that come to mind

 - Bluez daemons
 - HPLip

So, for example, my devious plan for Fedora 8 is to change the Bluez
package such that instead of using a SysV init script it simply provides
an XML file to start the Bluez daemons as a singleton addon.

> > So the way I see it the question is this; should we wait for the
> > singleton addon to say "OK, I've poked the device I'm handling, go ahead
> > and add it"? Just like we do today for normal addon's; e.g. normal
> > addon's today call AddonIsReady() and this allows them to set some
> > properties before the device object in question is announced to the
> > world. 
> > 
> > I think we want that for singleton addon's too, e.g. your add_device()
> > function in hald/linux/addons/addon-input.c needs to poke the hal daemon
> > back. That way an s-addon can set properties on a device object. 
> > 
> > Thoughts?
> 
> Hmm, well it does poke the daemon back, its just an empty reply message,
> so setting device properties in the singleton_device_added callback
> would  have the desired effect, except for the fact the addon's been
> called with a blocking call. 

Right. That's pretty clever; didn't think of that.

> This raises a big problem I think we have
> in our dbus code - everything is done blocking, we don't use the
> dbus-glib mainloop integration to allow async callbacks.
> 
> Should I update the patch to at least do this for this particular call?

Yeah, I think that's exactly what we want; note that method call
forwarding to addon's claiming interfaces is also done non-blocking for
the very same reason. You probably want the same for DeviceRemove.

> >> <libhal property set API additions>
> > 
> > Are you sure we want to add all that API? I agree it's kind of.. well..
> > less than useful the way the API is right now with iterators and what
> > not. But it forces people to write code that only iterates over the
> > properties in a single sweep; picking up what they need. Any chance you
> > can change your code to do that? It seems inefficient to do an O(n)
> > lookup for the M properties you want.
> 
> My thought was actually just to change the implementation to use a
> hash-table rather than a list...

That would be pretty nice.

Cheers,
David