Add -k option to shutdown for kexec functionality

[el-remph]

Adds a -k option to 'shutdown', analogous to that of systemd and OpenRC. Am daily driving this and it gets a nice speedup on reboot. Also tried to make the ifdef-ing out of shutdown code based on RB_* constants more consistent, so the user can clearly see which shutdown modes are supported (since kexec may not be available at compile time on very old environments).

The only real downside that I can see is if the user has neglected to load an image before calling 'reboot -k', then reboot will return -1 with EINVAL, init will exit and the kernel will panic. I can't see an easy way to avoid this, since we only find out that the user has remembered to load an image when we call reboot() and it doesn't return.

[davmac314]

In principle I'm fine with adding this feature (though in general: please remember to discuss new features before implementing them, eg. open a discussion or issue or send an email).

I think in general the handling of (and documentation for) unsupported shutdown types needs to be tightened up. If I'm reading the changes correctly:

• The -k option (kexec) will be listed in shutdown --help only if RB_KEXEC is defined
• But it will be accepted regardless (generating an "Unsupported shutdown type" error message if RB_KEXEC is not defined). I guess that's not terrible, but it's a bit of an odd implementation choice. Why not #ifdef out the functionality the same as the help message?
• And, the -k option will always be listed in the man page (ideally the man page would reflect the actual status, but at least it should mention that the option is not supported on all systems).
• Help for other shutdown types are now also guarded with #ifdef eg "halt" requires either RB_HALT_SYSTEM or RB_HALT. Is that a real concern? I.e. are there any systems where shutdown will compile and be useful but which will not have those available?

There are a few other minor niggles as well but these points are the main ones to be addressed, I think.

[el-remph]

In principle I'm fine with adding this feature (though in general: please remember to discuss new features before implementing them, eg. open a discussion or issue or send an email).

Sorry about that; I implemented this for my own convenience initially (on a Dell with the world's slowest EFI), then thought I might as well share the changes if they work.

But it will be accepted regardless (generating an "Unsupported shutdown type" error message if RB_KEXEC is not defined). I guess that's not terrible, but it's a bit of an odd implementation choice. Why not #ifdef out the functionality the same as the help message?

The "Unsupported shutdown type" is followed by return 1 from main, so the shutdown utility doesn't accept the -k option. However, the dinit process does accept messages of shutdown_type_t::KEXEC (which could come from eg. a shutdown utility from a different build), following Postel's law, and attempts to exec shutdown -k. This may work (if the shutdown utility that issued the bad request is on the PATH), or may cause a kernel panic. Now that I say that, that is a pretty bad idea.

The existing default on receiving an unrecognised shutdown_type from the socket was to poweroff, which probably isn't what the user meant. Should dinit reject an unsupported shutdown_type_t::KEXEC, which could seem like stubbornly refusing to reboot? Or should it try to do the next best thing and reboot, which can't be cancelled if the user really only wanted to kexec?

And, the -k option will always be listed in the man page (ideally the man page would reflect the actual status, but at least it should mention that the option is not supported on all systems).

Will fix

Help for other shutdown types are now also guarded with #ifdef eg "halt" requires either RB_HALT_SYSTEM or RB_HALT. Is that a real concern? I.e. are there any systems where shutdown will compile and be useful but which will not have those available?

I don't know, I presumed so because the ifdefs are there so they must have been useful somewhere. I wanted to keep the ifdefery consistent between the help message and the option parsing, and between the old options and -k.

[davmac314]

The "Unsupported shutdown type" is followed by return 1 from main, so the shutdown utility doesn't accept the -k option.

Maybe "accept" is ambiguous, what I meant was that it recognises the option. Anyway, I guess that's ok.

I don't know, I presumed so because the ifdefs are there

Ah right, they are indeed already there in the actual shutdown. But there they degrade more gracefully: if a requested shutdown type isn't supported, it falls back to a reboot, so the changes in the PR do change that: there would no longer be such a fallback.

This may work (if the shutdown utility that issued the bad request is on the PATH), or may cause a kernel panic. Now that I say that, that is a pretty bad idea.

Right; if --system is supplied to shutdown, I don't think it should outright reject unhandled shutdown types. The previous behaviour, where it falls back to reboot if an unsupported type is used, is preferable and I think it should be maintained.

[davmac314]

I did a thorough review this time. There shouldn't be anything else other than what I've highlighted here. Thanks

[davmac314]

I will nitpick a bit with the wording here:

"Don't forget to load a kernel image with \fBkexec\fR(8) first!" is not really the right tone. Better would be something like:

A suitable kernel image must be loaded first; see \fBkexec\fR(8).

Similarly, "only works" (on Linux 2.6.13 ...) would be better as "is only supported" (...). Rather than with \fBCONFIG_KEXEC=y\fR I think it would be better with something like built with suitable support (the CONFIG_KEXEC mention is probably meaningless to someone who hasn't built a kernel).

Can we remove or rephrase "if you're unsure, most distribution kernels qualify" - it's not really helpful (and the tone also doesn't match).

Ends of sentences should be followed by a newline (the other cases in this man page aren't correct either; I'll fix them separately). Apparently it can affect spacing when the man pages are converted to a printable format.

[davmac314]

Remove this, unless there's a good reason for it that I can't see?

[davmac314]

Remove (2) after reboot in the error message, that's not done elsewhere.

[davmac314]

The message should be made specific to the shutdown type, i.e. "if you try to kexec without loading an image" message should only be shown if the shutdown type is KEXEC.

Also check tone. Eg: "It is possible that no suitable kernel image was loaded before shutdown (with kexec) was issued" is more suitable.

[davmac314]

This should really be calling loop.run(), until sub_buf is drained (or perhaps forever in an infinite loop, to avoid a kernel panic).

Calling loop.poll() only checks if output is possible immediately, once. In the case the console is a serial tty for example, it might not be, and in that case, the important error message will never be shown before the kernel panics.

[davmac314]

match style used elsewhere - the return type should be on the same line as the function name.

[davmac314]

match style used elsewhere - case clause should be on a line by itself, put return true; on the next line (suitably indented). Same with the other case clauses and default clause.