[systemd-devel] [PATCH] systemd: powerd initctl support

[Lennart Poettering]

On Fri, 07.03.14 14:32, Hannes Reinecke (hare at suse.de) wrote:

> Old versions of powerd will be using the initctl fifo to signal
> state changes. To maintain backward compability systemd should
> be interpreting these messages, too.

Yuck. Can't say I am particularly enthusiastic adding compat kludges for
age old sw that so far nobody even noticed was broken with systemd. I'd
much rather get rid of systemd-initctl entirely...

But anyway, let's get this over with:

> +static int send_shutdownd(unsigned delay, char mode, const char *message) {
> +        usec_t t = now(CLOCK_REALTIME) + delay * USEC_PER_MINUTE;

Nitpicking: we try to avoid mixing variable declaration and function calls. It's OK
to initialize variables when you declare them with some fixed value, but
please keep variable declarations and function infocations separate.

>          case INIT_CMD_POWERFAIL:
> +                r = send_shutdownd(2, SD_SHUTDOWN_POWEROFF,
> +                                   "THE POWER IS FAILED! SYSTEM GOING
> DOWN! PLEASE LOG OFF NOW!");

Hmm, this is incorrect english, isn't it? it should be "The power *has*
failed", no?

Do we really have to make this ALL CAPS? I REALLY DON'T LIKE BEING
YELLED AT ALL THE TIME! ;-)

> +                if (r < 0) {
> +                        log_warning("Failed to talk to shutdownd, shutdown cancelled: %s", strerror(-r));
> +                }

Hmm, please remove redundant {} for one-line if blocks, like here. 

> +                return;
>          case INIT_CMD_POWERFAILNOW:
> +                r = send_shutdownd(0, SD_SHUTDOWN_POWEROFF,
> +                                   "THE POWER IS FAILED! LOW BATTERY - EMERGENCY SYSTEM SHUTDOWN!");
> +                if (r < 0) {
> +                        log_warning("Failed to talk to shutdownd, proceeding with immediate shutdown: %s", strerror(-r));
> +                        reboot(RB_ENABLE_CAD);
> +                        reboot(RB_POWER_OFF);

I'd prefer if we'd simply send SIGRTMIN+14 to PID 1. This triggers a
"medium clean" shutdown. It will kill all daemons immediately by force,
but it will still be putting all file systems into a clean state. It's
quick but still quite safe.

Lennart

-- 
Lennart Poettering, Red Hat