fff-hoods: remove dependency to /tmp/started

Submitted by Tim Niemeyer on Jan. 20, 2018, 10:03 p.m.

Details

Message ID 20180120220345.10999-1-tim@tn-x.org
State Superseded
Headers show

Commit Message

Tim Niemeyer Jan. 20, 2018, 10:03 p.m.
The dependency was only to avoid duplicated script instances.
Therefore, this patch implements a solution without the need to depend
on the /tmp/started file.

Signed-off-by: Tim Niemeyer <tim@tn-x.org>
---

 src/packages/fff/fff-hoods/Makefile                         | 2 +-
 src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods | 2 +-
 src/packages/fff/fff-hoods/files/usr/sbin/configurehood     | 7 +++++++
 3 files changed, 9 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/packages/fff/fff-hoods/Makefile b/src/packages/fff/fff-hoods/Makefile
index 11ab6d1..1a616ff 100644
--- a/src/packages/fff/fff-hoods/Makefile
+++ b/src/packages/fff/fff-hoods/Makefile
@@ -1,7 +1,7 @@ 
 include $(TOPDIR)/rules.mk
 
 PKG_NAME:=fff-hoods
-PKG_VERSION:=0.0.1
+PKG_VERSION:=2
 PKG_RELEASE:=1
 
 PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
diff --git a/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods b/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods
index 39e800e..ca8d798 100644
--- a/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods
+++ b/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods
@@ -1 +1 @@ 
-*/5 * * * * [ -f /tmp/started ] && /usr/sbin/configurehood
+*/5 * * * * /usr/sbin/configurehood
diff --git a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
index 2aaa245..e14995b 100755
--- a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
+++ b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
@@ -1,5 +1,12 @@ 
 #!/bin/sh
 
+lockfile="/var/lock/${0##*/}.lock"
+if ! lock -n "$lockfile"; then
+	echo "Only one instance of $0 allowed."
+	exit 1
+fi
+trap "lock -u \"$lockfile\"" INT TERM EXIT
+
 . /usr/share/libubox/jshn.sh
 . /lib/functions/fff/keyxchange
 . /lib/functions/fff/network

Comments

Adrian Schmutzler Jan. 20, 2018, 10:41 p.m.
Hallo Tim,

ich habe selbst immer wieder überlegt, hier mit einem Lockfile zu arbeiten.

Allerdings besteht hier das Risiko, dass das Skript nie mehr läuft, wenn das
Lockfile aus irgendeinem Grund nicht mehr gelöscht wird. Aus diesem Grund
habe ich mich immer dagegen entschieden, da es das tmp/started ja eh gibt
und das in meinen Augen sicherer ist.

Deshalb bin ich tendenziell eher gegen den Patch.

Frage: Ist /var RAM oder Flash? Nicht das wir hier wieder aus Versehen auf
dem Flash rumschreiben... Falls Flash wäre zudem ein vergessenes Lock-File
permanent.

Grüße

Adrian

> -----Original Message-----
> From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On Behalf
> Of Tim Niemeyer
> Sent: Samstag, 20. Januar 2018 23:04
> To: franken-dev@freifunk.net
> Subject: [PATCH] fff-hoods: remove dependency to /tmp/started
> 
> The dependency was only to avoid duplicated script instances.
> Therefore, this patch implements a solution without the need to depend on
> the /tmp/started file.
> 
> Signed-off-by: Tim Niemeyer <tim@tn-x.org>
> ---
> 
>  src/packages/fff/fff-hoods/Makefile                         | 2 +-
>  src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods | 2 +-
>  src/packages/fff/fff-hoods/files/usr/sbin/configurehood     | 7 +++++++
>  3 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/src/packages/fff/fff-hoods/Makefile b/src/packages/fff/fff-
> hoods/Makefile
> index 11ab6d1..1a616ff 100644
> --- a/src/packages/fff/fff-hoods/Makefile
> +++ b/src/packages/fff/fff-hoods/Makefile
> @@ -1,7 +1,7 @@
>  include $(TOPDIR)/rules.mk
> 
>  PKG_NAME:=fff-hoods
> -PKG_VERSION:=0.0.1
> +PKG_VERSION:=2
>  PKG_RELEASE:=1
> 
>  PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
> diff --git a/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods
> b/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods
> index 39e800e..ca8d798 100644
> --- a/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods
> +++ b/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods
> @@ -1 +1 @@
> -*/5 * * * * [ -f /tmp/started ] && /usr/sbin/configurehood
> +*/5 * * * * /usr/sbin/configurehood
> diff --git a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> index 2aaa245..e14995b 100755
> --- a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> +++ b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> @@ -1,5 +1,12 @@
>  #!/bin/sh
> 
> +lockfile="/var/lock/${0##*/}.lock"
> +if ! lock -n "$lockfile"; then
> +	echo "Only one instance of $0 allowed."
> +	exit 1
> +fi
> +trap "lock -u \"$lockfile\"" INT TERM EXIT
> +
>  . /usr/share/libubox/jshn.sh
>  . /lib/functions/fff/keyxchange
>  . /lib/functions/fff/network
> --
> 2.11.0
> 
> --
> franken-dev mailing list
> franken-dev@freifunk.net
> http://lists.freifunk.net/mailman/listinfo/franken-dev-freifunk.net
Tim Niemeyer Jan. 21, 2018, 10:22 a.m.
Hi Adrian

Am Samstag, den 20.01.2018, 23:41 +0100 schrieb
mail@adrianschmutzler.de:
> Hallo Tim,
> 
> ich habe selbst immer wieder überlegt, hier mit einem Lockfile zu
> arbeiten.
> 
> Allerdings besteht hier das Risiko, dass das Skript nie mehr läuft, 
Grundsätzlich stimmt das, ja. Aber genauso gut könnte das Script selbst
irgendwo hängen bleiben.

> wenn das
> Lockfile aus irgendeinem Grund nicht mehr gelöscht wird.
Dafür ist ja die trap eingebaut. In meinen Tests lief das sehr
zuverlässig.

> Aus diesem Grund
> habe ich mich immer dagegen entschieden, da es das tmp/started ja eh
> gibt
> und das in meinen Augen sicherer ist.
Das /tmp/started möchte ich lieber los werden. Das war mal für
irgendwas gut und wird heute gefühlt an der falschen Stelle verwendet.
Tatsächlich ist das aber etwas, was wir nicht unbedingt ins nächste
stable release einkippen müssen.

Ich denke, wenn man wissen will, wo das System gerade beim Booten ist,
dann gibt es bestimmt irgendeinen Mechanismus von OpenWRT, womit man
das rausfinden kann.

Hier geht es aber nicht um die Bootreihenfolge sondern um die
Verhinderung eines mehrfachen Aufrufs. Das wird mit /tmp/started nur
(zufällig) für den Startprozess gemacht.

> Deshalb bin ich tendenziell eher gegen den Patch.
> 
> Frage: Ist /var RAM oder Flash? Nicht das wir hier wieder aus
> Versehen auf
> dem Flash rumschreiben... Falls Flash wäre zudem ein vergessenes
> Lock-File
> permanent.
Das soll natürlich RAM sein.
# ls -l /var
lrwxrwxrwx    1 root     root             4 Oct 13 14:14 /var -> /tmp
Bei mir ist das auch so.

Tim

> Grüße
> 
> Adrian
> 
> > -----Original Message-----
> > From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On
> > Behalf
> > Of Tim Niemeyer
> > Sent: Samstag, 20. Januar 2018 23:04
> > To: franken-dev@freifunk.net
> > Subject: [PATCH] fff-hoods: remove dependency to /tmp/started
> > 
> > The dependency was only to avoid duplicated script instances.
> > Therefore, this patch implements a solution without the need to
> > depend on
> > the /tmp/started file.
> > 
> > Signed-off-by: Tim Niemeyer <tim@tn-x.org>
> > ---
> > 
> >  src/packages/fff/fff-hoods/Makefile                         | 2 +-
> >  src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods | 2 +-
> >  src/packages/fff/fff-hoods/files/usr/sbin/configurehood     | 7
> > +++++++
> >  3 files changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/packages/fff/fff-hoods/Makefile
> > b/src/packages/fff/fff-
> > hoods/Makefile
> > index 11ab6d1..1a616ff 100644
> > --- a/src/packages/fff/fff-hoods/Makefile
> > +++ b/src/packages/fff/fff-hoods/Makefile
> > @@ -1,7 +1,7 @@
> >  include $(TOPDIR)/rules.mk
> > 
> >  PKG_NAME:=fff-hoods
> > -PKG_VERSION:=0.0.1
> > +PKG_VERSION:=2
> >  PKG_RELEASE:=1
> > 
> >  PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
> > diff --git a/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-
> > hoods
> > b/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods
> > index 39e800e..ca8d798 100644
> > --- a/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods
> > +++ b/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods
> > @@ -1 +1 @@
> > -*/5 * * * * [ -f /tmp/started ] && /usr/sbin/configurehood
> > +*/5 * * * * /usr/sbin/configurehood
> > diff --git a/src/packages/fff/fff-
> > hoods/files/usr/sbin/configurehood
> > b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> > index 2aaa245..e14995b 100755
> > --- a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> > +++ b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> > @@ -1,5 +1,12 @@
> >  #!/bin/sh
> > 
> > +lockfile="/var/lock/${0##*/}.lock"
> > +if ! lock -n "$lockfile"; then
> > +	echo "Only one instance of $0 allowed."
> > +	exit 1
> > +fi
> > +trap "lock -u \"$lockfile\"" INT TERM EXIT
> > +
> >  . /usr/share/libubox/jshn.sh
> >  . /lib/functions/fff/keyxchange
> >  . /lib/functions/fff/network
> > --
> > 2.11.0
> > 
> > --
> > franken-dev mailing list
> > franken-dev@freifunk.net
> > http://lists.freifunk.net/mailman/listinfo/franken-dev-freifunk.net
> 
>
Adrian Schmutzler Feb. 12, 2018, 1:54 p.m.
Hallo Tim,

weiß nicht mehr, ob wir das schon diskutiert haben, aber:
Das Locking verhindert, dass das File zweimal gleichzeitig läuft.
Micrond wird aber mit S50 gestartet, configurenetwork mit S95.

D.h. es kann ohne /tmp/started dazu kommen (wenn Zeitfenster blöd), dass
durch micrond configurehood VOR configurenetwork ausführt, was definitiv
nicht gut ist.

Dies betrifft nur das Entfernen der /tmp/started Bedingung, das Lock selbst
ist davon ja nicht betroffen.

Grüße

Adrian


> -----Original Message-----
> From: mail@adrianschmutzler.de [mailto:mail@adrianschmutzler.de]
> Sent: Samstag, 20. Januar 2018 23:42
> To: 'Tim Niemeyer' <tim@tn-x.org>; franken-dev@freifunk.net
> Subject: RE: [PATCH] fff-hoods: remove dependency to /tmp/started
> 
> Hallo Tim,
> 
> ich habe selbst immer wieder überlegt, hier mit einem Lockfile zu
arbeiten.
> 
> Allerdings besteht hier das Risiko, dass das Skript nie mehr läuft, wenn
das
> Lockfile aus irgendeinem Grund nicht mehr gelöscht wird. Aus diesem Grund
> habe ich mich immer dagegen entschieden, da es das tmp/started ja eh gibt
> und das in meinen Augen sicherer ist.
> 
> Deshalb bin ich tendenziell eher gegen den Patch.
> 
> Frage: Ist /var RAM oder Flash? Nicht das wir hier wieder aus Versehen auf
> dem Flash rumschreiben... Falls Flash wäre zudem ein vergessenes Lock-File
> permanent.
> 
> Grüße
> 
> Adrian
> 
> > -----Original Message-----
> > From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On Behalf
> > Of Tim Niemeyer
> > Sent: Samstag, 20. Januar 2018 23:04
> > To: franken-dev@freifunk.net
> > Subject: [PATCH] fff-hoods: remove dependency to /tmp/started
> >
> > The dependency was only to avoid duplicated script instances.
> > Therefore, this patch implements a solution without the need to depend
> on
> > the /tmp/started file.
> >
> > Signed-off-by: Tim Niemeyer <tim@tn-x.org>
> > ---
> >
> >  src/packages/fff/fff-hoods/Makefile                         | 2 +-
> >  src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods | 2 +-
> >  src/packages/fff/fff-hoods/files/usr/sbin/configurehood     | 7 +++++++
> >  3 files changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/packages/fff/fff-hoods/Makefile
> > b/src/packages/fff/fff- hoods/Makefile index 11ab6d1..1a616ff 100644
> > --- a/src/packages/fff/fff-hoods/Makefile
> > +++ b/src/packages/fff/fff-hoods/Makefile
> > @@ -1,7 +1,7 @@
> >  include $(TOPDIR)/rules.mk
> >
> >  PKG_NAME:=fff-hoods
> > -PKG_VERSION:=0.0.1
> > +PKG_VERSION:=2
> >  PKG_RELEASE:=1
> >
> >  PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
> > diff --git
> > a/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods
> > b/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods
> > index 39e800e..ca8d798 100644
> > --- a/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods
> > +++ b/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods
> > @@ -1 +1 @@
> > -*/5 * * * * [ -f /tmp/started ] && /usr/sbin/configurehood
> > +*/5 * * * * /usr/sbin/configurehood
> > diff --git a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> > b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> > index 2aaa245..e14995b 100755
> > --- a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> > +++ b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> > @@ -1,5 +1,12 @@
> >  #!/bin/sh
> >
> > +lockfile="/var/lock/${0##*/}.lock"
> > +if ! lock -n "$lockfile"; then
> > +	echo "Only one instance of $0 allowed."
> > +	exit 1
> > +fi
> > +trap "lock -u \"$lockfile\"" INT TERM EXIT
> > +
> >  . /usr/share/libubox/jshn.sh
> >  . /lib/functions/fff/keyxchange
> >  . /lib/functions/fff/network
> > --
> > 2.11.0
> >
> > --
> > franken-dev mailing list
> > franken-dev@freifunk.net
> > http://lists.freifunk.net/mailman/listinfo/franken-dev-freifunk.net
Fabian Blaese June 13, 2018, 7:15 a.m.
Ich wollte an dieser Stelle nochmal an dieses Patch erinnern.
Imo will man das unbedingt noch im Release haben.

Soweit ich das mit dem trap richtig überblicke, müsste das Lockfile auch immer sauber gelöscht werden.

Grade auch gesehen, dass noch was fehlt:
Reviewed-by: Fabian Bläse <fabian@blaese.de>

> On 20. Jan 2018, at 23:03, Tim Niemeyer <tim@tn-x.org> wrote:
> 
> The dependency was only to avoid duplicated script instances.
> Therefore, this patch implements a solution without the need to depend
> on the /tmp/started file.
> 
> Signed-off-by: Tim Niemeyer <tim@tn-x.org>
> ---
> 
> src/packages/fff/fff-hoods/Makefile                         | 2 +-
> src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods | 2 +-
> src/packages/fff/fff-hoods/files/usr/sbin/configurehood     | 7 +++++++
> 3 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/src/packages/fff/fff-hoods/Makefile b/src/packages/fff/fff-hoods/Makefile
> index 11ab6d1..1a616ff 100644
> --- a/src/packages/fff/fff-hoods/Makefile
> +++ b/src/packages/fff/fff-hoods/Makefile
> @@ -1,7 +1,7 @@
> include $(TOPDIR)/rules.mk
> 
> PKG_NAME:=fff-hoods
> -PKG_VERSION:=0.0.1
> +PKG_VERSION:=2
> PKG_RELEASE:=1
> 
> PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
> diff --git a/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods b/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods
> index 39e800e..ca8d798 100644
> --- a/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods
> +++ b/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods
> @@ -1 +1 @@
> -*/5 * * * * [ -f /tmp/started ] && /usr/sbin/configurehood
> +*/5 * * * * /usr/sbin/configurehood
> diff --git a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> index 2aaa245..e14995b 100755
> --- a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> +++ b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> @@ -1,5 +1,12 @@
> #!/bin/sh
> 
> +lockfile="/var/lock/${0##*/}.lock"
> +if ! lock -n "$lockfile"; then
> +	echo "Only one instance of $0 allowed."
> +	exit 1
> +fi
> +trap "lock -u \"$lockfile\"" INT TERM EXIT
> +
> . /usr/share/libubox/jshn.sh
> . /lib/functions/fff/keyxchange
> . /lib/functions/fff/network
> --
> 2.11.0
> 
> --
> franken-dev mailing list
> franken-dev@freifunk.net
> http://lists.freifunk.net/mailman/listinfo/franken-dev-freifunk.net
Adrian Schmutzler June 13, 2018, 11:37 a.m.
Hallo Fabian/Tim,

bitte meinen alten Kommentar beachten:

> weiß nicht mehr, ob wir das schon diskutiert haben, aber:
> Das Locking verhindert, dass das File zweimal gleichzeitig läuft.
> Micrond wird aber mit S50 gestartet, configurenetwork mit S95.

> D.h. es kann ohne /tmp/started dazu kommen (wenn Zeitfenster blöd), dass
> durch micrond configurehood VOR configurenetwork ausführt, was definitiv
> nicht gut ist.

> Dies betrifft nur das Entfernen der /tmp/started Bedingung, das Lock selbst
> ist davon ja nicht betroffen.

Ich habe kein Problem mit dem Locking, ich würde nur das /tmp/started trotzdem im micrond belassen, dann kann gar nichts mehr passieren!

Solange das /tmp/started entfernt wird, ist das daher für mich NAK.

Grüße

Adrian


> -----Original Message-----
> From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On Behalf
> Of Fabian Bläse
> Sent: Mittwoch, 13. Juni 2018 09:16
> To: Tim Niemeyer <tim@tn-x.org>
> Cc: franken-dev <franken-dev@freifunk.net>
> Subject: Re: [PATCH] fff-hoods: remove dependency to /tmp/started
> 
> Ich wollte an dieser Stelle nochmal an dieses Patch erinnern.
> Imo will man das unbedingt noch im Release haben.
> 
> Soweit ich das mit dem trap richtig überblicke, müsste das Lockfile auch
> immer sauber gelöscht werden.
> 
> Grade auch gesehen, dass noch was fehlt:
> Reviewed-by: Fabian Bläse <fabian@blaese.de>
> 
> > On 20. Jan 2018, at 23:03, Tim Niemeyer <tim@tn-x.org> wrote:
> >
> > The dependency was only to avoid duplicated script instances.
> > Therefore, this patch implements a solution without the need to depend
> > on the /tmp/started file.
> >
> > Signed-off-by: Tim Niemeyer <tim@tn-x.org>
> > ---
> >
> > src/packages/fff/fff-hoods/Makefile                         | 2 +-
> > src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods | 2 +-
> > src/packages/fff/fff-hoods/files/usr/sbin/configurehood     | 7 +++++++
> > 3 files changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/packages/fff/fff-hoods/Makefile
> > b/src/packages/fff/fff-hoods/Makefile
> > index 11ab6d1..1a616ff 100644
> > --- a/src/packages/fff/fff-hoods/Makefile
> > +++ b/src/packages/fff/fff-hoods/Makefile
> > @@ -1,7 +1,7 @@
> > include $(TOPDIR)/rules.mk
> >
> > PKG_NAME:=fff-hoods
> > -PKG_VERSION:=0.0.1
> > +PKG_VERSION:=2
> > PKG_RELEASE:=1
> >
> > PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
> > diff --git
> > a/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods
> > b/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods
> > index 39e800e..ca8d798 100644
> > --- a/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods
> > +++ b/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods
> > @@ -1 +1 @@
> > -*/5 * * * * [ -f /tmp/started ] && /usr/sbin/configurehood
> > +*/5 * * * * /usr/sbin/configurehood
> > diff --git a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> > b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> > index 2aaa245..e14995b 100755
> > --- a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> > +++ b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> > @@ -1,5 +1,12 @@
> > #!/bin/sh
> >
> > +lockfile="/var/lock/${0##*/}.lock"
> > +if ! lock -n "$lockfile"; then
> > +	echo "Only one instance of $0 allowed."
> > +	exit 1
> > +fi
> > +trap "lock -u \"$lockfile\"" INT TERM EXIT
> > +
> > . /usr/share/libubox/jshn.sh
> > . /lib/functions/fff/keyxchange
> > . /lib/functions/fff/network
> > --
> > 2.11.0
> >
> > --
> > franken-dev mailing list
> > franken-dev@freifunk.net
> > http://lists.freifunk.net/mailman/listinfo/franken-dev-freifunk.net
Fabian Blaese June 13, 2018, 3:04 p.m.
Hallo,

dem wollte ich nicht widersprechen.
Entweder brauchen wir dafür eine bessere Lösung, oder das /tmp/started muss drin bleiben.

Gruß
Fabian

Adrian: Sorry für das Duplikat, die Mailingliste hat gesponnen und mir Fehler entgegen geschmissen.

> On 13. Jun 2018, at 13:37, Adrian Schmutzler <mail@adrianschmutzler.de> wrote:
> 
> Hallo Fabian/Tim,
> 
> bitte meinen alten Kommentar beachten:
> 
>> weiß nicht mehr, ob wir das schon diskutiert haben, aber:
>> Das Locking verhindert, dass das File zweimal gleichzeitig läuft.
>> Micrond wird aber mit S50 gestartet, configurenetwork mit S95.
> 
>> D.h. es kann ohne /tmp/started dazu kommen (wenn Zeitfenster blöd), dass
>> durch micrond configurehood VOR configurenetwork ausführt, was definitiv
>> nicht gut ist.
> 
>> Dies betrifft nur das Entfernen der /tmp/started Bedingung, das Lock selbst
>> ist davon ja nicht betroffen.
> 
> Ich habe kein Problem mit dem Locking, ich würde nur das /tmp/started trotzdem im micrond belassen, dann kann gar nichts mehr passieren!
> 
> Solange das /tmp/started entfernt wird, ist das daher für mich NAK.
> 
> Grüße
> 
> Adrian
Tim Niemeyer June 13, 2018, 7:39 p.m.
Am Mittwoch, den 13.06.2018, 13:37 +0200 schrieb Adrian Schmutzler:
> Hallo Fabian/Tim,
> 
> bitte meinen alten Kommentar beachten:
> 
> > weiß nicht mehr, ob wir das schon diskutiert haben, aber:
> > Das Locking verhindert, dass das File zweimal gleichzeitig läuft.
> > Micrond wird aber mit S50 gestartet, configurenetwork mit S95.
> > D.h. es kann ohne /tmp/started dazu kommen (wenn Zeitfenster blöd),
> > dass
> > durch micrond configurehood VOR configurenetwork ausführt, was
> > definitiv
> > nicht gut ist.
> > Dies betrifft nur das Entfernen der /tmp/started Bedingung, das
> > Lock selbst
> > ist davon ja nicht betroffen.
> 
> Ich habe kein Problem mit dem Locking, ich würde nur das /tmp/started
> trotzdem im micrond belassen, dann kann gar nichts mehr passieren!
> 
> Solange das /tmp/started entfernt wird, ist das daher für mich NAK.
Naja, Ziel des Patches ist ja dieses unglaublich furchtbare
/tmp/started _endlich_ mal los zu werden.

Wir brauchen einfach ne sauberere Architektur, sonst schießen wir uns
immer öfter selbst ins Knie!

Tim

> 
> Grüße
> 
> Adrian
> 
> 
> > -----Original Message-----
> > From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On
> > Behalf
> > Of Fabian Bläse
> > Sent: Mittwoch, 13. Juni 2018 09:16
> > To: Tim Niemeyer <tim@tn-x.org>
> > Cc: franken-dev <franken-dev@freifunk.net>
> > Subject: Re: [PATCH] fff-hoods: remove dependency to /tmp/started
> > 
> > Ich wollte an dieser Stelle nochmal an dieses Patch erinnern.
> > Imo will man das unbedingt noch im Release haben.
> > 
> > Soweit ich das mit dem trap richtig überblicke, müsste das Lockfile
> > auch
> > immer sauber gelöscht werden.
> > 
> > Grade auch gesehen, dass noch was fehlt:
> > Reviewed-by: Fabian Bläse <fabian@blaese.de>
> > 
> > > On 20. Jan 2018, at 23:03, Tim Niemeyer <tim@tn-x.org> wrote:
> > > 
> > > The dependency was only to avoid duplicated script instances.
> > > Therefore, this patch implements a solution without the need to
> > > depend
> > > on the /tmp/started file.
> > > 
> > > Signed-off-by: Tim Niemeyer <tim@tn-x.org>
> > > ---
> > > 
> > > src/packages/fff/fff-hoods/Makefile                         | 2
> > > +-
> > > src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods | 2
> > > +-
> > > src/packages/fff/fff-hoods/files/usr/sbin/configurehood     | 7
> > > +++++++
> > > 3 files changed, 9 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/src/packages/fff/fff-hoods/Makefile
> > > b/src/packages/fff/fff-hoods/Makefile
> > > index 11ab6d1..1a616ff 100644
> > > --- a/src/packages/fff/fff-hoods/Makefile
> > > +++ b/src/packages/fff/fff-hoods/Makefile
> > > @@ -1,7 +1,7 @@
> > > include $(TOPDIR)/rules.mk
> > > 
> > > PKG_NAME:=fff-hoods
> > > -PKG_VERSION:=0.0.1
> > > +PKG_VERSION:=2
> > > PKG_RELEASE:=1
> > > 
> > > PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
> > > diff --git
> > > a/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods
> > > b/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods
> > > index 39e800e..ca8d798 100644
> > > --- a/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods
> > > +++ b/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods
> > > @@ -1 +1 @@
> > > -*/5 * * * * [ -f /tmp/started ] && /usr/sbin/configurehood
> > > +*/5 * * * * /usr/sbin/configurehood
> > > diff --git a/src/packages/fff/fff-
> > > hoods/files/usr/sbin/configurehood
> > > b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> > > index 2aaa245..e14995b 100755
> > > --- a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> > > +++ b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> > > @@ -1,5 +1,12 @@
> > > #!/bin/sh
> > > 
> > > +lockfile="/var/lock/${0##*/}.lock"
> > > +if ! lock -n "$lockfile"; then
> > > +	echo "Only one instance of $0 allowed."
> > > +	exit 1
> > > +fi
> > > +trap "lock -u \"$lockfile\"" INT TERM EXIT
> > > +
> > > . /usr/share/libubox/jshn.sh
> > > . /lib/functions/fff/keyxchange
> > > . /lib/functions/fff/network
> > > --
> > > 2.11.0
> > > 
> > > --
> > > franken-dev mailing list
> > > franken-dev@freifunk.net
> > > http://lists.freifunk.net/mailman/listinfo/franken-dev-freifunk.n
> > > et
> 
>
Fabian Blaese June 14, 2018, 1:09 p.m.
Hallo Tim,

On 13.06.2018 21:39, Tim Niemeyer wrote:
> Naja, Ziel des Patches ist ja dieses unglaublich furchtbare
> /tmp/started _endlich_ mal los zu werden.

Joa, nur kann dieser Patch das leider nicht erfüllen, weil wir uns dann einen neuen Randfall bauen. (siehe Adrians Anmerkung)

Daher würde ich das in zwei Schritten machen:
- Lockfile für configurehood
- Irgendwas sinnvolles um sicherzustellen, dass configurehood erst laufen kann, wenn configurenetwork fertig ist.

Gruß
Fabian
Adrian Schmutzler Aug. 3, 2018, 10:43 a.m.
Hallo,

ich habe gerade mal wieder über das Thema nachgedacht.

Es kommt ja gelegentlich vor, dass configurehood hängt, und dann wird es zigmal
gestartet. Das würde dieser Patch beheben.

Die Konsequenz wäre aber, dass dann das ursprüngliche configurehood solange
alleine läuft, bis es "fertig ist". Hängt es allerdings wirklich, wird es nie
fertig und der Router hängt auch.

Wäre es nicht irgendwie sinnvoller, einen Mechanismus zu finden, der ein
hängendes configurehood abschießt?

Gibt es für sowas in Linux Mechanismen? Primitiv könnte man natürlich einfach
per micrond zwei Minuten später den Prozess killen, aber vll. gibt es für sowas
was elegantes? Vll. gibt es auch etwas, was nicht nur einfach von der Zeit
abhängt?

Grüße

Adrian

> -----Original Message-----
> From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On Behalf Of
> Tim Niemeyer
> Sent: Samstag, 20. Januar 2018 23:04
> To: franken-dev@freifunk.net
> Subject: [PATCH] fff-hoods: remove dependency to /tmp/started
> 
> The dependency was only to avoid duplicated script instances.
> Therefore, this patch implements a solution without the need to depend
> on the /tmp/started file.
> 
> Signed-off-by: Tim Niemeyer <tim@tn-x.org>
> ---
> 
>  src/packages/fff/fff-hoods/Makefile                         | 2 +-
>  src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods | 2 +-
>  src/packages/fff/fff-hoods/files/usr/sbin/configurehood     | 7 +++++++
>  3 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/src/packages/fff/fff-hoods/Makefile
> b/src/packages/fff/fff-hoods/Makefile
> index 11ab6d1..1a616ff 100644
> --- a/src/packages/fff/fff-hoods/Makefile
> +++ b/src/packages/fff/fff-hoods/Makefile
> @@ -1,7 +1,7 @@
>  include $(TOPDIR)/rules.mk
> 
>  PKG_NAME:=fff-hoods
> -PKG_VERSION:=0.0.1
> +PKG_VERSION:=2
>  PKG_RELEASE:=1
> 
>  PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
> diff --git a/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods
> b/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods
> index 39e800e..ca8d798 100644
> --- a/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods
> +++ b/src/packages/fff/fff-hoods/files/usr/lib/micron.d/fff-hoods
> @@ -1 +1 @@
> -*/5 * * * * [ -f /tmp/started ] && /usr/sbin/configurehood
> +*/5 * * * * /usr/sbin/configurehood
> diff --git a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> index 2aaa245..e14995b 100755
> --- a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> +++ b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> @@ -1,5 +1,12 @@
>  #!/bin/sh
> 
> +lockfile="/var/lock/${0##*/}.lock"
> +if ! lock -n "$lockfile"; then
> +	echo "Only one instance of $0 allowed."
> +	exit 1
> +fi
> +trap "lock -u \"$lockfile\"" INT TERM EXIT
> +
>  . /usr/share/libubox/jshn.sh
>  . /lib/functions/fff/keyxchange
>  . /lib/functions/fff/network
> --
> 2.11.0
> 
> --
> franken-dev mailing list
> franken-dev@freifunk.net
> http://lists.freifunk.net/mailman/listinfo/franken-dev-freifunk.net
Fabian Blaese Aug. 3, 2018, 10:52 a.m.
Hallo Adrian,

> On 3. Aug 2018, at 12:43, Adrian Schmutzler <mail@adrianschmutzler.de> wrote:
> 
> Hallo,
> 
> ich habe gerade mal wieder über das Thema nachgedacht.
> 
> Es kommt ja gelegentlich vor, dass configurehood hängt, und dann wird es zigmal
> gestartet. Das würde dieser Patch beheben.

Das ist auch der Teil aus diesem Patch, den ich unbedingt gerne hätte.
Der Patch zielt damit aber darauf ab, /tmp/started los zu werden, was du ja in dieser Form nicht wolltest.

Hier sollte man mal eine Lösung finden, die sicherstellt, dass configurehood (und ggf. auch die Firewall wie festgestellt wurde) erst nach configurenetwork laufen können.

> Die Konsequenz wäre aber, dass dann das ursprüngliche configurehood solange
> alleine läuft, bis es "fertig ist". Hängt es allerdings wirklich, wird es nie
> fertig und der Router hängt auch.

Was aber keinen Unterschied macht. Wenn ein configurehood hängt, hängen alle.
Mein Gefühl sagt mir aber, dass das nur passiert, wenn configurehood mehrfach läuft (z.B. wenn man es manuell startet) und das also ein Nebenläufigkeitsproblem ist.

> 
> Wäre es nicht irgendwie sinnvoller, einen Mechanismus zu finden, der ein
> hängendes configurehood abschießt?

Bisher hat das noch nie was geholfen. Bei mir äußerte sich immer in irgendeinem Lock auf den Wireless Interfaces, das nicht frei wurde, wenn ich die Meldung richtig im Kopf habe.

> Gibt es für sowas in Linux Mechanismen? Primitiv könnte man natürlich einfach
> per micrond zwei Minuten später den Prozess killen, aber vll. gibt es für sowas
> was elegantes? Vll. gibt es auch etwas, was nicht nur einfach von der Zeit
> abhängt?

timeout(1) kann sowas, ist aber nicht in der Firmware.

Gruß
Fabian
Adrian Schmutzler Aug. 3, 2018, 11 a.m.
Hallo Fabian,

> -----Original Message-----
> From: Fabian Bläse [mailto:fabian@blaese.de]
> Sent: Freitag, 3. August 2018 12:53
> To: Adrian Schmutzler <mail@adrianschmutzler.de>; franken-dev@freifunk.net
> Cc: Tim Niemeyer <tim@tn-x.org>
> Subject: Re: [PATCH] fff-hoods: remove dependency to /tmp/started
> 
> Hallo Adrian,
> 
> > On 3. Aug 2018, at 12:43, Adrian Schmutzler <mail@adrianschmutzler.de>
> wrote:
> >
> > Hallo,
> >
> > ich habe gerade mal wieder über das Thema nachgedacht.
> >
> > Es kommt ja gelegentlich vor, dass configurehood hängt, und dann wird es
> zigmal
> > gestartet. Das würde dieser Patch beheben.
> 
> Das ist auch der Teil aus diesem Patch, den ich unbedingt gerne hätte.
> Der Patch zielt damit aber darauf ab, /tmp/started los zu werden, was du ja in
> dieser Form nicht wolltest.

Den Teil mit dem einmal ausführen finde ich für sich auch gut, ich bin mir nur nicht sicher, ob das hilft.

> 
> Hier sollte man mal eine Lösung finden, die sicherstellt, dass configurehood (und
> ggf. auch die Firewall wie festgestellt wurde) erst nach configurenetwork laufen
> können.

Was ja das ist, was /tmp/started hervorragend leistet. Es ist nur nicht schön, wir brauchen also ggf. eine "bessere Lösung".

> 
> > Die Konsequenz wäre aber, dass dann das ursprüngliche configurehood
> solange
> > alleine läuft, bis es "fertig ist". Hängt es allerdings wirklich, wird es nie
> > fertig und der Router hängt auch.
> 
> Was aber keinen Unterschied macht. Wenn ein configurehood hängt, hängen
> alle.
> Mein Gefühl sagt mir aber, dass das nur passiert, wenn configurehood mehrfach
> läuft (z.B. wenn man es manuell startet) und das also ein
> Nebenläufigkeitsproblem ist.

Bei mir tritt das auch manchmal einfach so auf, ohne dass jemand manuell configurehood startet.

> 
> >
> > Wäre es nicht irgendwie sinnvoller, einen Mechanismus zu finden, der ein
> > hängendes configurehood abschießt?
> 
> Bisher hat das noch nie was geholfen. Bei mir äußerte sich immer in
> irgendeinem Lock auf den Wireless Interfaces, das nicht frei wurde, wenn ich
> die Meldung richtig im Kopf habe.

Ja, genau. Aber wenn configurehood dann drölfzig Mal gestartet wird, hört das Lock nie mehr auf. Ich hatte den Eindruck, dass das reine wifi lock irgendwann von selbst wieder weggeht (nach einigen Minuten), wenn man nicht durch weitere Aufrufe weiter reinfeuert.
Nach der Argumentation würde dann aber die Essenz aus Tims Patch reichen, weil wenn sich wifi fängt, läuft das ursprüngliche configurehood "irgendwann" durch.

> 
> > Gibt es für sowas in Linux Mechanismen? Primitiv könnte man natürlich
> einfach
> > per micrond zwei Minuten später den Prozess killen, aber vll. gibt es für sowas
> > was elegantes? Vll. gibt es auch etwas, was nicht nur einfach von der Zeit
> > abhängt?
> 
> timeout(1) kann sowas, ist aber nicht in der Firmware.

Das schaue ich mir mal an. Schön ist das aber in jedem Fall nicht.

Ich wäre dafür, den Patch ohne Entfernen von /tmp/started mal einzuspielen.
Ich kann aber leider den Code nicht selbst reviewen, du hattest aber ja nachvollzogen, dass er funktioniert?

Grüße

Adrian

> 
> Gruß
> Fabian