Re: runit: add support for SIGPWR signal

Re: runit: add support for SIGPWR signal

[Duncaen]

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

New comment by Duncaen on void-packages repository

https://github.com/void-linux/void-packages/pull/20098#issuecomment-599217657

Comment:
That it doesn't write to the selfpipe seems wrong.

I don't like how this changes the behavior of the stopit file and that this signal can't be disabled or even better optionally enabled by i.e. creating a new file like `/etc/runit/sigpwr` which is checked similar to the STOPIT checks.

Re: runit: add support for SIGPWR signal

[leahneukirchen]

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

New comment by leahneukirchen on void-packages repository

https://github.com/void-linux/void-packages/pull/20098#issuecomment-599218821

Comment:
I'd recommend using Debian's `0013-Shutdown-when-runit-init-receices-SIGPWR.patch`. It is still not configurable, but I think it makes sense. Some USP also send SIGPWR when power gets too low.

Re: runit: add support for SIGPWR signal

[innerspacepilot]

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

New comment by innerspacepilot on void-packages repository

https://github.com/void-linux/void-packages/pull/20098#issuecomment-599224575

Comment:
Oh well, If I only knew that the patch already exists :(
So, there is no significant differences between patches.
Yep, 0013-Shutdown-when-runit-init-receices-SIGPWR.patch looks better than mine (mine is working anyway).
SIGPWR is sent to init (process 1) directly, so no need to create any files (like stopit). Runit also reboots on getting SIGCONT (it is also not configurable)
@leahneukirchen can you find any UPS daemon that sends sigpwr to init? (seems like it was very loooong time ago)

Re: runit: add support for SIGPWR signal

[Duncaen]

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

New comment by Duncaen on void-packages repository

https://github.com/void-linux/void-packages/pull/20098#issuecomment-599243492

Comment:
> SIGPWR is sent to init (process 1) directly, so no need to create any files (like stopit). Runit also reboots on getting SIGCONT (it is also not configurable)

Its configurable through the `/etc/runit/stopit` file. If the file doesn't exist or is not `S_IXUSR`, then runit won't shutdown.
This makes sure only `runit-init 0` can bring down runit, not a random SIGCONT or SIGINT. 

Re: runit: add support for SIGPWR signal

[Duncaen]

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

New comment by Duncaen on void-packages repository

https://github.com/void-linux/void-packages/pull/20098#issuecomment-599243492

Comment:
> SIGPWR is sent to init (process 1) directly, so no need to create any files (like stopit). Runit also reboots on getting SIGCONT (it is also not configurable)

Its configurable through the `/etc/runit/stopit` file. If the file doesn't exist or is not `S_IXUSR`, then runit won't shutdown.
This makes sure only `runit-init 0` can bring down runit, not a random SIGCONT or SIGINT.
Edit: this is as far as I know, the whole deal about `stopit`, otherwise only `reboot` would exist.

Re: runit: add support for SIGPWR signal

[Duncaen]

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

New comment by Duncaen on void-packages repository

https://github.com/void-linux/void-packages/pull/20098#issuecomment-599243492

Comment:
> SIGPWR is sent to init (process 1) directly, so no need to create any files (like stopit). Runit also reboots on getting SIGCONT (it is also not configurable)

Its configurable through the `/etc/runit/stopit` file. If the file doesn't exist or is not `S_IXUSR`, then runit won't shutdown.
This makes sure only `runit-init 0` can bring down runit, not a random SIGCONT or SIGINT.

Edit: this is as far as I know, the whole deal about `stopit`, otherwise only `reboot` would exist.

Re: runit: add support for SIGPWR signal

[Duncaen]

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

New comment by Duncaen on void-packages repository

https://github.com/void-linux/void-packages/pull/20098#issuecomment-599243492

Comment:
> SIGPWR is sent to init (process 1) directly, so no need to create any files (like stopit). Runit also reboots on getting SIGCONT (it is also not configurable)

Its configurable through the `/etc/runit/stopit` file. If the file doesn't exist or is not `S_IXUSR`, then runit won't shutdown.
This makes sure only `runit-init 0` can bring down runit, not a random SIGCONT or SIGINT.

Edit: this is as far as I know the whole deal about `stopit`, otherwise only `reboot` would exist.

Re: runit: add support for SIGPWR signal

[Duncaen]

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

New comment by Duncaen on void-packages repository

https://github.com/void-linux/void-packages/pull/20098#issuecomment-599243492

Comment:
> SIGPWR is sent to init (process 1) directly, so no need to create any files (like stopit). Runit also reboots on getting SIGCONT (it is also not configurable)

Its configurable through the `/etc/runit/stopit` file. If the file doesn't exist or is not `S_IXUSR`, then runit won't shutdown.
This makes sure only `runit-init 0` can bring down runit, not a random `SIGCONT` or `SIGINT`.

Edit: this is as far as I know the whole deal about `stopit`, otherwise only `reboot` would exist.

Re: runit: add support for SIGPWR signal

[Duncaen]

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

New comment by Duncaen on void-packages repository

https://github.com/void-linux/void-packages/pull/20098#issuecomment-599244373

Comment:
```diff
-      if ((sigp) || (sigc && (stat(STOPIT, &s) != -1) && (s.st_mode & S_IXUSR))) {
+      if ((sigc || sigp) && (stat(STOPIT, &s) != -1) && (s.st_mode & S_IXUSR))) {
```
If we change the diff like this, on Void Linux the default would be to do the shutdown, as `stopit`
is created on boot (which is another issue, but its easy to remove from `/etc/rc.local`):
```
/etc/runit/1:install -m100 /dev/null /run/runit/stopit
```

Re: runit: add support for SIGPWR signal

[Duncaen]

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

New comment by Duncaen on void-packages repository

https://github.com/void-linux/void-packages/pull/20098#issuecomment-599244373

Comment:
```diff
-      if ((sigp) || (sigc && (stat(STOPIT, &s) != -1) && (s.st_mode & S_IXUSR))) {
+      if ((sigc || sigp) && (stat(STOPIT, &s) != -1) && (s.st_mode & S_IXUSR)) {
```
If we change the diff like this, on Void Linux the default would be to do the shutdown, as `stopit`
is created on boot (which is another issue, but its easy to remove from `/etc/rc.local`):
```
/etc/runit/1:install -m100 /dev/null /run/runit/stopit
```

Re: runit: add support for SIGPWR signal

[innerspacepilot]

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

New comment by innerspacepilot on void-packages repository

https://github.com/void-linux/void-packages/pull/20098#issuecomment-599395364

Comment:
@Duncaen  Do you need this as a quick fix for users if something goes wrong?
I am going to switch to debian's patch

Re: runit: add support for SIGPWR signal

[innerspacepilot]

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

New comment by innerspacepilot on void-packages repository

https://github.com/void-linux/void-packages/pull/20098#issuecomment-599395364

Comment:
@Duncaen  Do you need this as a quick fix for users if something goes wrong?

Re: runit: add support for SIGPWR signal

[innerspacepilot]

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

New comment by innerspacepilot on void-packages repository

https://github.com/void-linux/void-packages/pull/20098#issuecomment-599437065

Comment:
updated patch with stuff from debian
included fix from @Duncaen. Seems reasonable.

Re: runit: add support for SIGPWR signal

[xtraeme]

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

New comment by xtraeme on void-packages repository

https://github.com/void-linux/void-packages/pull/20098#issuecomment-600593120

Comment:
@void-linux/pkg-committers any comment?

Re: runit: add support for SIGPWR signal

[Gottox]

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

New comment by Gottox on void-packages repository

https://github.com/void-linux/void-packages/pull/20098#issuecomment-601175124

Comment:
I haven't done in depth testing, but it look reasonable.

Re: runit: add support for SIGPWR signal

[Duncaen]

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

New comment by Duncaen on void-packages repository

https://github.com/void-linux/void-packages/pull/20098#issuecomment-601189877

Comment:
I'm not completely happy with it, I would like to have a mechanism to completely disable sigpwr for non containers and users who may run a UPS that sends sigpwr but don't want this to shutdown the system or handle in some other way.

This also requires man page updates, we shouldn't just change how init behaves when it is signaled without documenting it, and considering edge cases.

I would add `/etc/runit/sigpwr` which is checked for for existence and executable bit.
Create it from our core services, and `chmod u+x` it if it is a "container" system.

This would not change anything for normal users, introduce the wanted behavior for "containers" and allows users with USPs that send a signal to pid 1 to enable it by `chmod u+x` the file.

Re: runit: add support for SIGPWR signal

[Duncaen]

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

New comment by Duncaen on void-packages repository

https://github.com/void-linux/void-packages/pull/20098#issuecomment-601189877

Comment:
I'm not completely happy with it, I would like to have a mechanism to completely disable sigpwr for non containers and users who may run a UPS that sends sigpwr but don't want this to shutdown the system or handle in some other way.

This also requires man page updates, we shouldn't just change how init behaves when it is signaled without documenting it and considering edge cases.

I would add `/etc/runit/sigpwr` which is checked for for existence and executable bit.
Create it from our core services, and `chmod u+x` it if it is a "container" system.

This would not change anything for normal users, introduce the wanted behavior for "containers" and allows users with USPs that send a signal to pid 1 to enable it by `chmod u+x` the file.

Re: runit: add support for SIGPWR signal

[xtraeme]

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

New comment by xtraeme on void-packages repository

https://github.com/void-linux/void-packages/pull/20098#issuecomment-601192122

Comment:
@Duncaen I agree with your suggestion

Re: runit: add support for SIGPWR signal

[innerspacepilot]

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

New comment by innerspacepilot on void-packages repository

https://github.com/void-linux/void-packages/pull/20098#issuecomment-601558862

Comment:
Well, that's your decision.

Re: [PR PATCH] [Closed]: runit: add support for SIGPWR signal

[innerspacepilot]

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

There's a closed pull request on the void-packages repository

runit: add support for SIGPWR signal
https://github.com/void-linux/void-packages/pull/20098

Description:
Hello!
This patch adds support for SIGPWR signal for runit. On receiving this signal runit gracefully shuts down machine.
This is needed for LXD containers running runit to shut down properly without any need for special configuration.
Seems like nothing else sends SIGPWR to process 0, so there should be no impact on normal operation, but some sort of testing should be done.
There was a huge conversation about it [here](https://www.mail-archive.com/supervision@list.skarnet.org/)

Re: runit: add support for SIGPWR signal

[xtraeme]

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

New comment by xtraeme on void-packages repository

https://github.com/void-linux/void-packages/pull/20098#issuecomment-599208082

Comment:
@duncaen?