[systemd-devel] [PATCH] [RFC] Make reboot to support additional command
WaLyong Cho
walyong.cho at samsung.com
Wed Nov 6 00:42:23 PST 2013
On 11/06/2013 03:53 PM, Zbigniew Jędrzejewski-Szmek wrote:
> 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
>
There are some of my mistakes.
I updated my patch.
Thank you.
WaLyong
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-make-reboot-to-support-additional-command.patch
Type: text/x-diff
Size: 6710 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/systemd-devel/attachments/20131106/6397abe9/attachment-0001.patch>
More information about the systemd-devel
mailing list