Robbie Harwood wrote on 2022-08-23T17:15:42 -0400: > From: Raymund Will <r...@suse.com> [...] > 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 fallbacks to executing the kexec user-space tool directly.
The last sentence should probably read more like: The provision to force a kexec-reboot, in case systemctl kexec fails, must only be used in controlled environments to avoid possible file-system corruption and data-loss. [...] > --- /dev/null > +++ b/grub-core/loader/emu/linux.c > @@ -0,0 +1,180 @@ [...] > +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 }; You might have noticed the change in the first parameter to kexec, which makes a perfect argument for Daniel's request to have that configurable! But implementation would be quite expensive, maybe unless it's strictly restricted to non-whitespace- bearing parameters. Would that be sufficient and viable? > + 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); Well, the original grub_printf() in this case was very helpful to "create" a kexec-load command for cut-n-paste. Is it really necessary to bury it in a ton of debug messages? > + > + if (kexecute) > + rc = grub_util_exec (kexec); > + > + grub_free(initrd_param); > + > + 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_error (rc, N_("Error trying to perform 'systemctl kexec'")); This grub_error() needs to be a grub_fatal() to achieve the intended behavior, right? If kexecute is 1 it should bail out here. Only if it's bigger the forced kexec should be tried! (Note, that "< 1" is already covered above!) > + > + /* need to check read-only root before resetting hard!? */ This comment probably needs to be replaced with a strict one (reflected in GRUB's docs) explaining, that the user takes full responsiblity in "force" being exclusively used in read-only environments, as grub-emu itself simply can't guarantee this. (Any check here would hardly scratch the surface.) > + grub_dprintf ("linux", "Performing 'kexec -e -x'"); > + kexec[1] = "-e"; > + kexec[2] = "-x"; > + kexec[3] = NULL; Provided the kexec-load gets a tunable, this kexec-exec probably deserves one as well (as this '-ex' initially was only a '-e' (see ----vv)). > + rc = grub_util_exec (kexec); > + if ( rc != GRUB_ERR_NONE ) > + grub_fatal (N_("Error trying to directly perform 'kexec -e'.")); > + > + return rc; > +} [...] Thanks Robbie for driving this, as I'm lacking the time... Best, -- Raymund WILL r...@suse.de SUSE Software Solutions Germany GmbH, Frankenstr. 146, D-90461 Nuernberg Geschaeftsfuehrer: Ivo Totev, et al (HRB 36809, AG Nuernberg) _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel