support for crashkernel= in 10_linux.in

support for crashkernel= in 10_linux.in

[Michael Vogt]

[-- Attachment #1: Type: text/plain, Size: 887 bytes --]

Hello,

in ubuntu we are working on making it easier to capture kernel
crashes. The system we use is based around kdump and it works by
triggering a kexec of a special crashdump kernel if the regular
kernels crashes. In order for this to work the normal kernel needs a
additional commandline option "crashkernel=".

Attached is a very simple patch that adds this option automatically if
kexec and makedumpfile are available.

If its not general enough for inclusion into the default grub2, may a
mechanism to pass additional kernel commandline paramters via some
sort of defaults.d/ directory would be the right apporoach? Then I
could just ship a config file in kexec-tools that looks like this:

GRUB_LINUX_CMDLINE_EXTRA="$GRUB_LINUX_CMDLINE_EXTRA crashkernel=384M-2G:64M,2G-:128M"


Thanks,
 Michael

P.S. I also put the patch in the bugtracker at
https://savannah.gnu.org/bugs/?26874

[-- Attachment #2: 10_crashkernel.patch --]
[-- Type: text/x-diff, Size: 919 bytes --]

diff -Nur -x '*.orig' -x '*~' grub2-1.96+20090611/util/grub.d/10_linux.in grub2-1.96+20090611.new/util/grub.d/10_linux.in
--- grub2-1.96+20090611/util/grub.d/10_linux.in	2009-06-24 10:17:45.202761012 +0200
+++ grub2-1.96+20090611.new/util/grub.d/10_linux.in	2009-06-24 10:19:11.198757708 +0200
@@ -43,6 +43,11 @@
   LINUX_ROOT_DEVICE=UUID=${GRUB_DEVICE_UUID}
 fi
 
+# add crashkernel option if we have the requierd tools
+if [ -x "/usr/bin/makedumpfile" ] && [ -x "/sbin/kexec" ]; then
+    GRUB_CMDLINE_EXTRA="$GRUB_CMDLINE_EXTRA crashkernel=384M-2G:64M,2G-:128M"
+fi
+
 test_numeric ()
 {
   local a=$1
@@ -143,7 +148,7 @@
   fi
 
   linux_entry "${OS}, Linux ${version}" \
-      "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}"
+      "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT} ${GRUB_CMDLINE_EXTRA}"
   linux_entry "${OS}, Linux ${version} (recovery mode)" \
       "single ${GRUB_CMDLINE_LINUX}"
 

Re: support for crashkernel= in 10_linux.in

[Robert Millan]

Hi Michael,

On Wed, Jun 24, 2009 at 01:20:49PM +0200, Michael Vogt wrote:
> diff -Nur -x '*.orig' -x '*~' grub2-1.96+20090611/util/grub.d/10_linux.in grub2-1.96+20090611.new/util/grub.d/10_linux.in
> --- grub2-1.96+20090611/util/grub.d/10_linux.in	2009-06-24 10:17:45.202761012 +0200
> +++ grub2-1.96+20090611.new/util/grub.d/10_linux.in	2009-06-24 10:19:11.198757708 +0200
> @@ -43,6 +43,11 @@
>    LINUX_ROOT_DEVICE=UUID=${GRUB_DEVICE_UUID}
>  fi
>  
> +# add crashkernel option if we have the requierd tools
> +if [ -x "/usr/bin/makedumpfile" ] && [ -x "/sbin/kexec" ]; then
> +    GRUB_CMDLINE_EXTRA="$GRUB_CMDLINE_EXTRA crashkernel=384M-2G:64M,2G-:128M"
> +fi
> +

So many variables start to look confusing.  Can't you append it to one of
the others?

Or give it a more explicit name, like ${GRUB_CMDLINE_LINUX_CRASHADDR} or so
(in any case GRUB_CMDLINE_EXTRA is too generic for a Linux-specific thing).

>    linux_entry "${OS}, Linux ${version}" \
> -      "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}"
> +      "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT} ${GRUB_CMDLINE_EXTRA}"
>    linux_entry "${OS}, Linux ${version} (recovery mode)" \
>        "single ${GRUB_CMDLINE_LINUX}"

Is this flag not desireable in recovery mode?

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: support for crashkernel= in 10_linux.in

[Michael Vogt]

[-- Attachment #1: Type: text/plain, Size: 1717 bytes --]

On Wed, Jun 24, 2009 at 02:11:59PM +0200, Robert Millan wrote:
> Hi Michael,
Hi Robert,

Thanks for your quick reply.
 
> On Wed, Jun 24, 2009 at 01:20:49PM +0200, Michael Vogt wrote:
> > diff -Nur -x '*.orig' -x '*~' grub2-1.96+20090611/util/grub.d/10_linux.in grub2-1.96+20090611.new/util/grub.d/10_linux.in
> > --- grub2-1.96+20090611/util/grub.d/10_linux.in	2009-06-24 10:17:45.202761012 +0200
> > +++ grub2-1.96+20090611.new/util/grub.d/10_linux.in	2009-06-24 10:19:11.198757708 +0200
> > @@ -43,6 +43,11 @@
> >    LINUX_ROOT_DEVICE=UUID=${GRUB_DEVICE_UUID}
> >  fi
> >  
> > +# add crashkernel option if we have the requierd tools
> > +if [ -x "/usr/bin/makedumpfile" ] && [ -x "/sbin/kexec" ]; then
> > +    GRUB_CMDLINE_EXTRA="$GRUB_CMDLINE_EXTRA crashkernel=384M-2G:64M,2G-:128M"
> > +fi
> > +
> 
> So many variables start to look confusing.  Can't you append it to one of
> the others?

Sure, attached is a updated version that does that.

> Or give it a more explicit name, like ${GRUB_CMDLINE_LINUX_CRASHADDR} or so
> (in any case GRUB_CMDLINE_EXTRA is too generic for a Linux-specific thing).
> 
> >    linux_entry "${OS}, Linux ${version}" \
> > -      "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}"
> > +      "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT} ${GRUB_CMDLINE_EXTRA}"
> >    linux_entry "${OS}, Linux ${version} (recovery mode)" \
> >        "single ${GRUB_CMDLINE_LINUX}"
> 
> Is this flag not desireable in recovery mode?

Maybe - my rational was to keep recovery mode as simple as
possible. And also in case there is a issue with the crashkernel=
commandlinethe user is still able to use recovery mode (its pretty
unlikely that there are problems though).

Cheers,
 Michael

[-- Attachment #2: 10_crashkernel2.patch --]
[-- Type: text/x-diff, Size: 595 bytes --]

=== modified file 'util/grub.d/10_linux.in'
--- util/grub.d/10_linux.in	2009-06-24 13:00:17 +0000
+++ util/grub.d/10_linux.in	2009-06-24 13:09:26 +0000
@@ -35,6 +35,11 @@
   ;;
 esac
 
+# add crashkernel option if we have the requierd tools
+if [ -x "/usr/bin/makedumpfile" ] && [ -x "/sbin/kexec" ]; then
+    GRUB_CMDLINE_LINUX_DEFAULT="$GRUB_CMDLINE_LINUX_DEFAULT crashkernel=384M-2G:64M,2G-:128M"
+fi
+
 if [ "x${GRUB_DEVICE_UUID}" = "x" ] || [ "x${GRUB_DISABLE_LINUX_UUID}" = "xtrue" ] \
     || ! test -e "/dev/disk/by-uuid/${GRUB_DEVICE_UUID}" ; then
   LINUX_ROOT_DEVICE=${GRUB_DEVICE}

Re: support for crashkernel= in 10_linux.in

[Pavel Roskin]

On Wed, 2009-06-24 at 15:11 +0200, Michael Vogt wrote:

> Maybe - my rational was to keep recovery mode as simple as
> possible. And also in case there is a issue with the crashkernel=
> commandlinethe user is still able to use recovery mode (its pretty
> unlikely that there are problems though).

I think it a patch that could stay in the Ubuntu package for the time
being.  It makes assumptions about the kernel that are not correct in
the general case.

You may want to explore the possibility or creating a file called e.g.
05_ubuntu or 05_crashkernel that would adjust GRUB_CMDLINE_LINUX_DEFAULT
and do other Ubuntu-specific processing.

-- 
Regards,
Pavel Roskin

Re: support for crashkernel= in 10_linux.in

[Robert Millan]

On Wed, Jun 24, 2009 at 03:18:08PM -0400, Pavel Roskin wrote:
> On Wed, 2009-06-24 at 15:11 +0200, Michael Vogt wrote:
> 
> > Maybe - my rational was to keep recovery mode as simple as
> > possible. And also in case there is a issue with the crashkernel=
> > commandlinethe user is still able to use recovery mode (its pretty
> > unlikely that there are problems though).
> 
> I think it a patch that could stay in the Ubuntu package for the time
> being.  It makes assumptions about the kernel that are not correct in
> the general case.

Uhm what does "384M-2G:64M,2G-:128M" mean?  It looks like some sort of
address or memory range.  Perhaps there's some portable way to obtain this
information from the system?

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."