<div dir="ltr">The bootloader spec does not say which entry is to be the default. I cannot support the spec unless I can choose a single default kernel.<div><br></div><div><a href="http://freedesktop.org/wiki/Specifications/BootLoaderSpec/">http://freedesktop.org/wiki/Specifications/BootLoaderSpec/</a><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Feb 17, 2015 at 7:00 AM, Zbigniew JÄ™drzejewski-Szmek <span dir="ltr"><<a href="mailto:zbyszek@in.waw.pl" target="_blank">zbyszek@in.waw.pl</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Mon, Feb 16, 2015 at 07:53:47PM -0800, Shawn Landden wrote:<br>
> On Mon, Feb 16, 2015 at 5:08 AM, Lennart Poettering <<a href="mailto:lennart@poettering.net">lennart@poettering.net</a>><br>
> wrote:<br>
><br>
> > On Fri, 13.02.15 14:18, Shawn Landden (<a href="mailto:shawn@churchofgit.com">shawn@churchofgit.com</a>) wrote:<br>
> ><br>
> > > Still use helper when Xen Dom0, to avoid duplicating some hairy<br>
> > > code.<br>
> ><br>
> > Hmm, what precisely does the helper do on xen?<br>
> ><br>
> > > So we don't have any logic to load kexec kernels?<br>
> ><br>
> > Currently we don't.<br>
> ><br>
> > My hope though was that we can make the whole kexec thing work without<br>
> > having kexec-tools installed. With the new kernel syscall for loading<br>
> > the kernel we should be able to implement this all without any other<br>
> > tools.<br>
> ><br>
> > Ideally we'd not use any external tools anymore, not even in the Xen<br>
> > case, hence I am curious what precisely the special hookup for Xen is<br>
> > here...?<br>
> ><br>
> > Lennart<br>
> ><br>
> ><br>
> <a href="https://git.kernel.org/cgit/utils/kernel/kexec/kexec-tools.git/tree/kexec/kexec-xen.c" target="_blank">https://git.kernel.org/cgit/utils/kernel/kexec/kexec-tools.git/tree/kexec/kexec-xen.c</a><br>
><br>
> I've attached my patch.<br>
> I'm having a problem with kexec_file_load() returning ENOMEM that I havn't<br>
> resolved.<br>
><br>
> > --<br>
> > Lennart Poettering, Red Hat<br>
> > _______________________________________________<br>
> > systemd-devel mailing list<br>
> > <a href="mailto:systemd-devel@lists.freedesktop.org">systemd-devel@lists.freedesktop.org</a><br>
> > <a href="http://lists.freedesktop.org/mailman/listinfo/systemd-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/systemd-devel</a><br>
> ><br>
><br>
><br>
><br>
> --<br>
> Shawn Landden<br>
<br>
</div></div>> From d5446e324143f55e67bc2defe1c78a4ea4201142 Mon Sep 17 00:00:00 2001<br>
> From: Shawn Landden <<a href="mailto:shawn@churchofgit.com">shawn@churchofgit.com</a>><br>
> Date: Fri, 13 Feb 2015 13:48:07 -0800<br>
> Subject: [PATCH] shutdown: avoid calling `kexec` binary unnessecarily<br>
<span class="">><br>
> Still use helper when Xen Dom0, to avoid duplicating some hairy code.<br>
</span><span class="">> So we don't have any logic to load kexec kernels?<br>
</span>> ---<br>
>  TODO  Â  Â  Â  Â  Â  Â  Â  Â |  Â 3 --<br>
>  src/core/shutdown.c  | 121 ++++++++++++++++++++++++++++++++++++++++++++++-----<br>
>  src/shared/missing.h |  Â 7 +++<br>
>  3 files changed, 116 insertions(+), 15 deletions(-)<br>
><br>
> diff --git a/TODO b/TODO<br>
> index 255a4f2..d914d2c 100644<br>
> --- a/TODO<br>
> +++ b/TODO<br>
> @@ -90,9 +90,6 @@ Features:<br>
<span class="">>  * maybe introduce WantsMountsFor=? Usecase:<br>
>  Â  <a href="http://lists.freedesktop.org/archives/systemd-devel/2015-January/027729.html" target="_blank">http://lists.freedesktop.org/archives/systemd-devel/2015-January/027729.html</a><br>
><br>
> -* rework kexec logic to use new kexec_file_load() syscall, so that we<br>
> -  don't have to call kexec tool anymore.<br>
> -<br>
>  * The udev blkid built-in should expose a property that reflects<br>
>  Â  whether media was sensed in USB CF/SD card readers. This should then<br>
>  Â  be used to control SYSTEMD_READY=1/0 so that USB card readers aren't<br>
> diff --git a/src/core/shutdown.c b/src/core/shutdown.c<br>
</span>> index 71f001a..14febf3 100644<br>
> --- a/src/core/shutdown.c<br>
> +++ b/src/core/shutdown.c<br>
> @@ -19,6 +19,7 @@<br>
>  Â  along with systemd; If not, see <<a href="http://www.gnu.org/licenses/" target="_blank">http://www.gnu.org/licenses/</a>>.<br>
>  ***/<br>
><br>
> +#include <ctype.h><br>
>  #include <sys/mman.h><br>
>  #include <sys/types.h><br>
>  #include <sys/reboot.h><br>
> @@ -27,6 +28,7 @@<br>
>  #include <sys/stat.h><br>
>  #include <sys/mount.h><br>
>  #include <sys/syscall.h><br>
> +#include <sys/utsname.h><br>
>  #include <fcntl.h><br>
>  #include <dirent.h><br>
>  #include <errno.h><br>
> @@ -138,6 +140,35 @@ static int parse_argv(int argc, char *argv[]) {<br>
>  Â  Â  Â  Â  return 0;<br>
>  }<br>
><br>
> +/*<br>
> + * Remove parameter from a kernel command line. Helper function for kexec.<br>
> + * (copied from kexec-tools)<br>
> + */<br>
> +static void remove_parameter(char *line, const char *param_name) {<br>
> +  Â  Â  Â  char *start, *end;<br>
> +<br>
> +  Â  Â  Â  start = strstr(line, param_name);<br>
> +<br>
> +  Â  Â  Â  /* parameter not found */<br>
> +  Â  Â  Â  if (!start)<br>
> +  Â  Â  Â  Â  Â  Â  Â  return;<br>
> +<br>
> +  Â  Â  Â  /*<br>
> +  Â  Â  Â  Â * check if that's really the start of a parameter and not in<br>
> +  Â  Â  Â  Â * the middle of the word<br>
> +  Â  Â  Â  Â */<br>
> +  Â  Â  Â  if (start != line && !isspace(*(start-1)))<br>
> +  Â  Â  Â  Â  Â  Â  Â  return;<br>
> +<br>
> +  Â  Â  Â  end = strstr(start, " ");<br>
> +  Â  Â  Â  if (!end)<br>
> +  Â  Â  Â  Â  Â  Â  Â  *start = 0;<br>
> +  Â  Â  Â  else {<br>
> +  Â  Â  Â  Â  Â  Â  Â  memmove(start, end+1, strlen(end));<br>
> +  Â  Â  Â  Â  Â  Â  Â  *(end + strlen(end)) = 0;<br>
> +  Â  Â  Â  }<br>
> +}<br>
> +<br>
>  static int switch_root_initramfs(void) {<br>
>  Â  Â  Â  Â  if (mount("/run/initramfs", "/run/initramfs", NULL, MS_BIND, NULL) < 0)<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  return log_error_errno(errno, "Failed to mount bind /run/initramfs on /run/initramfs: %m");<br>
> @@ -152,6 +183,57 @@ static int switch_root_initramfs(void) {<br>
>  Â  Â  Â  Â  return switch_root("/run/initramfs", "/oldroot", false, MS_BIND);<br>
>  }<br>
><br>
> +static int kernel_load(bool overwrite) {<br>
> +  Â  Â  Â  int r = -ENOTSUP;<br>
> +<br>
> +/* only x86_64 and x32 in 3.18 */<br>
> +#ifdef __NR_kexec_file_load<br>
> +  Â  Â  Â  Â  Â  Â  Â  /* If uses has no already loaded a kexec kernel assume they<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â * want the same one they are currently running. */<br>
> +  Â  Â  Â  Â  Â  Â  Â  if (!overwrite && !kexec_loaded()) {<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  struct utsname u;<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  _cleanup_free_ char *vmlinuz = NULL, *initrd = NULL, *cmdline = NULL;<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  _cleanup_close_ int vmlinuz_fd = -1, initrd_fd = -1;<br>
> +<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  r = uname(&u);<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  if (r < 0)<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  return -errno;<br>
> +<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  vmlinuz = malloc(strlen("/boot/vmlinuz-") + strlen(u.release) + 1);<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  initrd  = malloc(strlen("/boot/initrd.img-") + strlen(u.release) + 1);<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  if (!vmlinuz || !initrd)<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  return -ENOMEM;<br>
> +<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  r = read_one_line_file("/proc/cmdline", &cmdline);<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  if (r < 0)<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  return r;<br>
> +<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  strcpy(stpcpy(vmlinuz, "/boot/vmlinuz-"),  Â  u.release);<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  strcpy(stpcpy(initrd,  "/boot/initrd.img-"), u.release);<br>
Wouldn't a simple strjoina work just as well here?<br>
<br>
We also have the boot loader specification. We really need to follow<br>
it here, and look for kernels in any place where we would install them.<br>
So this scheme most likely would have to be more comples.<br>
<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  if (access(vmlinuz, R_OK) != 0 || access(initrd, R_OK) != 0)<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  return -errno;<br>
No need to to do this check, the open()s below work just as well.<br>
<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  vmlinuz_fd = open(vmlinuz, O_RDONLY);<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  if (vmlinuz_fd < 0)<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  return -errno;<br>
> +<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  initrd_fd  = open(initrd,  O_RDONLY);<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  if (initrd_fd < 0)<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  return -errno;<br>
> +<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  /* clean up cmdline like kexec-tools does */<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  remove_parameter(cmdline, "BOOT_IMAGE");<br>
> +<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  log_info("kexec: kexec -l %s --initrd=%s --command-line=%s", vmlinuz, initrd, cmdline);<br>
> +<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  r = syscall(__NR_kexec_file_load, vmlinuz_fd, initrd_fd, cmdline, strlen(cmdline), 0);<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  if (r < 0)<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  return -errno;<br>
> +  Â  Â  Â  Â  Â  Â  Â  } else<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  r = 0;<br>
> +#endif<br>
> +  Â  Â  Â  return r;<br>
> +}<br>
<span class="">><br>
>  int main(int argc, char *argv[]) {<br>
</span>>  Â  Â  Â  Â  bool need_umount, need_swapoff, need_loop_detach, need_dm_detach;<br>
> @@ -175,11 +257,13 @@ int main(int argc, char *argv[]) {<br>
><br>
>  Â  Â  Â  Â  umask(0022);<br>
><br>
> -  Â  Â  Â  if (getpid() != 1) {<br>
> +  Â  Â  Â  /*if (getpid() != 1) {<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  log_error("Not executed by init (PID 1).");<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  r = -EPERM;<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  goto error;<br>
> -  Â  Â  Â  }<br>
> +  Â  Â  Â  }*/<br>
> +<br>
> +  Â  Â  Â  in_container = detect_container(NULL) > 0;<br>
><br>
>  Â  Â  Â  Â  if (streq(arg_verb, "reboot"))<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  cmd = RB_AUTOBOOT;<br>
> @@ -187,9 +271,19 @@ int main(int argc, char *argv[]) {<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  cmd = RB_POWER_OFF;<br>
>  Â  Â  Â  Â  else if (streq(arg_verb, "halt"))<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  cmd = RB_HALT_SYSTEM;<br>
> -  Â  Â  Â  else if (streq(arg_verb, "kexec"))<br>
> -  Â  Â  Â  Â  Â  Â  Â  cmd = LINUX_REBOOT_CMD_KEXEC;<br>
> -  Â  Â  Â  else {<br>
> +  Â  Â  Â  else if (streq(arg_verb, "kexec")) {<br>
> +  Â  Â  Â  Â  Â  Â  Â  if (in_container) {<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  log_warning("Can't kexec from container. Rebooting…");<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  cmd = RB_AUTOBOOT;<br>
> +  Â  Â  Â  Â  Â  Â  Â  } else {<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  r = kernel_load(false);<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  if (r < 0) {<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  log_warning("Failed to load kernel: %s. Rebooting…", strerror(-r));<br>
<br>
I think this should be changed too. I'd very much prefer to get an error if kexec fails.<br>
Maybe we could add a parameter like --always-reboot to reboot as a fallback if the<br>
kenrel cannot be loaded.<br>
<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  cmd = RB_AUTOBOOT;<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  } else<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  cmd = LINUX_REBOOT_CMD_KEXEC;<br>
> +  Â  Â  Â  Â  Â  Â  Â  }<br>
> +  Â  Â  Â  } else {<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  r = -EINVAL;<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  log_error("Unknown action '%s'.", arg_verb);<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  goto error;<br>
> @@ -208,8 +302,6 @@ int main(int argc, char *argv[]) {<br>
>  Â  Â  Â  Â  log_info("Sending SIGKILL to remaining processes...");<br>
>  Â  Â  Â  Â  broadcast_signal(SIGKILL, true, false);<br>
><br>
> -  Â  Â  Â  in_container = detect_container(NULL) > 0;<br>
> -<br>
>  Â  Â  Â  Â  need_umount = !in_container;<br>
>  Â  Â  Â  Â  need_swapoff = !in_container;<br>
>  Â  Â  Â  Â  need_loop_detach = !in_container;<br>
> @@ -349,11 +441,14 @@ int main(int argc, char *argv[]) {<br>
><br>
>  Â  Â  Â  Â  case LINUX_REBOOT_CMD_KEXEC:<br>
><br>
> -  Â  Â  Â  Â  Â  Â  Â  if (!in_container) {<br>
<span class="">> -  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  /* We cheat and exec kexec to avoid doing all its work */<br>
> -  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  pid_t pid;<br>
</span>> +  Â  Â  Â  Â  Â  Â  Â  log_info("Rebooting with kexec.");<br>
><br>
> -  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  log_info("Rebooting with kexec.");<br>
<span class="">> +  Â  Â  Â  Â  Â  Â  Â  /* kexec-tools has a bunch of special code to make Xen Dom0 work,<br>
</span>> +  Â  Â  Â  Â  Â  Â  Â  Â * otherwise it is only doing stuff we have already done.<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â * This is true for Dom0 and DomU but we only get Dom0<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â * because of the !in_container check */<br>
<span class="">> +  Â  Â  Â  Â  Â  Â  Â  if (access("/proc/xen", F_OK) == 0) {<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  pid_t pid;<br>
><br>
</span>>  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  pid = fork();<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  if (pid < 0)<br>
> @@ -370,7 +465,9 @@ int main(int argc, char *argv[]) {<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  _exit(EXIT_FAILURE);<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  } else<br>
<span class="">>  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  wait_for_terminate_and_warn("kexec", pid, true);<br>
> -  Â  Â  Â  Â  Â  Â  Â  }<br>
</span>> +<br>
> +  Â  Â  Â  Â  Â  Â  Â  } else<br>
<span class="">> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  reboot(cmd);<br>
><br>
>  Â  Â  Â  Â  Â  Â  Â  Â  cmd = RB_AUTOBOOT;<br>
</span>>  Â  Â  Â  Â  Â  Â  Â  Â  /* Fall through */<br>
> diff --git a/src/shared/missing.h b/src/shared/missing.h<br>
> index b33a70c..efd650a 100644<br>
> --- a/src/shared/missing.h<br>
> +++ b/src/shared/missing.h<br>
> @@ -763,3 +763,10 @@ static inline int kcmp(pid_t pid1, pid_t pid2, int type, unsigned long idx1, uns<br>
>  #ifndef KCMP_FILE<br>
>  #define KCMP_FILE 0<br>
>  #endif<br>
> +<br>
> +/* v3.17 */<br>
> +#ifndef __NR_kexec_file_load<br>
> +#ifdef __x86_64__<br>
> +#define __NR_kexec_file_load 320<br>
> +#endif<br>
> +#endif<br>
> --<br>
<br>
Zbyszek<br>
<div class="HOEnZb"><div class="h5">_______________________________________________<br>
systemd-devel mailing list<br>
<a href="mailto:systemd-devel@lists.freedesktop.org">systemd-devel@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/systemd-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/systemd-devel</a><br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature">Shawn Landden</div>
</div>