[Buildroot] [PATCH 1/1] package/openssh: improve init script
thomas.petazzoni at bootlin.com
Mon Feb 3 14:26:03 UTC 2020
On Mon, 3 Feb 2020 15:12:53 +0100
Ignacy Gawędzki <ignacy.gawedzki at green-communications.fr> wrote:
> > > Improve the init script in the following ways:
> > >
> > > Use start-stop-daemon to avoid running the daemon more than once and
> > > to safely send termination signals to the right process.
> > >
> > > Add a proper reload action, by sending the daemon the SIGHUP signal.
> > This should ideally be done in a separate patch.
> > > During restart, check that the daemon has properly terminated and if
> > > not, wait one second before sending it the SIGKILL signal.
> > We never do that in any of our other init scripts, I'm not sure we want
> > to do that here. Is there any reason to do that?
> Of course, if start-stop-daemon -S finds a matching running process,
> it returns an error. Sending a SIGTERM to the running process during
> the stop phase doesn't guarantee that the process will have already terminated
> during the start phase, even after sleeping. If I could use
> the --retry option of standalone start-stop-daemon, I would. But with
> the assumption of Busybox, I need to make sure that the daemon has
> terminated by the time I attempt to start it again.
The problem that you're exposing here doesn't seem to be specific to
openssh, so I think we don't want to solve it specifically for openssh.
> > Overall, could you look at package/busybox/S01syslogd, and use it as a
> > template for S50openssh ?
> If you mean that I should be testing for start-stop-daemon's return
> code explicitly by setting status=$? and using if-then-else-fi
> constructs, then sure, yes.
I meant to make sure the init script looks as much as possible like
S01syslogd, i.e same indentation, same organization, same logic. As I
said, Carlos started an effort to try to make our init scripts more
consistent between each other, so let's try to go in that direction :-)
> > I am a bit confused by this touch /var/lock/sshd. Is it really
> > necessary? The git history was not very helpful on this, as it has been
> > in Buildroot since the init script was introduced years ago. The fact
> > that it is created *after* the sshd daemon is started makes it really
> > weird.
> > Do you have any comment/insight on this ?
> Quite frankly, I have no idea why it's there and by keeping it, I just
> wanted stay compatible with whatever is possibly using it.
We had a brief look at it with Peter Korsgaard, we don't see why it
would be needed. Could you include a preparation patch in your series
that drops this touch+rm of /var/lock/sshd ?
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
More information about the buildroot