[systemd-devel] [PATCH] [RFC] Make reboot to support additional command
Zbigniew Jędrzejewski-Szmek
zbyszek at in.waw.pl
Tue Nov 5 22:53:28 PST 2013
On Wed, Nov 06, 2013 at 03:22:49PM +0900, WaLyong Cho wrote:
>
>
> On 09/12/2013 02:20 AM, Lennart Poettering wrote:
> > On Tue, 13.08.13 03:01, WaLyong Cho (fyd0706 at gmail.com) wrote:
> >
> >> From: WaLyong Cho <walyong.cho at samsung.com>
> >>
> >> reboot syscall can be performed with additional argument. In some of
> >> system, this functionality can be useful to ask next boot mode to
> >> bootloader.
> >
> > Hmm, interesting stuff.
> >
> >> @@ -5200,9 +5204,19 @@ static int halt_parse_argv(int argc, char *argv[]) {
> >> }
> >> }
> >>
> >
> > Note that halt is not called during normal shutdown, but is only a
> > compat fallback for the old sysv "halt" command. If you want this to
> > work we probably need to come up with a way to pass the string from
> > systemctl to the final systemd-shutdown process.
> >
> > (A bit of background: systemctl normally will just tell PID 1 to
> > shutdown cleanly, via D-Bus. As last step of that PID 1 will execute the
> > special systemd-shutdown binary -- which will then also run as PID 1 --
> > which will do some final shutdown steps and then invoke the reboot()
> > syscall -- see src/core/shutdown.c for details)
>
> Sorry for my long silence.
>
> You were right. Previous my patch did not pass the argument without
> force option. So I modified according to your opinion. But I think we
> have to keep some codes in systemctl for supporting forced reboot.
>
> >
> > So, before we continue with this, could you point me to some
> > docs/commits/code which explain a bit how the new reboot() syscall
> > semantics work? i.e. what the possible params are and such?
>
> There are not any params specified. Any string can be used. That string
> is used in kernel side.
> http://lxr.linux.no/linux+v3.4/kernel/sys.c#L486
>
> Sorry, there are no visible commits for Tizen in the web. But, for
> simple example, fota, recovery, download and more are used in Tizen.
> fota is used when restart to enter fota updated mode. In the mobile,
> user can download firmware image by on air. And update device firmware
> to downloaded. During this, reboot is performed. See
> http://www.redbend.com/en/products-solutions/fota-updating for detail.
> recovery is used for detecting hacked kernel. I am not sure that I can
> explain more.
> download is used to enter download mode. As you may know, most of
> devices are downloaded(not installed) when the device is produced. At
> this time, special boot mode is used to flush device.
>
> I found similar codes in android.
> https://github.com/CyanogenMod/android_system_core/blob/gingerbread/toolbox/reboot.c
> https://github.com/CyanogenMod/android_system_core/blob/gingerbread/libreboot/reboot.c
>
> >
> > One way to implement this could be to add add a new parameter to
> > systemctl which is written to /run/systemd/reboot-param or so if
> > specified. systemd-shutdown would then look for that file and pass it to
> > the reboot() syscall.
>
> According to man page of reboot. They called this 'arg'.
> http://man7.org/linux/man-pages/man2/reboot.2.htm
> I'm not sure which is more suitable word between 'param' and 'arg'. You
> offered 'param', so I used also 'param'. Because, 'arg' can be confused
> with argc/argv.
>
> >
> >> - if (optind < argc) {
> >> - log_error("Too many arguments.");
> >> - return -EINVAL;
> >> + if (arg_action == ACTION_REBOOT) {
> >> + /* reboot allow just one argument for his command string */
> >> + if (optind+1 == argc)
> >> + arg_reboot = strdup(argv[argc-1]);
> >> + else if (optind+1 < argc) {
> >> + log_error("Too many arguments.");
> >> + return -EINVAL;
> >> + }
> >> + } else {
> >> + if (optind < argc) {
> >> + log_error("Too many arguments.");
> >> + return -EINVAL;
> >> + }
> >> }
> >>
> >> return 1;
> >> @@ -5949,7 +5963,11 @@ static _noreturn_ void halt_now(enum action a) {
> >>
> >> case ACTION_REBOOT:
> >> log_info("Rebooting.");
> >> - reboot(RB_AUTOBOOT);
> >> + if (arg_reboot)
> >> + syscall(SYS_reboot, LINUX_REBOOT_MAGIC1, LINUX_REBOOT_MAGIC2,
> >> + LINUX_REBOOT_CMD_RESTART2, arg_reboot);
> >> + else
> >> + reboot(RB_AUTOBOOT);
> >> break;
> >>
> >> default:
> >
> >
> > Lennart
> >
>
> WaLyong
> From 3e68b25edab207cdeffb267368e24f6c3a5830ae Mon Sep 17 00:00:00 2001
> From: WaLyong Cho <walyong.cho at samsung.com>
> Date: Tue, 5 Nov 2013 17:11:09 +0900
> Subject: [PATCH] make reboot to support additional command
>
> reboot syscall can be performed with additional argument. In some of
> system, this functionality can be useful to ask next boot mode to
> bootloader.
> ---
> Makefile.am | 4 ++-
> src/core/shutdown.c | 15 +++++++---
> src/shared/reboot-param.c | 68 +++++++++++++++++++++++++++++++++++++++++++++
> src/shared/reboot-param.h | 28 +++++++++++++++++++
> src/systemctl/systemctl.c | 43 +++++++++++++++++++++++++---
> 5 files changed, 149 insertions(+), 9 deletions(-)
> create mode 100644 src/shared/reboot-param.c
> create mode 100644 src/shared/reboot-param.h
>
> diff --git a/Makefile.am b/Makefile.am
> index ed61884..db79069 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -752,7 +752,9 @@ libsystemd_shared_la_SOURCES = \
> src/shared/ptyfwd.c \
> src/shared/ptyfwd.h \
> src/shared/net-util.c \
> - src/shared/net-util.h
> + src/shared/net-util.h \
> + src/shared/reboot-param.c \
> + src/shared/reboot-param.h
>
> #-------------------------------------------------------------------------------
> noinst_LTLIBRARIES += \
> diff --git a/src/core/shutdown.c b/src/core/shutdown.c
> index aa9548e..7f82446 100644
> --- a/src/core/shutdown.c
> +++ b/src/core/shutdown.c
> @@ -47,6 +47,7 @@
> #include "watchdog.h"
> #include "killall.h"
> #include "cgroup-util.h"
> +#include "reboot-param.h"
>
> #define FINALIZE_ATTEMPTS 50
>
> @@ -133,9 +134,10 @@ static int pivot_to_new_root(void) {
>
> int main(int argc, char *argv[]) {
> bool need_umount = true, need_swapoff = true, need_loop_detach = true, need_dm_detach = true;
> - bool in_container, use_watchdog = false;
> + bool in_container, use_watchdog, with_param = false;
Why use_watchdog is changed?
> _cleanup_free_ char *line = NULL, *cgroup = NULL;
> char *arguments[3];
> + char param[LINE_MAX];
Please no.
> unsigned retries;
> int cmd, r;
>
> @@ -173,9 +175,11 @@ int main(int argc, char *argv[]) {
>
> in_container = detect_container(NULL) > 0;
>
> - if (streq(argv[1], "reboot"))
> + if (streq(argv[1], "reboot")) {
> cmd = RB_AUTOBOOT;
> - else if (streq(argv[1], "poweroff"))
> + if (reboot_get_param(param, sizeof(param)) > 0)
> + with_param = true;
> + } else if (streq(argv[1], "poweroff"))
> cmd = RB_POWER_OFF;
> else if (streq(argv[1], "halt"))
> cmd = RB_HALT_SYSTEM;
> @@ -337,7 +341,10 @@ int main(int argc, char *argv[]) {
> cmd = RB_AUTOBOOT;
> }
>
> - reboot(cmd);
> + if (with_param)
> + reboot_with_param(param);
> + else
> + reboot(cmd);
>
> if (errno == EPERM && in_container) {
> /* If we are in a container, and we lacked
> diff --git a/src/shared/reboot-param.c b/src/shared/reboot-param.c
> new file mode 100644
> index 0000000..55f64dd
> --- /dev/null
> +++ b/src/shared/reboot-param.c
> @@ -0,0 +1,68 @@
> +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
> +
> +/***
> + This file is part of systemd.
> +
> + Copyright 2013 Samsung Electronics
> +
> + Author: WaLyong Cho <walyong.cho at samsung.com>
> +
> + systemd is free software; you can redistribute it and/or modify it
> + under the terms of the GNU Lesser General Public License as published by
> + the Free Software Foundation; either version 2.1 of the License, or
> + (at your option) any later version.
> +
> + systemd is distributed in the hope that it will be useful, but
> + WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public License
> + along with systemd; If not, see <http://www.gnu.org/licenses/>.
> +***/
> +
> +#include <sys/reboot.h>
> +#include <linux/reboot.h>
> +#include <sys/syscall.h>
> +
> +#include "reboot-param.h"
> +#include "util.h"
> +
> +#define REBOOT_PARAM "/run/systemd/reboot-param"
> +
> +int reboot_set_param(const char *param) {
> + _cleanup_fclose_ FILE *f = NULL;
> +
> + f = fopen(REBOOT_PARAM, "w");
> + if (!f) {
> + log_error("Failed to create %s: %m", REBOOT_PARAM);
> + return -errno;
> + }
> + fputs(param, f);
> + if (fflush(f)) {
> + log_error("Failed to flush %s: %m", REBOOT_PARAM);
> + return -errno;
> + }
> +
> + return 0;
> +}
Please use write_string_file instead.
> +
> +int reboot_get_param(char *param, int len) {
> + _cleanup_fclose_ FILE *f = NULL;
> +
> + f = fopen(REBOOT_PARAM, "r");
> + if (f == NULL )
> + return 0;
> +
> + if (!fgets(param, len, f)) {
> + log_error("Failed to read reboot param: %m");
> + return -errno;
> + }
> +
> + return 1;
> +}
Please use read_one_line_file instead.
With those two functions gone, those two new files will not be necessary.
REBOOT_PARAM can live in defs.h.
> +
> +long int reboot_with_param(const char *param) {
> + return syscall(SYS_reboot, LINUX_REBOOT_MAGIC1, LINUX_REBOOT_MAGIC2,
> + LINUX_REBOOT_CMD_RESTART2, param);
> +}
> diff --git a/src/shared/reboot-param.h b/src/shared/reboot-param.h
> new file mode 100644
> index 0000000..78dcd88
> --- /dev/null
> +++ b/src/shared/reboot-param.h
> @@ -0,0 +1,28 @@
> +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
> +
> +#pragma once
> +
> +/***
> + This file is part of systemd.
> +
> + Copyright 2013 Samsung Electronics
> +
> + Author: WaLyong Cho <walyong.cho at samsung.com>
> +
> + systemd is free software; you can redistribute it and/or modify it
> + under the terms of the GNU Lesser General Public License as published by
> + the Free Software Foundation; either version 2.1 of the License, or
> + (at your option) any later version.
> +
> + systemd is distributed in the hope that it will be useful, but
> + WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public License
> + along with systemd; If not, see <http://www.gnu.org/licenses/>.
> +***/
> +
> +int reboot_set_param(const char *param);
> +int reboot_get_param(char *param, int len);
> +long int reboot_with_param(const char *param);
> diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
> index 9f5e273..6f5700f 100644
> --- a/src/systemctl/systemctl.c
> +++ b/src/systemctl/systemctl.c
> @@ -67,6 +67,7 @@
> #include "path-util.h"
> #include "socket-util.h"
> #include "fileio.h"
> +#include "reboot-param.h"
>
> static char **arg_types = NULL;
> static char **arg_states = NULL;
> @@ -4804,7 +4805,7 @@ static int systemctl_help(void) {
> " emergency Enter system emergency mode\n"
> " halt Shut down and halt the system\n"
> " poweroff Shut down and power-off the system\n"
> - " reboot Shut down and reboot the system\n"
> + " reboot [ARG] Shut down and reboot the system\n"
> " kexec Shut down and reboot the system with kexec\n"
> " exit Request user instance exit\n"
> " switch-root [ROOT] [INIT] Change to a different root file system\n"
> @@ -4818,7 +4819,7 @@ static int systemctl_help(void) {
>
> static int halt_help(void) {
>
> - printf("%s [OPTIONS...]\n\n"
> + printf("%s [OPTIONS...]%s\n\n"
> "%s the system.\n\n"
> " --help Show this help\n"
> " --halt Halt the machine\n"
> @@ -4829,6 +4830,7 @@ static int halt_help(void) {
> " -d --no-wtmp Don't write wtmp record\n"
> " --no-wall Don't send wall message before halt/power-off/reboot\n",
> program_invocation_short_name,
> + arg_action == ACTION_REBOOT ? " [ARG]" : "",
> arg_action == ACTION_REBOOT ? "Reboot" :
> arg_action == ACTION_POWEROFF ? "Power off" :
> "Halt");
> @@ -5235,6 +5237,9 @@ static int systemctl_parse_argv(int argc, char *argv[]) {
> }
>
> static int halt_parse_argv(int argc, char *argv[]) {
> + _cleanup_fclose_ FILE *f = NULL;
Hm, where is f used?
> + int r;
> + const char *param;
>
> enum {
> ARG_HELP = 0x100,
> @@ -5315,6 +5320,28 @@ static int halt_parse_argv(int argc, char *argv[]) {
> }
> }
>
> + param = argv[optind];
> +
> + if (param != NULL) {
> + switch(arg_action) {
> +
> + case ACTION_REBOOT:
> + if (argc-optind != 1)
> + log_error("Too many arguments.");
> + else {
> + if ((r = reboot_set_param(param)) < 0) {
> + log_error("Failed to set reboot param '%s'.", param);
> + return r;
> + }
> + argc--;
> + }
> + break;
> +
> + default:
> + break;
> + }
> + }
> +
> if (optind < argc) {
> log_error("Too many arguments.");
> return -EINVAL;
> @@ -5953,6 +5980,9 @@ done:
>
> static _noreturn_ void halt_now(enum action a) {
>
> + _cleanup_fclose_ FILE *f = NULL;
Hm, where is f used?
> + char param[LINE_MAX];
> +
> /* Make sure C-A-D is handled by the kernel from this
> * point on... */
> reboot(RB_ENABLE_CAD);
> @@ -5970,8 +6000,13 @@ static _noreturn_ void halt_now(enum action a) {
> break;
>
> case ACTION_REBOOT:
> - log_info("Rebooting.");
> - reboot(RB_AUTOBOOT);
> + if (reboot_get_param(param, sizeof(param)) > 0) {
> + log_info("Rebooting with arg '%s'.", param);
> + reboot_with_param(param);
> + } else {
> + log_info("Rebooting.");
> + reboot(RB_AUTOBOOT);
> + }
> break;
>
> default:
Zbyszek
More information about the systemd-devel
mailing list