[systemd-devel] [PATCH] shutdown: avoid calling `kexec` binary unnessecarily

[Zbigniew Jędrzejewski-Szmek]

On Fri, Feb 13, 2015 at 02:18:07PM -0800, Shawn Landden wrote:
> Still use helper when Xen Dom0, to avoid duplicating some hairy code.
> So we don't have any logic to load kexec kernels?
> ---
>  TODO                |  3 ---
>  src/core/shutdown.c | 33 ++++++++++++++++++++-------------
>  2 files changed, 20 insertions(+), 16 deletions(-)
> 
> diff --git a/TODO b/TODO
> index 883b994..68b0af6 100644
> --- a/TODO
> +++ b/TODO
> @@ -85,9 +85,6 @@ Features:
>  * maybe introduce WantsMountsFor=? Usecase:
>    http://lists.freedesktop.org/archives/systemd-devel/2015-January/027729.html
>  
> -* rework kexec logic to use new kexec_file_load() syscall, so that we
> -  don't have to call kexec tool anymore.
You patch does not really change this. We can still use kexec_file_load() to
load a kernel. But removing a call to kexec to actually do the reboot seems good.
So I guess this TODO item should stay.

One comment below.

>  * The udev blkid built-in should expose a property that reflects
>    whether media was sensed in USB CF/SD card readers. This should then
>    be used to control SYSTEMD_READY=1/0 so that USB card readers aren't
> diff --git a/src/core/shutdown.c b/src/core/shutdown.c
> index 71f001a..c64c05d 100644
> --- a/src/core/shutdown.c
> +++ b/src/core/shutdown.c
> @@ -350,26 +350,33 @@ int main(int argc, char *argv[]) {
>          case LINUX_REBOOT_CMD_KEXEC:
>  
>                  if (!in_container) {
> -                        /* We cheat and exec kexec to avoid doing all its work */
> -                        pid_t pid;
> +                        _cleanup_free_ char *param = NULL;
>  
>                          log_info("Rebooting with kexec.");
>  
> -                        pid = fork();
> -                        if (pid < 0)
> -                                log_error_errno(errno, "Failed to fork: %m");
> -                        else if (pid == 0) {
> +                        /* kexec-tools has a bunch of special code to make Xen Dom0 work,
> +                         * otherwise it is only doing stuff we have already done */
> +                        if (access("/proc/xen", F_OK) == 0) {
Wouldn't it be better to use detect_virtualization() here instead of open-coding the check?

> +                                pid_t pid;
> +
> +                                pid = fork();
> +                                if (pid < 0)
> +                                        log_error_errno(errno, "Failed to fork: %m");
> +                                else if (pid == 0) {
> +
> +                                        const char * const args[] = {
> +                                                KEXEC, "-e", NULL
> +                                        };
>  
> -                                const char * const args[] = {
> -                                        KEXEC, "-e", NULL
> -                                };
> +                                        /* Child */
>  
> -                                /* Child */
> +                                        execv(args[0], (char * const *) args);
> +                                        _exit(EXIT_FAILURE);
> +                                } else
> +                                        wait_for_terminate_and_warn("kexec", pid, true);
>  
> -                                execv(args[0], (char * const *) args);
> -                                _exit(EXIT_FAILURE);
>                          } else
> -                                wait_for_terminate_and_warn("kexec", pid, true);
> +                                reboot(cmd);
>                  }
>  
>                  cmd = RB_AUTOBOOT;

Zbyszek