Bug#462218: Bug#461442: detection of other OSes in update-grub
Marco Gerards
mgerards at xs4all.nl
Mon Feb 4 15:27:50 UTC 2008
Fabian Greffrath <greffrath at leat.rub.de> writes:
Hi!
> Robert Millan schrieb:
>> If you want the former, feel free to send a patch to grub-devel. For the
>> latter, look at my questions and send some feedback.
>
> please find attached a patch that adds a new parameter '--device, -d'
> to grub-probe. If this parameter is set, grub-probe expects the given
> argument to be a block device. All of the '--target' parameters work
> with this option as well. If the '--device' parameter is not set,
> grub-probe will work as before.
> Furthermore I had to add a new public function to getroot.c that
> returns the given argument if it is a block device and returns NULL
> else. This was necessary, because else you could force grub-probe to
> print 'foobar' if run as 'grub-probe --target=device --device foobar'.
Thanks for this patch, however you didn't really describe the problem
you are solving. I am not sure if this patch replaces your patch.
This thread was initiated with a follow up to a mail Robert sent
off-list. So can you please describe what you are doing and why,
otherwise this is simply a piece of code to me that I have to review
;-)
So please explain what this is, I cannot accept patches if I do not
know why I should accept them.
> I consider this (i.e. adding a new option to grub-probe) a better
> solution than writing a separate tool only for this purpose.
>
> The second file that I attached is the 30_os-prober script making use
> of the new grub-probe feature. Please note that the 'case ... in' part
> is only run if 'grub-probe --device' actually returns something
> usefull.
>
> I am looking forward to your feedback.
Here it is ;-)
First of all, a ChangeLog entry is missing.
Is this code only yours or did you use code from other places?
> diff -urNad grub2-1.95+20080201~/include/grub/util/getroot.h grub2-1.95+20080201/include/grub/util/getroot.h
> --- grub2-1.95+20080201~/include/grub/util/getroot.h 2008-01-12 16:11:56.000000000 +0100
> +++ grub2-1.95+20080201/include/grub/util/getroot.h 2008-02-03 18:14:11.000000000 +0100
> @@ -29,5 +29,6 @@
> char *grub_get_prefix (const char *dir);
> int grub_util_get_dev_abstraction (const char *os_dev);
> char *grub_util_get_grub_dev (const char *os_dev);
> +char *grub_util_check_block_device (const char *blk_dev);
>
> #endif /* ! GRUB_UTIL_GETROOT_HEADER */
> diff -urNad grub2-1.95+20080201~/util/getroot.c grub2-1.95+20080201/util/getroot.c
> --- grub2-1.95+20080201~/util/getroot.c 2008-01-12 16:11:56.000000000 +0100
> +++ grub2-1.95+20080201/util/getroot.c 2008-02-03 21:56:29.000000000 +0100
> @@ -327,3 +327,17 @@
>
> return grub_dev;
> }
> +
> +char *
> +grub_util_check_block_device (const char *blk_dev)
> +{
> + struct stat st;
> +
> + if (stat (blk_dev, &st) < 0)
> + grub_util_error ("Cannot stat `%s'", blk_dev);
> +
> + if (S_ISBLK (st.st_mode))
> + return strdup(blk_dev);
strdup (blk_dev);
> + else
> + return 0;
> +}
> diff -urNad grub2-1.95+20080201~/util/grub-probe.c grub2-1.95+20080201/util/grub-probe.c
> --- grub2-1.95+20080201~/util/grub-probe.c 2008-01-25 23:33:57.000000000 +0100
> +++ grub2-1.95+20080201/util/grub-probe.c 2008-02-03 19:30:50.000000000 +0100
> @@ -50,6 +50,7 @@
> };
>
> int print = PRINT_FS;
> +unsigned int argument_is_device = 0;
Shouldn't this be static?
> void
> grub_putchar (int c)
> @@ -84,9 +85,18 @@
> int abstraction_type;
> grub_device_t dev = NULL;
>
> - device_name = grub_guess_root_device (path);
> + if (argument_is_device)
> + device_name = grub_util_check_block_device (path);
> + else
> + device_name = grub_guess_root_device (path);
> +
> if (! device_name)
> - grub_util_error ("cannot find a device for %s.\n", path);
> + {
> + if (argument_is_device)
> + grub_util_error ("%s is not a block device.\n", path);
> + else
> + grub_util_error ("cannot find a device for %s.\n", path);
> + }
>
> if (print == PRINT_DEVICE)
> {
> @@ -201,6 +211,7 @@
>
> static struct option options[] =
> {
> + {"device", no_argument, 0, 'd'},
> {"device-map", required_argument, 0, 'm'},
> {"target", required_argument, 0, 't'},
> {"help", no_argument, 0, 'h'},
> @@ -217,10 +228,11 @@
> "Try ``grub-probe --help'' for more information.\n");
> else
> printf ("\
> -Usage: grub-probe [OPTION]... PATH\n\
> +Usage: grub-probe [OPTION]... [PATH|DEVICE]\n\
> \n\
> -Probe device information for a given path.\n\
> +Probe device information for a given path or device.\n\
> \n\
> + -d, --device given argument is a system device, not a path\n\
> -m, --device-map=FILE use FILE as the device map [default=%s]\n\
> -t, --target=(fs|drive|device|partmap|abstraction)\n\
> print filesystem module, GRUB drive, system device, partition map module or abstraction module [default=fs]\n\
> @@ -246,13 +258,17 @@
> /* Check for options. */
> while (1)
> {
> - int c = getopt_long (argc, argv, "m:t:hVv", options, 0);
> + int c = getopt_long (argc, argv, "dm:t:hVv", options, 0);
>
> if (c == -1)
> break;
> else
> switch (c)
> {
> + case 'd':
> + argument_is_device = 1;
> + break;
> +
> case 'm':
> if (dev_map)
> free (dev_map);
> #! /bin/sh -e
Huh, you are mixing C code with a shellscript?
What does this code below do and why does it appear to be in this .c
file?
> # update-grub helper script.
> # <insert copyright and license blurb here>
>
> if [ -x "`which os-prober 2>/dev/null`" ] ; then
> OSPROBED="`os-prober | tr ' ' '|' | paste -s -d ' '`"
> fi
>
> if [ -n "${OSPROBED}" ] ; then
> for OS in ${OSPROBED} ; do
> DEVICE="`echo ${OS} | cut -d ':' -f 1`"
> LONGNAME="`echo ${OS} | cut -d ':' -f 2 | tr '|' ' '`"
> LABEL="`echo ${OS} | cut -d ':' -f 3 | tr '|' ' '`"
> BOOT="`echo ${OS} | cut -d ':' -f 4`"
>
> if [ -z "${LONGNAME}" ] ; then
> LONGNAME="${LABEL}"
> fi
>
> echo "Found ${LONGNAME} on ${DEVICE}" >&2
>
> TESTDEVICE="`grub-probe --target=device $0`"
> if [ -n "`grub-probe --target=drive --device ${TESTDEVICE} 2>/dev/null`" ] ; then
> case "${BOOT}" in
> chain)
> CHAINROOT="`grub-probe --target=drive --device ${DEVICE}`"
>
> cat << EOF
> menuentry "${LONGNAME} (on ${DEVICE})" {
> set root=${CHAINROOT}
> chainloader +1
> }
> EOF
> ;;
> linux)
> if [ -x "`which linux-boot-prober 2>/dev/null`" ] ; then
> LINUXPROBED="`linux-boot-prober ${DEVICE} | tr ' ' '|' | paste -s -d ' '`"
> fi
>
> if [ -n "${LINUXPROBED}" ] ; then
> for LINUX in ${LINUXPROBED} ; do
> LROOT="`echo ${LINUX} | cut -d ':' -f 1`"
> LBOOT="`echo ${LINUX} | cut -d ':' -f 2`"
> LLABEL="`echo ${LINUX} | cut -d ':' -f 3 | tr '|' ' '`"
> LKERNEL="`echo ${LINUX} | cut -d ':' -f 4`"
> LINITRD="`echo ${LINUX} | cut -d ':' -f 5`"
> LPARAMS="`echo ${LINUX} | cut -d ':' -f 6 | tr '|' ' '`"
>
> LINUXROOT="`grub-probe --target=drive --device ${LBOOT}`"
>
> if [ -z "${LLABEL}" ] ; then
> LLABEL="${LONGNAME}"
> fi
>
> cat << EOF
> menuentry "${LLABEL} (on ${DEVICE})" {
> set root=${LINUXROOT}
> linux ${LKERNEL} ${LPARAMS}
> EOF
> if [ -n "${LINITRD}" ] ; then
> cat << EOF
> initrd ${LINITRD}
> EOF
> fi
> cat << EOF
> }
> EOF
> done
> fi
> ;;
> hurd)
> # not yet...
> ;;
> *)
> ;;
> esac
> fi
> done
> fi
More information about the Pkg-grub-devel
mailing list