Hi Jakob,
Jakob Kirsch <jakob.kir...@web.de> skribis:
> This patch adds preliminary support for rebooting into kexec using `reboot
> -k` or `reboot --do-kexec`.
Nice! That sounds useful.
Some comments inline below. In the future, could you send patches to
guix-patc...@gnu.org, with “Shepherd” in the subject?
> --- a/configure.ac
> +++ b/configure.ac
> @@ -104,6 +104,7 @@ case "$host_os" in
> AC_COMPUTE_INT([RB_DISABLE_CAD], [RB_DISABLE_CAD], [#include
> <sys/reboot.h>])
> AC_COMPUTE_INT([RB_POWER_OFF], [RB_POWER_OFF], [#include <sys/reboot.h>])
> AC_COMPUTE_INT([RB_SW_SUSPEND], [RB_SW_SUSPEND], [#include
> <sys/reboot.h>])
> + AC_COMPUTE_INT([RB_KEXEC], [RB_KEXEC], [#include <sys/reboot.h>])
> ;;
> gnu*)
> # On GNU/Hurd, the Mach-derived reboot.h uses different names, and
Please also handle the Hurd case, similar to how ‘RB_DISABLE_CAD’ is
handled.
> +++ b/modules/shepherd/scripts/reboot.scm
> @@ -30,6 +30,8 @@
> (define (main . args)
> (initialize-cli)
>
> + (define reboot-action 'stop)
Or just ‘action’?
> + #:action (lambda () (set! reboot-action 'reboot-kexec))
… and just ‘kexec’?
> + (reboot-kexec
> + "Reboot the system and run kexec."
Likewise, I’d call it just ‘kexec’.
Could you expand the docstring a bit to give an overview of what “kexec”
does for users not familiar with it?
> +(define (reboot-kexec)
> + "Execute kernel loaded with 'kexec -l' now. Return #f on failure."
“Execute the Linux kernel previously loaded with 'kexec -l'. Throw an
exception on failure.”
> + (%libc-reboot RB_KEXEC))
Please address the case where RB_KEXEC is #f by doing something like:
(throw 'system-error 'kexec "~A" (list (strerror ENOSYS))
(list ENOSYS))
Could you also update ‘doc/shepherd.texi’, specifically the reference of
the ‘reboot’ command and the list of actions of the ‘root’ service?
Thanks for this work!
Ludo’.
