Skip to contentSkip to conversation

package/wpa_supplicant: Fix systemd service for new DBus bus name

PATCH Superseded Marcin Niestroj 3 participants 5 messages Apr 12, 2016 – Apr 13, 2016

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

PATCH v3 1/1 package/wpa_supplicant: Fix systemd service for new DBus bus name

systemd service files were supplied with old DBus bus name. After service activation systemd was waiting for appearance of specified bus name to consider it started successfully. However, if wpa_supplicant was compiled only with new dbus interface name, then systemd didn't notice configured (old) DBus bus name appearance. In the end service was considered malfunctioning and it was deactivated.

Add upstream patch, which updates systemd service BusName property according to configured DBus interface version.

---
Hi,

Sorry for posting this patch as v2 moment ago. I'm just resending it with proper version (v3) and updated changelog.

Changes v2 -> v3:
* Update subject line and commit description to make the problem more clear (suggested by Arnout)
* Use upstreamable patch instead of change in buildroot package makefile (suggested by Arnout)

Changes v1 -> v2:
* Added missing signed-off tag

 ...ate-service-files-according-to-dbus-inter.patch | 77 ++++++++++++++++++++++
 1 file changed, 77 insertions(+)
 create mode 100644 package/wpa_supplicant/0007-systemd-Update-service-files-according-to-dbus-inter.patch
package/wpa_supplicant/0007-systemd-Update-service-files-according-to-dbus-inter.patch new file +77 -0
new file mode 100644
index 0000000..59c5450
@@ -0,0 +1,77 @@
+From b6cea24d6191d9ccdcd1ac38a5322e3da73218db Mon Sep 17 00:00:00 2001
+From: Marcin Niestroj <[email protected]>
+Date: Mon, 11 Apr 2016 13:23:54 +0200
+Subject: [PATCH] systemd: Update service files according to dbus interface
+ version used
+
+systemd service files were supplied with old DBus bus name. After
+service activation systemd was waiting for appearance of specified bus
+name to consider it started successfully. However, if wpa_supplicant was
+compiled only with new dbus interface name, then systemd didn't notice
+configured (old) DBus bus name appearance. In the end service was
+considered malfunctioning and it was deactivated.
+
+Update systemd service BusName property according to supported DBus
+interface version.
+
+Signed-off-by: Marcin Niestroj <[email protected]>
+---
+ wpa_supplicant/Makefile                          | 8 ++++++--
+ wpa_supplicant/systemd/wpa_supplicant.service.in | 4 ++--
+ 2 files changed, 8 insertions(+), 4 deletions(-)
+
+diff --git a/wpa_supplicant/Makefile b/wpa_supplicant/Makefile
+index ad9ead9..fbd1b25 100644
+--- a/wpa_supplicant/Makefile
++++ b/wpa_supplicant/Makefile
+@@ -1374,6 +1374,7 @@ ifndef DBUS_INCLUDE
+ DBUS_INCLUDE := $(shell $(PKG_CONFIG) --cflags dbus-1)
+ endif
+ DBUS_CFLAGS += $(DBUS_INCLUDE)
++DBUS_INTERFACE=fi.epitest.hostap.WPASupplicant
+ endif
+ 
+ ifdef CONFIG_CTRL_IFACE_DBUS_NEW
+@@ -1399,6 +1400,7 @@ DBUS_OBJS += dbus/dbus_new_introspect.o
+ DBUS_CFLAGS += -DCONFIG_CTRL_IFACE_DBUS_INTRO
+ endif
+ DBUS_CFLAGS += $(DBUS_INCLUDE)
++DBUS_INTERFACE=fi.w1.wpa_supplicant1
+ endif
+ 
+ ifdef DBUS
+@@ -1760,11 +1762,13 @@ else
+ endif
+ 
+ %.service: %.service.in
+-	$(Q)sed -e 's|\@BINDIR\@|$(BINDIR)|g' $< >$@
++	$(Q)sed -e 's|\@BINDIR\@|$(BINDIR)|g' \
++		-e 's|\@DBUS_INTERFACE\@|$(DBUS_INTERFACE)|g' $< >$@
+ 	@$(E) "  sed" $<
+ 
+ %@.service: %.service.arg.in
+-	$(Q)sed -e 's|\@BINDIR\@|$(BINDIR)|g' $< >$@
++	$(Q)sed -e 's|\@BINDIR\@|$(BINDIR)|g' \
++		-e 's|\@DBUS_INTERFACE\@|$(DBUS_INTERFACE)|g' $< >$@
+ 	@$(E) "  sed" $<
+ 
+ wpa_supplicant.exe: wpa_supplicant
+diff --git a/wpa_supplicant/systemd/wpa_supplicant.service.in b/wpa_supplicant/systemd/wpa_supplicant.service.in
+index ea964ce..bc5d49a 100644
+--- a/wpa_supplicant/systemd/wpa_supplicant.service.in
++++ b/wpa_supplicant/systemd/wpa_supplicant.service.in
+@@ -5,9 +5,9 @@ Wants=network.target
+ 
+ [Service]
+ Type=dbus
+-BusName=fi.epitest.hostap.WPASupplicant
++BusName=@DBUS_INTERFACE@
+ ExecStart=@BINDIR@/wpa_supplicant -u
+ 
+ [Install]
+ WantedBy=multi-user.target
+-Alias=dbus-fi.epitest.hostap.WPASupplicant.service
++Alias=dbus-@[email protected]
+-- 
+2.8.0
+
Signature · 1 line
2.8.0
#1 lore raw

PATCH v3 1/1 package/wpa_supplicant: Fix systemd service for new DBus bus name

Quoted 8 lines · Marcin Niestroj

systemd service files were supplied with old DBus bus name. After service activation systemd was waiting for appearance of specified bus name to consider it started successfully. However, if wpa_supplicant was compiled only with new dbus interface name, then systemd didn't notice configured (old) DBus bus name appearance. In the end service was considered malfunctioning and it was deactivated.

Add upstream patch, which updates systemd service BusName property

It's not an upstream patch, it's an upstreamable patch. At least, I can't find
it in the upstream git repo nor on the mailing list. Did you even send it upstream already?

Quoted 33 lines

according to configured DBus interface version.

---
Hi,

Sorry for posting this patch as v2 moment ago. I'm just resending it with proper version (v3) and updated changelog.

Changes v2 -> v3:
* Update subject line and commit description to make the problem more clear (suggested by Arnout)
* Use upstreamable patch instead of change in buildroot package makefile (suggested by Arnout)

Changes v1 -> v2:
* Added missing signed-off tag

  ...ate-service-files-according-to-dbus-inter.patch | 77 ++++++++++++++++++++++
  1 file changed, 77 insertions(+)

create mode 100644 package/wpa_supplicant/0007-systemd-Update-service-files-according-to-dbus-inter.patch

package/wpa_supplicant/0007-systemd-Update-service-files-according-to-dbus-inter.patch new file +5 -0
new file mode 100644
index 0000000..59c5450
@@ -0,0 +1,77 @@
+From b6cea24d6191d9ccdcd1ac38a5322e3da73218db Mon Sep 17 00:00:00 2001
+From: Marcin Niestroj <[email protected]>
+Date: Mon, 11 Apr 2016 13:23:54 +0200
+Subject: [PATCH] systemd: Update service files according to dbus interface
+ version used

package/wpa_supplicant/0007-systemd-Update-service-files-according-to-dbus-inter.patch line 15

Quoted 10 lines
package/wpa_supplicant/0007-systemd-Update-service-files-according-to-dbus-inter.patch new file +10 -0
+
+systemd service files were supplied with old DBus bus name. After
+service activation systemd was waiting for appearance of specified bus
+name to consider it started successfully. However, if wpa_supplicant was
+compiled only with new dbus interface name, then systemd didn't notice
+configured (old) DBus bus name appearance. In the end service was
+considered malfunctioning and it was deactivated.
+
+Update systemd service BusName property according to supported DBus
+interface version.

If you send this patch upstream before posting a v4, please include a
reference to the upstream mailing list message (or better yet, the commit :-)

Quoted 6 lines
package/wpa_supplicant/0007-systemd-Update-service-files-according-to-dbus-inter.patch new file +6 -0
+
+Signed-off-by: Marcin Niestroj <[email protected]>
+---
+ wpa_supplicant/Makefile                          | 8 ++++++--
+ wpa_supplicant/systemd/wpa_supplicant.service.in | 4 ++--
+ 2 files changed, 8 insertions(+), 4 deletions(-)

package/wpa_supplicant/0007-systemd-Update-service-files-according-to-dbus-inter.patch line 31

Quoted 10 lines
package/wpa_supplicant/0007-systemd-Update-service-files-according-to-dbus-inter.patch new file +10 -0
+
+diff --git a/wpa_supplicant/Makefile b/wpa_supplicant/Makefile
+index ad9ead9..fbd1b25 100644
+--- a/wpa_supplicant/Makefile
++++ b/wpa_supplicant/Makefile
+@@ -1374,6 +1374,7 @@ ifndef DBUS_INCLUDE
+ DBUS_INCLUDE := $(shell $(PKG_CONFIG) --cflags dbus-1)
+ endif
+ DBUS_CFLAGS += $(DBUS_INCLUDE)
++DBUS_INTERFACE=fi.epitest.hostap.WPASupplicant

Coding style in this file is with spaces around the =

Otherwise, the patch looks good to me. Since the spaces are just a triviality:

Regards,
Arnout

Quoted 47 lines
package/wpa_supplicant/0007-systemd-Update-service-files-according-to-dbus-inter.patch new file +46 -0
+ endif
+
+ ifdef CONFIG_CTRL_IFACE_DBUS_NEW
+@@ -1399,6 +1400,7 @@ DBUS_OBJS += dbus/dbus_new_introspect.o
+ DBUS_CFLAGS += -DCONFIG_CTRL_IFACE_DBUS_INTRO
+ endif
+ DBUS_CFLAGS += $(DBUS_INCLUDE)
++DBUS_INTERFACE=fi.w1.wpa_supplicant1
+ endif
+
+ ifdef DBUS
+@@ -1760,11 +1762,13 @@ else
+ endif
+
+ %.service: %.service.in
+-	$(Q)sed -e 's|\@BINDIR\@|$(BINDIR)|g' $< >$@
++	$(Q)sed -e 's|\@BINDIR\@|$(BINDIR)|g' \
++		-e 's|\@DBUS_INTERFACE\@|$(DBUS_INTERFACE)|g' $< >$@
+ 	@$(E) "  sed" $<
+
+ %@.service: %.service.arg.in
+-	$(Q)sed -e 's|\@BINDIR\@|$(BINDIR)|g' $< >$@
++	$(Q)sed -e 's|\@BINDIR\@|$(BINDIR)|g' \
++		-e 's|\@DBUS_INTERFACE\@|$(DBUS_INTERFACE)|g' $< >$@
+ 	@$(E) "  sed" $<
+
+ wpa_supplicant.exe: wpa_supplicant
+diff --git a/wpa_supplicant/systemd/wpa_supplicant.service.in b/wpa_supplicant/systemd/wpa_supplicant.service.in
+index ea964ce..bc5d49a 100644
+--- a/wpa_supplicant/systemd/wpa_supplicant.service.in
++++ b/wpa_supplicant/systemd/wpa_supplicant.service.in
+@@ -5,9 +5,9 @@ Wants=network.target
+
+ [Service]
+ Type=dbus
+-BusName=fi.epitest.hostap.WPASupplicant
++BusName=@DBUS_INTERFACE@
+ ExecStart=@BINDIR@/wpa_supplicant -u
+
+ [Install]
+ WantedBy=multi-user.target
+-Alias=dbus-fi.epitest.hostap.WPASupplicant.service
++Alias=dbus- at [email protected]
+--
+2.8.0
+
Signature · 6 lines
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
#2 lore raw

PATCH v3 1/1 package/wpa_supplicant: Fix systemd service for new DBus bus name

Hi Arnout,

Quoted 8 lines · Arnout Vandecappelle
Quoted 7 lines · Marcin Niestroj

systemd service files were supplied with old DBus bus name. After service activation systemd was waiting for appearance of specified bus name to consider it started successfully. However, if wpa_supplicant was compiled only with new dbus interface name, then systemd didn't notice configured (old) DBus bus name appearance. In the end service was considered malfunctioning and it was deactivated.

Quoted 5 lines
Quoted 1 line

Add upstream patch, which updates systemd service BusName property

It's not an upstream patch, it's an upstreamable patch. At least, I can't
find it in the upstream git repo nor on the mailing list. Did you even send it upstream already?

It's at http://lists.infradead.org/pipermail/hostap/2016-April/035599.html.

baruch

Signature · 3 lines
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
#3 lore raw

PATCH v3 1/1 package/wpa_supplicant: Fix systemd service for new DBus bus name

In reply to Arnout Vandecappelle

Hi,

Quoted 8 lines · Arnout Vandecappelle
Quoted 7 lines · Marcin Niestroj

systemd service files were supplied with old DBus bus name. After service activation systemd was waiting for appearance of specified bus name to consider it started successfully. However, if wpa_supplicant was compiled only with new dbus interface name, then systemd didn't notice configured (old) DBus bus name appearance. In the end service was considered malfunctioning and it was deactivated.

Quoted 5 lines
Quoted 1 line

Add upstream patch, which updates systemd service BusName property

It's not an upstream patch, it's an upstreamable patch. At least, I
can't find it in the upstream git repo nor on the mailing list. Did you even send it upstream already?

Sorry for misleading here, I will update description. As Baruch wrote earlier, it's already been sent to mailing list.

Quoted 14 lines
Quoted 14 lines
package/wpa_supplicant/0007-systemd-Update-service-files-according-to-dbus-inter.patch new file +4 -0
@@ -0,0 +1,77 @@
+From b6cea24d6191d9ccdcd1ac38a5322e3da73218db Mon Sep 17 00:00:00 2001
+From: Marcin Niestroj <[email protected]>
+Date: Mon, 11 Apr 2016 13:23:54 +0200
+Subject: [PATCH] systemd: Update service files according to dbus

interface
+ version used

package/wpa_supplicant/0007-systemd-Update-service-files-according-to-dbus-inter.patch new file +6 -0
+
+systemd service files were supplied with old DBus bus name. After
+service activation systemd was waiting for appearance of specified bus
+name to consider it started successfully. However, if wpa_supplicant was
+compiled only with new dbus interface name, then systemd didn't notice
+configured (old) DBus bus name appearance. In the end service was
Quoted 8 lines

package/wpa_supplicant/0007-systemd-Update-service-files-according-to-dbus-inter.patch line 15

Quoted 4 lines
package/wpa_supplicant/0007-systemd-Update-service-files-according-to-dbus-inter.patch new file +4 -0
+considered malfunctioning and it was deactivated.
+
+Update systemd service BusName property according to supported DBus
+interface version.

If you send this patch upstream before posting a v4, please include a
reference to the upstream mailing list message (or better yet, the commit :-)

Thanks for pointing it, I will add the reference.

Quoted 20 lines
Quoted 7 lines
package/wpa_supplicant/0007-systemd-Update-service-files-according-to-dbus-inter.patch new file +6 -0
+
+Signed-off-by: Marcin Niestroj <[email protected]>
+---
+ wpa_supplicant/Makefile                          | 8 ++++++--
+ wpa_supplicant/systemd/wpa_supplicant.service.in | 4 ++--
+ 2 files changed, 8 insertions(+), 4 deletions(-)

package/wpa_supplicant/0007-systemd-Update-service-files-according-to-dbus-inter.patch line 31

Quoted 10 lines
package/wpa_supplicant/0007-systemd-Update-service-files-according-to-dbus-inter.patch new file +10 -0
+
+diff --git a/wpa_supplicant/Makefile b/wpa_supplicant/Makefile
+index ad9ead9..fbd1b25 100644
+--- a/wpa_supplicant/Makefile
++++ b/wpa_supplicant/Makefile
+@@ -1374,6 +1374,7 @@ ifndef DBUS_INCLUDE
+ DBUS_INCLUDE := $(shell $(PKG_CONFIG) --cflags dbus-1)
+ endif
+ DBUS_CFLAGS += $(DBUS_INCLUDE)
++DBUS_INTERFACE=fi.epitest.hostap.WPASupplicant

Coding style in this file is with spaces around the =

Quoted 4 lines

Otherwise, the patch looks good to me. Since the spaces are just a
triviality:

As I am newbie in mailing list workflow: does this tag mean, that I have permission to add it to v4 patch directly (if I fix all pointed issues)? Or it's just for other reviewers, so they know what has been already reviewed in previous version of patch?

Regards,
Marcin

Quoted 54 lines

Regards,
Arnout

Quoted 48 lines

+ endif

package/wpa_supplicant/0007-systemd-Update-service-files-according-to-dbus-inter.patch new file +27 -0
+
+ ifdef CONFIG_CTRL_IFACE_DBUS_NEW
+@@ -1399,6 +1400,7 @@ DBUS_OBJS += dbus/dbus_new_introspect.o
+ DBUS_CFLAGS += -DCONFIG_CTRL_IFACE_DBUS_INTRO
+ endif
+ DBUS_CFLAGS += $(DBUS_INCLUDE)
++DBUS_INTERFACE=fi.w1.wpa_supplicant1
+ endif
+
+ ifdef DBUS
+@@ -1760,11 +1762,13 @@ else
+ endif
+
+ %.service: %.service.in
+-    $(Q)sed -e 's|\@BINDIR\@|$(BINDIR)|g' $< >$@
++    $(Q)sed -e 's|\@BINDIR\@|$(BINDIR)|g' \
++        -e 's|\@DBUS_INTERFACE\@|$(DBUS_INTERFACE)|g' $< >$@
+     @$(E) "  sed" $<
+
+ %@.service: %.service.arg.in
+-    $(Q)sed -e 's|\@BINDIR\@|$(BINDIR)|g' $< >$@
++    $(Q)sed -e 's|\@BINDIR\@|$(BINDIR)|g' \
++        -e 's|\@DBUS_INTERFACE\@|$(DBUS_INTERFACE)|g' $< >$@
+     @$(E) "  sed" $<
+
+ wpa_supplicant.exe: wpa_supplicant
+diff --git a/wpa_supplicant/systemd/wpa_supplicant.service.in

b/wpa_supplicant/systemd/wpa_supplicant.service.in

package/wpa_supplicant/0007-systemd-Update-service-files-according-to-dbus-inter.patch new file +18 -0
+index ea964ce..bc5d49a 100644
+--- a/wpa_supplicant/systemd/wpa_supplicant.service.in
++++ b/wpa_supplicant/systemd/wpa_supplicant.service.in
+@@ -5,9 +5,9 @@ Wants=network.target
+
+ [Service]
+ Type=dbus
+-BusName=fi.epitest.hostap.WPASupplicant
++BusName=@DBUS_INTERFACE@
+ ExecStart=@BINDIR@/wpa_supplicant -u
+
+ [Install]
+ WantedBy=multi-user.target
+-Alias=dbus-fi.epitest.hostap.WPASupplicant.service
++Alias=dbus- at [email protected]
+--
+2.8.0
+
Signature · 1 line
Marcin Niestroj
#4 lore raw

PATCH v3 1/1 package/wpa_supplicant: Fix systemd service for new DBus bus name

Quoted 3 lines · Marcin Niestroj

Hi,

On 12.04.2016 23:47, Arnout Vandecappelle wrote:

[snip]

Quoted 9 lines
Quoted 4 lines

Otherwise, the patch looks good to me. Since the spaces are just a
triviality:

As I am newbie in mailing list workflow: does this tag mean, that I have permission to add it to v4 patch directly (if I fix all pointed issues)? Or it's just for other reviewers, so they know what has been
already reviewed in previous version of patch?

It means that you can add my Reviewed-by tag to your next iteration, on
condition that you don't change anything except the things I mentioned.

Regards,
Arnout

Signature · 6 lines
Arnout Vandecappelle      arnout dot vandecappelle at essensium dot com
Senior Embedded Software Architect . . . . . . +32-478-010353 (mobile)
Essensium, Mind division . . . . . . . . . . . . . . http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium . . . . . BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF