[PATCH] Add on demand functionality to bluetooth init script

[PATCH] Add on demand functionality to bluetooth init script

[Petr Lautrbach]

[-- Attachment #1: Type: text/plain, Size: 326 bytes --]

Hi.

This patch adds on demand functionality for bluetooth service based on udev events.
It's adapted from Fedora [1].

Idea is that bluetoothd doesn't need to be running unless the user actually has a bluetooth device.


[1] https://bugzilla.redhat.com/show_bug.cgi?id=484345

Cheers,

Petr
-- 
Petr Lautrbach, Red Hat, Inc.

[-- Attachment #2: bluetooth-ondemand-service-based-on-udev.patch --]
[-- Type: text/plain, Size: 4030 bytes --]

From 96c257e72a6a2ab5ef1325f98498817e0709600e Mon Sep 17 00:00:00 2001
From: Petr Lautrbach <plautrba@redhat.com>
Date: Thu, 4 Jun 2009 15:09:52 +0200
Subject: [PATCH] bluetooth ondemand service based on udev

---
 acinclude.m4                     |    6 ++++++
 scripts/Makefile.am              |   10 +++++++++-
 scripts/bluetooth-ondemand.rules |    3 +++
 scripts/bluetooth.init           |   24 ++++++++++++++++++++++--
 4 files changed, 40 insertions(+), 3 deletions(-)
 create mode 100644 scripts/bluetooth-ondemand.rules

diff --git a/acinclude.m4 b/acinclude.m4
index eb7cdeb..4082110 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -199,6 +199,7 @@ AC_DEFUN([AC_ARG_BLUEZ], [
 	configfiles_enable=yes
 	initscripts_enable=no
 	pcmciarules_enable=no
+	ondemand_enable=no
 	telephony_driver=dummy
 
 	AC_ARG_ENABLE(fortify, AC_HELP_STRING([--disable-fortify], [disable compile time buffer checks]), [
@@ -297,6 +298,10 @@ AC_DEFUN([AC_ARG_BLUEZ], [
 		pcmciarules_enable=${enableval}
 	])
 
+	AC_ARG_ENABLE(ondemand, AC_HELP_STRING([--enable-ondemand], [install ONDEMAND udev rules]), [
+		ondemand_enable=${enableval}
+	])
+
 	AC_ARG_ENABLE(debug, AC_HELP_STRING([--enable-debug], [enable compiling with debugging information]), [
 		debug_enable=${enableval}
 	])
@@ -357,4 +362,5 @@ AC_DEFUN([AC_ARG_BLUEZ], [
 	AM_CONDITIONAL(CONFIGFILES, test "${configfiles_enable}" = "yes")
 	AM_CONDITIONAL(INITSCRIPTS, test "${initscripts_enable}" = "yes")
 	AM_CONDITIONAL(PCMCIARULES, test "${pcmciarules_enable}" = "yes")
+	AM_CONDITIONAL(ONDEMAND, test "${ondemand_enable}" = "yes")
 ])
diff --git a/scripts/Makefile.am b/scripts/Makefile.am
index 494a9c2..4914612 100644
--- a/scripts/Makefile.am
+++ b/scripts/Makefile.am
@@ -11,6 +11,10 @@ endif
 if PCMCIARULES
 rules_DATA += bluetooth-serial.rules
 endif
+
+if ONDEMAND
+rules_DATA += bluetooth-ondemand.rules
+endif
 endif
 
 if PCMCIARULES
@@ -20,7 +24,8 @@ udev_SCRIPTS = bluetooth_serial
 endif
 
 EXTRA_DIST = bluetooth.init bluetooth.default bluetooth-hid2hci.rules \
-				bluetooth-serial.rules bluetooth_serial
+				bluetooth-serial.rules bluetooth_serial \
+				bluetooth-ondemand.rules
 
 MAINTAINERCLEANFILES = Makefile.in
 
@@ -30,6 +35,9 @@ install-data-local:
 	$(mkinstalldirs) $(DESTDIR)$(sysconfdir)/default
 	[ -f $(DESTDIR)$(sysconfdir)/default/bluetooth ] || \
 		$(INSTALL_DATA) $(srcdir)/bluetooth.default $(DESTDIR)$(sysconfdir)/default/bluetooth
+if ONDEMAND
+	echo "DAEMON_ONDEMAND=true" >> $(DESTDIR)$(sysconfdir)/default/bluetooth
+endif
 
 uninstall-local:
 	@rm -f $(DESTDIR)$(sysconfdir)/init.d/bluetooth
diff --git a/scripts/bluetooth-ondemand.rules b/scripts/bluetooth-ondemand.rules
new file mode 100644
index 0000000..6146935
--- /dev/null
+++ b/scripts/bluetooth-ondemand.rules
@@ -0,0 +1,3 @@
+#bluetooth on demand rules
+SUBSYSTEM=="bluetooth", ACTION=="add", RUN+="/etc/init.d/bluetooth condstart"
+SUBSYSTEM=="bluetooth", ACTION=="remove", RUN+="/etc/init.d/bluetooth condstop"
diff --git a/scripts/bluetooth.init b/scripts/bluetooth.init
index 3ea8a89..b9a1454 100644
--- a/scripts/bluetooth.init
+++ b/scripts/bluetooth.init
@@ -17,21 +17,41 @@ DAEMON_ENABLE=true
 
 [ -e /etc/default/bluetooth ] && . /etc/default/bluetooth
 
+has_bt_devices()
+{
+	#Look for Bluetooth adapters:
+	udevadm info --export-db | grep -q -e '/devices/.*/bluetooth/.*'
+
+	return $?
+}
+
 case "$1" in
   start)
 	echo -n "Starting $DESC:"
 	if $DAEMON_ENABLE && [ -x "$DAEMON_EXEC" ]; then
-		$DAEMON_EXEC
-		echo -n " $DAEMON_NAME"
+		if [ -z $DAEMON_ONDEMAND ] || has_bt_devices; then
+			$DAEMON_EXEC
+			echo -n " $DAEMON_NAME"
+		fi
 	fi
 	echo "."
 	;;
+  condstart)
+	if [ -n $DAEMON_ONDEMAND ]; then
+		/etc/init.d/$NAME start
+	fi
+	;;
   stop)
 	echo -n "Stopping $DESC:"
 	killall $DAEMON_NAME > /dev/null 2>&1 || true
 	echo -n " $DAEMON_NAME"
 	echo "."
 	;;
+  condstop)
+	if [ -n $DAEMON_ONDEMAND ]; then
+		has_bt_devices || /etc/init.d/$NAME stop
+	fi
+	;;
   *)
 	N=/etc/init.d/$NAME
 	echo "Usage: $N {start|stop}" >&2
-- 
1.6.0.6

Re: [PATCH] Add on demand functionality to bluetooth init script

[Stefan Seyfried]

On Thu, Jun 04, 2009 at 04:52:46PM +0200, Petr Lautrbach wrote:
> Hi.
>
> This patch adds on demand functionality for bluetooth service based on udev events.
> It's adapted from Fedora [1].

JFTR: I'd appreciate if this (or something similar) was added. I have a similar hack in
the openSUSE package (though not as elaborate as this one), and for consistency across
distributions, having a solution upstream is preferred.

> Idea is that bluetoothd doesn't need to be running unless the user actually has a bluetooth device.

Just out of interest: how did you solve the "udev event for bluetooth adapter
fires before dbus is started" probem?

Best regards,

	Stefan
-- 
Stefan Seyfried
R&D Team Mobile Devices            |              "Any ideas, John?"
SUSE LINUX Products GmbH, Nürnberg | "Well, surrounding them's out." 

This footer brought to you by insane German lawmakers:
SUSE Linux Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg)

Re: [PATCH] Add on demand functionality to bluetooth init script

[Petr Lautrbach]

On 06/05/2009 04:51 PM, Stefan Seyfried wrote:
> On Thu, Jun 04, 2009 at 04:52:46PM +0200, Petr Lautrbach wrote:
>> Hi.
>>
>> This patch adds on demand functionality for bluetooth service based on udev events.
>> It's adapted from Fedora [1].
>
> JFTR: I'd appreciate if this (or something similar) was added. I have a similar hack in
> the openSUSE package (though not as elaborate as this one), and for consistency across
> distributions, having a solution upstream is preferred.	
>
>> Idea is that bluetoothd doesn't need to be running unless the user actually has a bluetooth device.
>
> Just out of interest: how did you solve the "udev event for bluetooth adapter
> fires before dbus is started" probem?
>

"udev event for bluetooth adapter fires before dbus is started" can happend in these cases:

boot process or changing runlevel:
udev is started, dbus is not started yet and somebody inserts bluetooth device ->
udev calls "/etc/init.d/bluetooth start" which fails

but there is running boot (changing runlevel) process, so sometime after starting dbus is started
bluetooth again, this time successfully


running system without dbus:
bluetoothd will not run even without this feature


Regards,

Petr
-- 
Petr Lautrbach, Red Hat, Inc.

Re: [PATCH] Add on demand functionality to bluetooth init script

[Stefan Seyfried]

Petr Lautrbach wrote:

> but there is running boot (changing runlevel) process, so sometime after starting dbus is started
> bluetooth again, this time successfully

Triggered by what? A init script that is always enabled? Is it checking if
there is an adapter present? If not, it's unconditionally always starting
bluetoothd (and thus the ondemand functionality is not strictly needed ;)

Best regards,

	Stefan

-- 
Stefan Seyfried
R&D Team Mobile Devices            |              "Any ideas, John?"
SUSE LINUX Products GmbH, Nürnberg | "Well, surrounding them's out."

This footer brought to you by insane German lawmakers:
SUSE Linux Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg)

Re: [PATCH] Add on demand functionality to bluetooth init script

[Petr Lautrbach]

On 06/08/2009 01:59 PM, Stefan Seyfried wrote:
> Petr Lautrbach wrote:
>
>> but there is running boot (changing runlevel) process, so sometime after starting dbus is started
>> bluetooth again, this time successfully
>
> Triggered by what? A init script that is always enabled? Is it checking if
> there is an adapter present? If not, it's unconditionally always starting
> bluetoothd (and thus the ondemand functionality is not strictly needed ;)
>

triggered by this code:

+has_bt_devices()
+{
+	#Look for Bluetooth adapters:
+	udevadm info --export-db | grep -q -e '/devices/.*/bluetooth/.*'
+
+	return $?
+}
+
  case "$1" in
    start)
  	echo -n "Starting $DESC:"
  	if $DAEMON_ENABLE && [ -x "$DAEMON_EXEC" ]; then
-		$DAEMON_EXEC
-		echo -n " $DAEMON_NAME"
+		if [ -z $DAEMON_ONDEMAND ] || has_bt_devices; then
+			$DAEMON_EXEC
+			echo -n " $DAEMON_NAME"
+		fi


It starts if $DAEMON_ONDEMAND is not set or if there are some bluetooth adapters.
In case that $DAEMON_ONDEMAND is set and has_bt_devices() has returned non-zero, it won't start.

Petr
-- 
Petr Lautrbach, Red Hat, Inc.

Re: [PATCH] Add on demand functionality to bluetooth init script

[Johan Hedberg]

Hi Petr,

On Thu, Jun 04, 2009, Petr Lautrbach wrote:
> This patch adds on demand functionality for bluetooth service based on
> udev events.  It's adapted from Fedora [1].
>
> Idea is that bluetoothd doesn't need to be running unless the user
> actually has a bluetooth device.

The patch looks good to me (though my udev knowledge is rather limited).
However, I'd like to get a second opinion from Marcel before pushing
upstream.

Johan

Re: [PATCH] Add on demand functionality to bluetooth init script

[Marcel Holtmann]

Hi Johan,

> > This patch adds on demand functionality for bluetooth service based on
> > udev events.  It's adapted from Fedora [1].
> >
> > Idea is that bluetoothd doesn't need to be running unless the user
> > actually has a bluetooth device.
> 
> The patch looks good to me (though my udev knowledge is rather limited).
> However, I'd like to get a second opinion from Marcel before pushing
> upstream.

first of all the question from Stefan needs to be handled on how we
handle the case for udev event before D-Bus system daemon is started.

Personally I think having an extra init script style for this makes no
sense and just adds complexity in the start/stop process of the daemon.

I might prefer if we add special bluetoothd --udev={start,stop} handling
to it (including polling code for D-Bus system daemon availability).

Regards

Marcel

Re: [PATCH] Add on demand functionality to bluetooth init script

[Stefan Seyfried]

On Fri, Jun 05, 2009 at 05:48:26PM +0200, Marcel Holtmann wrote:

> first of all the question from Stefan needs to be handled on how we
> handle the case for udev event before D-Bus system daemon is started.
> 
> Personally I think having an extra init script style for this makes no
> sense and just adds complexity in the start/stop process of the daemon.
> 
> I might prefer if we add special bluetoothd --udev={start,stop} handling
> to it (including polling code for D-Bus system daemon availability).

I actually solved it like this (no, I'm not proud of it, but it works):

* instead of calling "/etc/init.d/bluetooth condstart" from udev, I call
  a bluetooth.sh which basically does:

  mkdir /dev/shm/bluetooth-adapter-present # this is just a marker
  /bin/dbus-send --system --type=method_call --print-reply \
	--reply-timeout=1000 --dest=org.bluez / org.bluez.hello

  If D-Bus is already up (adapter hotplug), then the dbus-send activates
  bluetoothd via dbus-activation.

* I have a second, trivial init script which is always enabled and runs
  at the end of the boot process (after D-Bus start, that is) and which
  just does

  [ -x /dev/shm/bluetooth-adapter-present ] && \
	/etc/init.d/bluetooth start

  This handles the "adapter was plugged in before D-Bus was ready" case.

It's not particularly pretty, and I do not want to suggest that this is
a correct solution, but it solved the problem nicely for me.

Regards,

	Stefan
-- 
Stefan Seyfried
R&D Team Mobile Devices            |              "Any ideas, John?"
SUSE LINUX Products GmbH, Nürnberg | "Well, surrounding them's out." 

This footer brought to you by insane German lawmakers:
SUSE Linux Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg)

Re: [PATCH] Add on demand functionality to bluetooth init script

[Stefan Seyfried]

On Fri, Jun 05, 2009 at 06:42:00PM +0200, Stefan Seyfried wrote:
>   [ -x /dev/shm/bluetooth-adapter-present ] && \

"test -e" of course

> 	/etc/init.d/bluetooth start

Regards,

	Stefan
-- 
Stefan Seyfried
R&D Team Mobile Devices            |              "Any ideas, John?"
SUSE LINUX Products GmbH, Nürnberg | "Well, surrounding them's out." 

This footer brought to you by insane German lawmakers:
SUSE Linux Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg)