[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 1/1] Add support for grub-emu to kexec Linux menu entries
From: |
Daniel Kiper |
Subject: |
Re: [PATCH v5 1/1] Add support for grub-emu to kexec Linux menu entries |
Date: |
Thu, 20 Oct 2022 15:41:30 +0200 |
User-agent: |
NeoMutt/20170113 (1.7.2) |
On Wed, Oct 19, 2022 at 02:50:00PM -0400, Robbie Harwood wrote:
> From: Raymund Will <rw@suse.com>
>
> The GRUB emulator is used as a debugging utility but it could also be
> used as a user-space bootloader if there is support to boot an operating
> system.
>
> The Linux kernel is already able to (re)boot another kernel via the
> kexec boot mechanism. So the grub-emu tool could rely on this feature
> and have linux and initrd commands that are used to pass a kernel,
> initramfs image and command line parameters to kexec for booting a
> selected menu entry.
>
> By default the systemctl kexec option is used so systemd can shutdown
> all of the running services before doing a reboot using kexec. But if
> this is not present, it can fall back to executing the kexec user-space
> tool directly. The ability to force a kexec-reboot when systemctl kexec
> fails must only be used in controlled environments to avoid possible
> filesystem corruption and data loss.
>
> Signed-off-by: Raymund Will <rw@suse.com>
> Signed-off-by: John Jolly <jjolly@suse.com>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> [rharwood: documentation, code style]
> Signed-off-by: Robbie Harwood <rharwood@redhat.com>
> ---
> docs/grub.texi | 28 ++++--
> grub-core/Makefile.am | 1 +
> grub-core/Makefile.core.def | 2 +-
> grub-core/kern/emu/main.c | 4 +
> grub-core/kern/emu/misc.c | 18 +++-
> grub-core/loader/emu/linux.c | 183 +++++++++++++++++++++++++++++++++++
> include/grub/emu/exec.h | 4 +-
> include/grub/emu/hostfile.h | 3 +-
> include/grub/emu/misc.h | 3 +
> 9 files changed, 233 insertions(+), 13 deletions(-)
> create mode 100644 grub-core/loader/emu/linux.c
>
> diff --git a/docs/grub.texi b/docs/grub.texi
> index af119dea3b..1463e62e6b 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -1,4 +1,4 @@
> -\input texinfo
> +i\input texinfo
> @c -*-texinfo-*-
> @c %**start of header
> @setfilename grub.info
> @@ -923,17 +923,17 @@ magic.
> @node General boot methods
> @section How to boot operating systems
>
> -GRUB has two distinct boot methods. One of the two is to load an
> -operating system directly, and the other is to chain-load another boot
> -loader which then will load an operating system actually. Generally
> -speaking, the former is more desirable, because you don't need to
> -install or maintain other boot loaders and GRUB is flexible enough to
> -load an operating system from an arbitrary disk/partition. However,
> -the latter is sometimes required, since GRUB doesn't support all the
> -existing operating systems natively.
> +GRUB has three distinct boot methods: loading an operating system
> +directly, using kexec from userspace, and chainloading another
> +bootloader. Generally speaking, the first two are more desirable
> +because you don't need to install or maintain other boot loaders and
> +GRUB is flexible enough to load an operating system from an arbitrary
> +disk/partition. However, chainloading is sometimes required, as GRUB
> +doesn't support all existing operating systems natively.
>
> @menu
> * Loading an operating system directly::
> +* Kexec::
> * Chain-loading::
> @end menu
>
> @@ -959,6 +959,16 @@ use more complicated instructions. @xref{DOS/Windows},
> for more
> information.
>
>
> +@node Kexec
> +@subsection Kexec with grub2-emu
> +
> +grub2 can be run in userspace by invoking the grub2-emu tool. It will
Please be consistent, s/grub2/GRUB/ if you use "GRUB" above...
> +read all configuration scripts as if booting directly (see @xref{Loading
> +an operating system directly}). With the @code{--kexec} flag, and
> +kexec(8) support from the operating system, the @command{linux} command
> +will directly boot the target image.
> +
> +
> @node Chain-loading
> @subsection Chain-loading an OS
>
> diff --git a/grub-core/Makefile.am b/grub-core/Makefile.am
> index ee88e44e97..80e7a83edf 100644
> --- a/grub-core/Makefile.am
> +++ b/grub-core/Makefile.am
> @@ -307,6 +307,7 @@ KERNEL_HEADER_FILES +=
> $(top_srcdir)/include/grub/emu/net.h
> KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/emu/hostdisk.h
> KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/emu/hostfile.h
> KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/extcmd.h
> +KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/emu/exec.h
> if COND_GRUB_EMU_SDL
> KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/sdl.h
> endif
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 7159948721..5350408601 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -1816,9 +1816,9 @@ module = {
> arm64 = loader/arm64/linux.c;
> riscv32 = loader/riscv/linux.c;
> riscv64 = loader/riscv/linux.c;
> + emu = loader/emu/linux.c;
> common = loader/linux.c;
> common = lib/cmdline.c;
> - enable = noemu;
> };
>
> module = {
> diff --git a/grub-core/kern/emu/main.c b/grub-core/kern/emu/main.c
> index 44e087e988..855b11c3de 100644
> --- a/grub-core/kern/emu/main.c
> +++ b/grub-core/kern/emu/main.c
> @@ -107,6 +107,7 @@ static struct argp_option options[] = {
> N_("use GRUB files in the directory DIR [default=%s]"), 0},
> {"verbose", 'v', 0, 0, N_("print verbose messages."), 0},
> {"hold", 'H', N_("SECS"), OPTION_ARG_OPTIONAL, N_("wait until a
> debugger will attach"), 0},
> + {"kexec", 'X', 0, 0, N_("use kexec to boot Linux kernels via
> systemctl (pass twice to enable dangerous fallback to non-systemctl)."), 0},
Please document this option in the docs/grub.texi too.
> { 0, 0, 0, 0, 0, 0 }
> };
>
> @@ -164,6 +165,9 @@ argp_parser (int key, char *arg, struct argp_state *state)
> case 'v':
> verbosity++;
> break;
> + case 'X':
> + grub_util_set_kexecute ();
> + break;
>
> case ARGP_KEY_ARG:
> {
> diff --git a/grub-core/kern/emu/misc.c b/grub-core/kern/emu/misc.c
> index d0e7a107e7..521220b49d 100644
> --- a/grub-core/kern/emu/misc.c
> +++ b/grub-core/kern/emu/misc.c
> @@ -39,6 +39,7 @@
> #include <grub/emu/misc.h>
>
> int verbosity;
> +int kexecute;
We have bool type now so I would use it here. Or at least try it does
not break other builds... :-)
> grub_util_warn (const char *fmt, ...)
> @@ -82,7 +83,7 @@ grub_util_error (const char *fmt, ...)
> vfprintf (stderr, fmt, ap);
> va_end (ap);
> fprintf (stderr, ".\n");
> - exit (1);
> + grub_exit ();
> }
>
> void *
> @@ -153,6 +154,9 @@ xasprintf (const char *fmt, ...)
> void
> grub_exit (void)
> {
> +#if defined (GRUB_KERNEL)
> + grub_reboot ();
> +#endif
> exit (1);
> }
> #endif
> @@ -214,3 +218,15 @@ grub_util_load_image (const char *path, char *buf)
>
> fclose (fp);
> }
> +
> +void
> +grub_util_set_kexecute (void)
> +{
> + kexecute++;
> +}
Ditto.
> +int
> +grub_util_get_kexecute (void)
> +{
> + return kexecute;
> +}
Ditto.
> diff --git a/grub-core/loader/emu/linux.c b/grub-core/loader/emu/linux.c
> new file mode 100644
> index 0000000000..f232f195b3
> --- /dev/null
> +++ b/grub-core/loader/emu/linux.c
> @@ -0,0 +1,183 @@
> +/*
> + * GRUB -- GRand Unified Bootloader
> + * Copyright (C) 2022 Free Software Foundation, Inc.
> + *
> + * GRUB is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 3 of the License, or
> + * (at your option) any later version.
> + *
> + * GRUB 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 General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <grub/loader.h>
> +#include <grub/dl.h>
> +#include <grub/command.h>
> +#include <grub/time.h>
> +
> +#include <grub/emu/exec.h>
> +#include <grub/emu/hostfile.h>
> +#include <grub/emu/misc.h>
> +
> +GRUB_MOD_LICENSE ("GPLv3+");
> +
> +static grub_dl_t my_mod;
> +
> +static char *kernel_path;
> +static char *initrd_path;
> +static char *boot_cmdline;
> +
> +static grub_err_t
> +grub_linux_boot (void)
> +{
> + grub_err_t rc = GRUB_ERR_NONE;
> + char *initrd_param;
> + const char *kexec[] = {"kexec", "-la", kernel_path, boot_cmdline, NULL,
> + NULL};
Please do not wrap this line. I am fine with lines a bit longer than 80.
> + const char *systemctl[] = {"systemctl", "kexec", NULL};
> + int kexecute = grub_util_get_kexecute ();
> +
> + if (initrd_path)
> + {
> + initrd_param = grub_xasprintf ("--initrd=%s", initrd_path);
> + kexec[3] = initrd_param;
> + kexec[4] = boot_cmdline;
> + }
> + else
> + initrd_param = grub_xasprintf ("%s", "");
> +
> + grub_dprintf ("linux", "%serforming 'kexec -la %s %s %s'\n",
> + (kexecute) ? "P" : "Not p",
> + kernel_path, initrd_param, boot_cmdline);
> +
> + if (kexecute)
> + rc = grub_util_exec (kexec);
> +
> + grub_free(initrd_param);
Missing space before "("...
> + if (rc != GRUB_ERR_NONE)
> + {
> + grub_error (rc, N_("error trying to perform kexec load operation"));
> + grub_sleep (3);
> + return rc;
> + }
> +
> + if (kexecute < 1)
> + grub_fatal (N_("use '"PACKAGE"-emu --kexec' to force a system restart"));
> +
> + grub_dprintf ("linux", "Performing 'systemctl kexec' (%s) ",
> + (kexecute==1) ? "do-or-die" : "just-in-case");
> + rc = grub_util_exec (systemctl);
> +
> + if (kexecute == 1)
> + grub_fatal (N_("error trying to perform 'systemctl kexec': %d"), rc);
> +
> + /*
> + * WARNING: forcible reset should only be used in read-only environments.
> + * grub-emu cannot check for these - users beware.
> + */
This behavior has to be properly documented in the docs/grub.texi too.
> + grub_dprintf ("linux", "Performing 'kexec -e -x'");
> + kexec[1] = "-e";
> + kexec[2] = "-x";
You glue two options above. So, why do not do the same here, i.e. kexec[1] =
"-ex"?
> + kexec[3] = NULL;
> + rc = grub_util_exec (kexec);
> + if ( rc != GRUB_ERR_NONE )
Too many spaces here...
> + grub_fatal (N_("error trying to directly perform 'kexec -e': %d"), rc);
> +
> + return rc;
> +}
> +
> +static grub_err_t
> +grub_linux_unload (void)
> +{
> + /* Unloading: we're no longer in use. */
Thanks for adding this comment and ones below...
> + grub_dl_unref (my_mod);
> + grub_free (boot_cmdline);
> + boot_cmdline = NULL;
> + return GRUB_ERR_NONE;
> +}
> +
> +static grub_err_t
> +grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)), int argc,
> + char *argv[])
> +{
> + int i;
> + char *tempstr;
> +
> + /* Mark ourselves as in-use. */
> + grub_dl_ref (my_mod);
> +
> + if (argc == 0)
> + return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected"));
> +
> + if (!grub_util_is_regular (argv[0]))
> + return grub_error (GRUB_ERR_FILE_NOT_FOUND,
> + N_("cannot find kernel file %s"), argv[0]);
> +
> + grub_free (kernel_path);
> + kernel_path = grub_xasprintf ("%s", argv[0]);
> +
> + grub_free (boot_cmdline);
> + boot_cmdline = NULL;
> +
> + if (argc > 1)
> + {
> + boot_cmdline = grub_xasprintf ("--command-line=%s", argv[1]);
> + for ( i = 2; i < argc; i++ )
Too many spaces...
> + {
> + tempstr = grub_xasprintf ("%s %s", boot_cmdline, argv[i]);
> + grub_free (boot_cmdline);
> + boot_cmdline = tempstr;
> + }
> + }
> +
> + grub_loader_set (grub_linux_boot, grub_linux_unload, 0);
> +
> + return GRUB_ERR_NONE;
> +}
> +
> +static grub_err_t
> +grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)), int argc,
> + char *argv[])
> +{
> + if (argc == 0)
> + return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected"));
> +
> + if (!grub_util_is_regular (argv[0]))
> + return grub_error (GRUB_ERR_FILE_NOT_FOUND,
> + N_("Cannot find initrd file %s"), argv[0]);
> +
> + grub_free (initrd_path);
> + initrd_path = grub_xasprintf("%s", argv[0]);
Missing space...
> +
> + /* We are done - mark ourselves as on longer in use. */
> + grub_dl_unref (my_mod);
> +
> + return GRUB_ERR_NONE;
> +}
> +
> +static grub_command_t cmd_linux, cmd_initrd;
> +
> +GRUB_MOD_INIT (linux)
> +{
> + cmd_linux = grub_register_command ("linux", grub_cmd_linux, 0,
> + N_("Load Linux."));
> + cmd_initrd = grub_register_command ("initrd", grub_cmd_initrd, 0,
> + N_("Load initrd."));
> + my_mod = mod;
> + kernel_path = NULL;
> + initrd_path = NULL;
> + boot_cmdline = NULL;
You can drop these three lines and initialize variables in the
definition above.
Daniel