Bug#923924: Please review and apply attached patch to support shutdown on SIGPWR
On Mon, 11 Mar 2019 20:51:29 +0100 Andras Korn
wrote:
> On Mon, Mar 11, 2019 at 06:12:06PM +, Dmitry Bogatov wrote:
>
> > > On Fri, Mar 08, 2019 at 02:39:47PM +, Dmitry Bogatov wrote:
> > > > [2019-03-07 12:57] Andras Korn
> > > > > part 1 text/plain 218
> > > > > Sorry, I sent an earlier version of the patch by mistake.
> > > > >
> > > > > I'm attaching the correct one, which I tested and which works for
me.
> > > > > [...]
> > > > > - if (sigc && (stat(STOPIT, ) != -1) && (s.st_mode &
S_IXUSR)) {
> > > > > + if ((sigp) || (sigc && (stat(STOPIT, ) != -1) &&
(s.st_mode & S_IXUSR))) {
> > > >
> > > > As far as I can tell by glance on patch, you want SIGPWR trigger
reboot.
> > > > If so, why don't you create REBOOT file in, say, /etc/rc.local and
make
> > > > lxc controller to send SIGCONT?
> > >
> > > No -- I want SIGPWR to trigger a halt.
> > >
> > > For the purposes of LXC, any signal will do; I just need for a signal
to
> > > trigger a shutdown regardless of the permissions on runit.stopit and
> > > runit.reboot.
> >
> > Halt. Fine. But why can't you pre-provision you container with apporiate
> > `stop.*' file with apporiate permissions?
>
> Because that adds complexity elsewhere -- /etc/runit/1 as shipped creates
> /run/runit.stopit with mode 0, so either all containers would need ot
have a
> custom /etc/runit/1, or run a custom script to chmod 100 /run/runit.stopit
> on every boot, or have an immutable /run/runit.stopit.
>
> It's not just about me; this affects everyone who wants to use runit
inside
> an lxc container.
>
> My goal is to make using runit as hassle-free as possible, without
> sacrificing any of its core values.
>
> > > > By the way, SIGPWR is not in POSIX, according to signal(7).
> > >
> > > You're right; in that case, maybe we can use SIGQUIT?
> >
> > SIGTERM feels better, imho. TERM is graceful termination, while SIGQUIT
> > creates coredump. By default.
>
> SIGPWR would be nice to use as the halt signal because it's the lxc
default,
> so that runit could be a drop-in replacement for sysvinit in LXC
containers.
We should recognize that SIGPWR was chosen in a fairly arbitrary way.
Of course, SIGPWR is in use today by LXC and powerstatd so it is useful to
support.
>
> If we're not going to use SIGPWR it's pretty much all the same which
signal
> we use, because it will need to be configured explicitly in LXC (but
that's
> acceptable -- POSIX is important enough).
>
> > But this naming only matters if you explain to me, how solution not
> > involving changing C code does not suffice. Two lines for convenience in
> > this case, three there -- and we all know where it ends.
>
> I'm sorry, I don't buy the slippery slope argument. I'm not adding a DNS
> resolver, a DHCP client or a QR encoder, merely making the user interface
a
> tiny bit more similar to sysvinit's, to make integration easier. This is
> entirely in line with The Unix Way: making one program a drop-in
replacement
> for another such that other programs interfacing with them don't see a
> difference unless they need to. It's why bzip2 and gzip take most of the
You are taking a previously portable codebase and making it not portable.
As a distribution patch, this might be acceptable. However it is an
unfortunate compromise.
Personally, I elect to replace the runit-init program entirely and only use
the supervision suite.
There is absolutely zero reason for an init system to call reboot(2). It is
simply unnecessary.
I have written a guide to do exactly this. It leverages two small C
programs:
* linit:
* Reaps zombies
* Ignores SIGCHLD, preventing new zombies
* Sets up signal handlers for SIGINT and SIGPWR that spawn hooks
* Spawns a boot hook
* Calls pause()
* lreboot: calls reboot(2)
The rest is simply some scripting to emulate what runit-init does.
Please review the guide and source code for the above C programs:
https://gitlab.com/chinstrap/linit/blob/master/README.runit.md
Regards,
--
Cameron Nemo
Bug#923924: Please review and apply attached patch to support shutdown on SIGPWR
control: tags -1 +patch +pending [2019-03-11 20:51] Andras Korn > This is entirely in line with The Unix Way: making one program a > drop-in replacement for another such that other programs interfacing > with them don't see a difference unless they need to. It's why bzip2 > and gzip take most of the same switches etc. Well, this sounds convincingly. Added three more lines, using SIGUSR2 instead of SIGPWR if latter is not available, as sysvinit do. > Since you apparently don't agree that this is a good idea, should I > take the patch to Gerrit instead? Not sure it will help, his activity seems dormant for quite a long time, but good luck. -- Note, that I send and fetch email in batch, once every 24 hours. If matter is urgent, try https://t.me/kaction --
Bug#923924: Please review and apply attached patch to support shutdown on SIGPWR
On Mon, Mar 11, 2019 at 06:12:06PM +, Dmitry Bogatov wrote:
> > On Fri, Mar 08, 2019 at 02:39:47PM +, Dmitry Bogatov wrote:
> > > [2019-03-07 12:57] Andras Korn
> > > > part 1 text/plain 218
> > > > Sorry, I sent an earlier version of the patch by mistake.
> > > >
> > > > I'm attaching the correct one, which I tested and which works for me.
> > > > [...]
> > > > - if (sigc && (stat(STOPIT, ) != -1) && (s.st_mode & S_IXUSR)) {
> > > > + if ((sigp) || (sigc && (stat(STOPIT, ) != -1) && (s.st_mode &
> > > > S_IXUSR))) {
> > >
> > > As far as I can tell by glance on patch, you want SIGPWR trigger reboot.
> > > If so, why don't you create REBOOT file in, say, /etc/rc.local and make
> > > lxc controller to send SIGCONT?
> >
> > No -- I want SIGPWR to trigger a halt.
> >
> > For the purposes of LXC, any signal will do; I just need for a signal to
> > trigger a shutdown regardless of the permissions on runit.stopit and
> > runit.reboot.
>
> Halt. Fine. But why can't you pre-provision you container with apporiate
> `stop.*' file with apporiate permissions?
Because that adds complexity elsewhere -- /etc/runit/1 as shipped creates
/run/runit.stopit with mode 0, so either all containers would need ot have a
custom /etc/runit/1, or run a custom script to chmod 100 /run/runit.stopit
on every boot, or have an immutable /run/runit.stopit.
It's not just about me; this affects everyone who wants to use runit inside
an lxc container.
My goal is to make using runit as hassle-free as possible, without
sacrificing any of its core values.
> > > By the way, SIGPWR is not in POSIX, according to signal(7).
> >
> > You're right; in that case, maybe we can use SIGQUIT?
>
> SIGTERM feels better, imho. TERM is graceful termination, while SIGQUIT
> creates coredump. By default.
SIGPWR would be nice to use as the halt signal because it's the lxc default,
so that runit could be a drop-in replacement for sysvinit in LXC containers.
If we're not going to use SIGPWR it's pretty much all the same which signal
we use, because it will need to be configured explicitly in LXC (but that's
acceptable -- POSIX is important enough).
> But this naming only matters if you explain to me, how solution not
> involving changing C code does not suffice. Two lines for convenience in
> this case, three there -- and we all know where it ends.
I'm sorry, I don't buy the slippery slope argument. I'm not adding a DNS
resolver, a DHCP client or a QR encoder, merely making the user interface a
tiny bit more similar to sysvinit's, to make integration easier. This is
entirely in line with The Unix Way: making one program a drop-in replacement
for another such that other programs interfacing with them don't see a
difference unless they need to. It's why bzip2 and gzip take most of the
same switches etc.
Since you apparently don't agree that this is a good idea, should I take the
patch to Gerrit instead?
András
--
Warranty is void in case of nuclear war, whether caused by the program or not.
Bug#923924: Please review and apply attached patch to support shutdown on SIGPWR
[2019-03-10 12:09] Andras Korn
> On Fri, Mar 08, 2019 at 02:39:47PM +, Dmitry Bogatov wrote:
> > [2019-03-07 12:57] Andras Korn
> > > part 1 text/plain 218
> > > Sorry, I sent an earlier version of the patch by mistake.
> > >
> > > I'm attaching the correct one, which I tested and which works for me.
> > > [...]
> > > - if (sigc && (stat(STOPIT, ) != -1) && (s.st_mode & S_IXUSR)) {
> > > + if ((sigp) || (sigc && (stat(STOPIT, ) != -1) && (s.st_mode &
> > > S_IXUSR))) {
> >
> > As far as I can tell by glance on patch, you want SIGPWR trigger reboot.
> > If so, why don't you create REBOOT file in, say, /etc/rc.local and make
> > lxc controller to send SIGCONT?
>
> No -- I want SIGPWR to trigger a halt.
>
> For the purposes of LXC, any signal will do; I just need for a signal to
> trigger a shutdown regardless of the permissions on runit.stopit and
> runit.reboot.
Halt. Fine. But why can't you pre-provision you container with apporiate
`stop.*' file with apporiate permissions?
> > By the way, SIGPWR is not in POSIX, according to signal(7).
>
> You're right; in that case, maybe we can use SIGQUIT?
SIGTERM feels better, imho. TERM is graceful termination, while SIGQUIT
creates coredump. By default.
But this naming only matters if you explain to me, how solution not
involving changing C code does not suffice. Two lines for convenience in
this case, three there -- and we all know where it ends.
--
Note, that I send and fetch email in batch, once every 24 hours.
If matter is urgent, try https://t.me/kaction
--
Bug#923924: Please review and apply attached patch to support shutdown on SIGPWR
On Fri, Mar 08, 2019 at 02:39:47PM +, Dmitry Bogatov wrote:
Hi,
> [2019-03-07 12:57] Andras Korn
> > part 1 text/plain 218
> > Sorry, I sent an earlier version of the patch by mistake.
> >
> > I'm attaching the correct one, which I tested and which works for me.
> > [...]
> > - if (sigc && (stat(STOPIT, ) != -1) && (s.st_mode & S_IXUSR)) {
> > + if ((sigp) || (sigc && (stat(STOPIT, ) != -1) && (s.st_mode &
> > S_IXUSR))) {
>
> As far as I can tell by glance on patch, you want SIGPWR trigger reboot.
> If so, why don't you create REBOOT file in, say, /etc/rc.local and make
> lxc controller to send SIGCONT?
No -- I want SIGPWR to trigger a halt.
For the purposes of LXC, any signal will do; I just need for a signal to
trigger a shutdown regardless of the permissions on runit.stopit and
runit.reboot.
> By the way, SIGPWR is not in POSIX, according to signal(7).
You're right; in that case, maybe we can use SIGQUIT?
I suppose the other option is to wrap all changes in #ifdefs so the new bits
are only built on Linux (since it's currently only relevant for Linux, this
might be appropriate), but I think that would make the code
disproportionately ugly.
András
--
How to lose a guy in 10 days? Buy him a computer.
Bug#923924: Please review and apply attached patch to support shutdown on SIGPWR
[2019-03-07 12:57] Andras Korn
> part 1 text/plain 218
> Sorry, I sent an earlier version of the patch by mistake.
>
> I'm attaching the correct one, which I tested and which works for me.
> [...]
> - if (sigc && (stat(STOPIT, ) != -1) && (s.st_mode & S_IXUSR)) {
> + if ((sigp) || (sigc && (stat(STOPIT, ) != -1) && (s.st_mode &
> S_IXUSR))) {
As far as I can tell by glance on patch, you want SIGPWR trigger reboot.
If so, why don't you create REBOOT file in, say, /etc/rc.local and make
lxc controller to send SIGCONT?
Or am I missing something, and your patch gives more flexibility?
By the way, SIGPWR is not in POSIX, according to signal(7).
--
Note, that I send and fetch email in batch, once every 24 hours.
If matter is urgent, try https://t.me/kaction
--
Bug#923924: Please review and apply attached patch to support shutdown on SIGPWR
Sorry, I sent an earlier version of the patch by mistake.
I'm attaching the correct one, which I tested and which works for me.
András
--
Reality is that which, when you stop believing in it, doesn't go away.
diff --git a/runit-2.1.2/man/runit.8 b/runit-2.1.2/man/runit.8
index 6c07cf8..d597fdd 100644
--- a/runit-2.1.2/man/runit.8
+++ b/runit-2.1.2/man/runit.8
@@ -75,6 +75,12 @@ exists and has the execute by owner permission set,
.B runit
is told to shutdown the system.
.P
+If
+.B runit
+receives a PWR signal,
+.B runit
+is told to shutdown the system.
+.P
if
.B runit
receives an INT signal, a ctrl-alt-del keyboard request is triggered.
diff --git a/runit-2.1.2/src/runit.c b/runit-2.1.2/src/runit.c
index 2bb4794..35cda0a 100644
--- a/runit-2.1.2/src/runit.c
+++ b/runit-2.1.2/src/runit.c
@@ -30,6 +30,7 @@ const char * const stage[3] ={
int selfpipe[2];
int sigc =0;
int sigi =0;
+int sigp =0;
void sig_cont_handler (void) {
sigc++;
@@ -39,6 +40,10 @@ void sig_int_handler (void) {
sigi++;
write(selfpipe[1], "", 1);
}
+void sig_pwr_handler (void) {
+ sigp++;
+ write(selfpipe[1], "", 1);
+}
void sig_child_handler (void) { write(selfpipe[1], "", 1); }
void sync_if_needed() {
@@ -71,6 +76,8 @@ int main (int argc, const char * const *argv, char * const *envp) {
sig_block(sig_hangup);
sig_block(sig_int);
sig_catch(sig_int, sig_int_handler);
+ sig_block(sig_pwr);
+ sig_catch(sig_pwr, sig_pwr_handler);
sig_block(sig_pipe);
sig_block(sig_term);
@@ -150,6 +157,7 @@ int main (int argc, const char * const *argv, char * const *envp) {
sig_unblock(sig_child);
sig_unblock(sig_cont);
sig_unblock(sig_int);
+ sig_unblock(sig_pwr);
#ifdef IOPAUSE_POLL
poll(, 1, 14000);
#else
@@ -161,6 +169,7 @@ int main (int argc, const char * const *argv, char * const *envp) {
sig_block(sig_cont);
sig_block(sig_child);
sig_block(sig_int);
+ sig_block(sig_pwr);
while (read(selfpipe[0], , 1) == 1) {}
while ((child =wait_nohang()) > 0)
@@ -211,7 +220,7 @@ int main (int argc, const char * const *argv, char * const *envp) {
}
/* sig? */
- if (!sigc && !sigi) {
+ if (!sigc && !sigi && !sigp) {
#ifdef DEBUG
strerr_warn2(WARNING, "poll: ", _sys);
#endif
@@ -244,7 +253,7 @@ int main (int argc, const char * const *argv, char * const *envp) {
sigi =0;
sigc++;
}
- if (sigc && (stat(STOPIT, ) != -1) && (s.st_mode & S_IXUSR)) {
+ if ((sigp) || (sigc && (stat(STOPIT, ) != -1) && (s.st_mode & S_IXUSR))) {
int i;
/* unlink(STOPIT); */
chmod(STOPIT, 0);
@@ -286,7 +295,7 @@ int main (int argc, const char * const *argv, char * const *envp) {
/* enter stage 3 */
break;
}
- sigc =sigi =0;
+ sigc =sigi =sigp =0;
#ifdef DEBUG
strerr_warn2(WARNING, "no request.", 0);
#endif
@@ -308,7 +317,7 @@ int main (int argc, const char * const *argv, char * const *envp) {
switch (pid) {
case 0:
case -1:
- if ((stat(REBOOT, ) != -1) && (s.st_mode & S_IXUSR)) {
+ if ((!sigp) && ((stat(REBOOT, ) != -1) && (s.st_mode & S_IXUSR))) {
strerr_warn2(INFO, "system reboot.", 0);
sync_if_needed();
reboot_system(RB_AUTOBOOT);
diff --git a/runit-2.1.2/src/sig.c b/runit-2.1.2/src/sig.c
index 423d18e..9d9d69b 100644
--- a/runit-2.1.2/src/sig.c
+++ b/runit-2.1.2/src/sig.c
@@ -9,6 +9,7 @@ int sig_cont = SIGCONT;
int sig_hangup = SIGHUP;
int sig_int = SIGINT;
int sig_pipe = SIGPIPE;
+int sig_pwr = SIGPWR;
int sig_term = SIGTERM;
void (*sig_defaulthandler)() = SIG_DFL;
diff --git a/runit-2.1.2/src/sig.h b/runit-2.1.2/src/sig.h
index 2a3c780..0ced62d 100644
--- a/runit-2.1.2/src/sig.h
+++ b/runit-2.1.2/src/sig.h
@@ -9,6 +9,7 @@ extern int sig_cont;
extern int sig_hangup;
extern int sig_int;
extern int sig_pipe;
+extern int sig_pwr;
extern int sig_term;
extern void (*sig_defaulthandler)();
Bug#923924: Please review and apply attached patch to support shutdown on SIGPWR
Package: runit
Version: 2.1.2-22
Severity: wishlist
Tags: upstream
Hi,
currently lxc can't cleanly shutdown a container running runit as its init
system, because all it can do to bring about a shutdown is send signals to
init (sigpwr by default), and runit requires additional steps to initiate a
shutdown.
I wrote a trivial patch that should add sigpwr support to runit. Please
review it and if you think it's OK, apply it and perhaps submit it to Gerrit
for inclusion.
Thanks!
András
--
Enter any 11-digit prime number to continue.
diff --git a/runit-2.1.2/man/runit.8 b/runit-2.1.2/man/runit.8
index 6c07cf8..d597fdd 100644
--- a/runit-2.1.2/man/runit.8
+++ b/runit-2.1.2/man/runit.8
@@ -75,6 +75,12 @@ exists and has the execute by owner permission set,
.B runit
is told to shutdown the system.
.P
+If
+.B runit
+receives a PWR signal,
+.B runit
+is told to shutdown the system.
+.P
if
.B runit
receives an INT signal, a ctrl-alt-del keyboard request is triggered.
diff --git a/runit-2.1.2/src/runit.c b/runit-2.1.2/src/runit.c
index 2bb4794..584207a 100644
--- a/runit-2.1.2/src/runit.c
+++ b/runit-2.1.2/src/runit.c
@@ -30,6 +30,7 @@ const char * const stage[3] ={
int selfpipe[2];
int sigc =0;
int sigi =0;
+int sigp =0;
void sig_cont_handler (void) {
sigc++;
@@ -39,6 +40,10 @@ void sig_int_handler (void) {
sigi++;
write(selfpipe[1], "", 1);
}
+void sig_pwr_handler (void) {
+ sigp++;
+ write(selfpipe[1], "", 1);
+}
void sig_child_handler (void) { write(selfpipe[1], "", 1); }
void sync_if_needed() {
@@ -71,6 +76,7 @@ int main (int argc, const char * const *argv, char * const *envp) {
sig_block(sig_hangup);
sig_block(sig_int);
sig_catch(sig_int, sig_int_handler);
+ sig_catch(sig_pwr, sig_pwr_handler);
sig_block(sig_pipe);
sig_block(sig_term);
@@ -244,7 +250,7 @@ int main (int argc, const char * const *argv, char * const *envp) {
sigi =0;
sigc++;
}
- if (sigc && (stat(STOPIT, ) != -1) && (s.st_mode & S_IXUSR)) {
+ if ((sigp) || (sigc && (stat(STOPIT, ) != -1) && (s.st_mode & S_IXUSR))) {
int i;
/* unlink(STOPIT); */
chmod(STOPIT, 0);
@@ -308,7 +314,7 @@ int main (int argc, const char * const *argv, char * const *envp) {
switch (pid) {
case 0:
case -1:
- if ((stat(REBOOT, ) != -1) && (s.st_mode & S_IXUSR)) {
+ if ((!sigp) && ((stat(REBOOT, ) != -1) && (s.st_mode & S_IXUSR))) {
strerr_warn2(INFO, "system reboot.", 0);
sync_if_needed();
reboot_system(RB_AUTOBOOT);
diff --git a/runit-2.1.2/src/sig.c b/runit-2.1.2/src/sig.c
index 423d18e..9d9d69b 100644
--- a/runit-2.1.2/src/sig.c
+++ b/runit-2.1.2/src/sig.c
@@ -9,6 +9,7 @@ int sig_cont = SIGCONT;
int sig_hangup = SIGHUP;
int sig_int = SIGINT;
int sig_pipe = SIGPIPE;
+int sig_pwr = SIGPWR;
int sig_term = SIGTERM;
void (*sig_defaulthandler)() = SIG_DFL;
diff --git a/runit-2.1.2/src/sig.h b/runit-2.1.2/src/sig.h
index 2a3c780..0ced62d 100644
--- a/runit-2.1.2/src/sig.h
+++ b/runit-2.1.2/src/sig.h
@@ -9,6 +9,7 @@ extern int sig_cont;
extern int sig_hangup;
extern int sig_int;
extern int sig_pipe;
+extern int sig_pwr;
extern int sig_term;
extern void (*sig_defaulthandler)();
