Re: [PATCH] grub-probefs --root-device

[Vesa Jääskeläinen]

Hi,

Here are some comments about your patch. As requested :)

Robert Millan wrote:
> 2006-09-23  Robert Millan  <address@hidden>
> 
>       * util/i386/pc/getroot.c (grub_guess_root_device): Stop using
>       grub_util_biosdisk_get_grub_dev to convert system device to GRUB device.
>       * util/grub-emu.c: Use grub_util_biosdisk_get_grub_dev with the string
>       returned by grub_guess_root_device.

Method name missing from grub-emu.

>       * util/i386/pc/grub-setup.c: Ditto.
>       * util/i386/pc/grub-probefs.c: Ditto.
> 
>       * util/i386/pc/grub-probefs.c: Rename to ...
>       * util/i386/pc/grub-probe.c: ... this.
>       * DISTLIST: Update reference to grub-probefs.
>       * conf/i386-efi.mk: Ditto.
>       * conf/i386-efi.rmk: Ditto.
>       * conf/i386-pc.mk: Ditto.
>       * conf/i386-pc.rmk: Ditto.
>       * util/i386/pc/grub-install.in: Ditto.

Describe what changed in terms of source code not in terms of "english".
 Eg. DISTLIST: Removed grub-probefs.  Added grub-probe.

I am not sure is "ditto" a good term.. I have seen "Likewise." in other
entries and used that myself too.

And no need to inform about changes to .mk as they are generated files.
Just make sure on commit they are there.

>       * util/i386/pc/grub-probe.c: Add --target=(fs|device|drive) option to
>       choose which information we want to print.

Method...

> Index: DISTLIST
> ===================================================================
> RCS file: /sources/grub/grub2/DISTLIST,v
> retrieving revision 1.36
> diff -u -r1.36 DISTLIST
> --- DISTLIST  4 Jun 2006 12:59:19 -0000       1.36
> +++ DISTLIST  23 Sep 2006 07:16:31 -0000
> @@ -253,7 +253,7 @@
>  util/i386/pc/grub-install.in
>  util/i386/pc/grub-mkdevicemap.c
>  util/i386/pc/grub-mkimage.c
> -util/i386/pc/grub-probefs.c
> +util/i386/pc/grub-probe.c
>  util/i386/pc/grub-setup.c
>  util/i386/pc/misc.c
>  util/powerpc/ieee1275/grub-install.in

> Index: conf/i386-efi.rmk
> ===================================================================
> RCS file: /sources/grub/grub2/conf/i386-efi.rmk,v
> retrieving revision 1.9
> diff -u -r1.9 i386-efi.rmk
> --- conf/i386-efi.rmk 28 May 2006 23:01:43 -0000      1.9
> +++ conf/i386-efi.rmk 23 Sep 2006 07:16:31 -0000
> @@ -6,7 +6,7 @@
>  
>  # Utilities.
>  bin_UTILITIES = grub-mkimage
> -#sbin_UTILITIES = grub-setup grub-emu grub-mkdevicemap grub-probefs
> +#sbin_UTILITIES = grub-setup grub-emu grub-mkdevicemap grub-probe
>  
>  # For grub-mkimage.
>  grub_mkimage_SOURCES = util/i386/efi/grub-mkimage.c util/misc.c \
> @@ -23,8 +23,8 @@
>  # For grub-mkdevicemap.
>  #grub_mkdevicemap_SOURCES = util/i386/pc/grub-mkdevicemap.c util/misc.c
>  
> -# For grub-probefs.
> -#grub_probefs_SOURCES = util/i386/pc/grub-probefs.c  \
> +# For grub-probe.
> +#grub_probefs_SOURCES = util/i386/pc/grub-probe.c    \

Should be grub_probe_SOURCES.

>  #    util/i386/pc/biosdisk.c util/misc.c util/i386/pc/getroot.c      \
>  #    kern/device.c kern/disk.c kern/err.c kern/misc.c fs/fat.c       \
>  #    fs/ext2.c kern/parser.c kern/partition.c partmap/pc.c fs/ufs.c  \
> Index: conf/i386-pc.rmk
> ===================================================================
> RCS file: /sources/grub/grub2/conf/i386-pc.rmk,v
> retrieving revision 1.69
> diff -u -r1.69 i386-pc.rmk
> --- conf/i386-pc.rmk  22 Sep 2006 00:27:38 -0000      1.69
> +++ conf/i386-pc.rmk  23 Sep 2006 07:16:32 -0000
> @@ -51,7 +51,7 @@
>  
>  # Utilities.
>  bin_UTILITIES = grub-mkimage
> -sbin_UTILITIES = grub-setup grub-emu grub-mkdevicemap grub-probefs
> +sbin_UTILITIES = grub-setup grub-emu grub-mkdevicemap grub-probe
>  
>  # For grub-mkimage.
>  grub_mkimage_SOURCES = util/i386/pc/grub-mkimage.c util/misc.c \
> @@ -69,8 +69,8 @@
>  # For grub-mkdevicemap.
>  grub_mkdevicemap_SOURCES = util/i386/pc/grub-mkdevicemap.c util/misc.c
>  
> -# For grub-probefs.
> -grub_probefs_SOURCES = util/i386/pc/grub-probefs.c   \
> +# For grub-probe.
> +grub_probefs_SOURCES = util/i386/pc/grub-probe.c     \

Same here.

>       util/i386/pc/biosdisk.c util/misc.c util/i386/pc/getroot.c      \
>       kern/device.c kern/disk.c kern/err.c kern/misc.c fs/fat.c       \
>       fs/ext2.c kern/parser.c kern/partition.c partmap/pc.c fs/ufs.c  \
> Index: util/grub-emu.c
> ===================================================================
> RCS file: /sources/grub/grub2/util/grub-emu.c,v
> retrieving revision 1.30
> diff -u -r1.30 grub-emu.c
> --- util/grub-emu.c   13 Jun 2006 22:50:01 -0000      1.30
> +++ util/grub-emu.c   23 Sep 2006 07:16:32 -0000
> @@ -185,7 +185,7 @@
>    /* Make sure that there is a root device.  */
>    if (! args.root_dev)
>      {
> -      args.root_dev = grub_guess_root_device (args.dir ? : 
> DEFAULT_DIRECTORY);
> +      args.root_dev = grub_util_biosdisk_get_grub_dev 
> (grub_guess_root_device (args.dir ? : DEFAULT_DIRECTORY));
>        if (! args.root_dev)
>       {
>         grub_util_info ("guessing the root device failed, because of `%s'",
> Index: util/i386/pc/getroot.c
> ===================================================================
> RCS file: /sources/grub/grub2/util/i386/pc/getroot.c,v
> retrieving revision 1.5
> diff -u -r1.5 getroot.c
> --- util/i386/pc/getroot.c    14 Sep 2006 18:52:50 -0000      1.5
> +++ util/i386/pc/getroot.c    23 Sep 2006 07:16:32 -0000
> @@ -223,5 +223,5 @@
>    if (! os_dev)
>      return 0;
>  
> -  return grub_util_biosdisk_get_grub_dev (os_dev);
> +  return os_dev;
>  }
> Index: util/i386/pc/grub-install.in
> ===================================================================
> RCS file: /sources/grub/grub2/util/i386/pc/grub-install.in,v
> retrieving revision 1.9
> diff -u -r1.9 grub-install.in
> --- util/i386/pc/grub-install.in      14 Sep 2006 18:52:50 -0000      1.9
> +++ util/i386/pc/grub-install.in      23 Sep 2006 07:16:32 -0000
> @@ -33,7 +33,7 @@
>  grub_setup=${sbindir}/grub-setup
>  grub_mkimage=${bindir}/grub-mkimage
>  grub_mkdevicemap=${sbindir}/grub-mkdevicemap
> -grub_probefs=${sbindir}/grub-probefs
> +grub_probefs=${sbindir}/grub-probe

Should be grub_probe as name changes... May need changes to other places
too.

>  rootdir=
>  grub_prefix=/boot/grub
>  modules=
> @@ -59,7 +59,7 @@
>    --grub-setup=FILE       use FILE as grub-setup
>    --grub-mkimage=FILE     use FILE as grub-mkimage
>    --grub-mkdevicemap=FILE use FILE as grub-mkdevicemap
> -  --grub-probefs=FILE     use FILE as grub-probefs
> +  --grub-probe=FILE     use FILE as grub-probe

At least here we are missing couple of spaces.

>    --no-floppy             do not probe any floppy drive
>    --recheck               probe a device map even if it already exists
>  
> @@ -92,8 +92,8 @@
>       grub_mkimage=`echo "$option" | sed 's/--grub-mkimage=//'` ;;
>      --grub-mkdevicemap=*)
>       grub_mkdevicemap=`echo "$option" | sed 's/--grub-mkdevicemap=//'` ;;
> -    --grub-probefs=*)
> -     grub_probefs=`echo "$option" | sed 's/--grub-probefs=//'` ;;
> +    --grub-probe=*)
> +     grub_probefs=`echo "$option" | sed 's/--grub-probe=//'` ;;

Needs change here at least...

>      --no-floppy)
>       no_floppy="--no-floppy" ;;
>      --recheck)
> Index: util/i386/pc/grub-probefs.c
> ===================================================================
> RCS file: /sources/grub/grub2/util/i386/pc/grub-probefs.c,v
> retrieving revision 1.3
> diff -u -r1.3 grub-probefs.c
> --- util/i386/pc/grub-probefs.c       23 Apr 2006 13:37:36 -0000      1.3
> +++ util/i386/pc/grub-probefs.c       23 Sep 2006 07:16:32 -0000
> @@ -1,4 +1,4 @@
> -/* grub-probefs.c - probe a filesystem module for a given path */
> +/* grub-probe.c - probe device information for a given path */
>  /*
>   *  GRUB  --  GRand Unified Bootloader
>   *  Copyright (C) 2005,2006 Free Software Foundation, Inc.
> @@ -47,6 +47,12 @@
>  
>  #define DEFAULT_DEVICE_MAP   DEFAULT_DIRECTORY "/device.map"
>  
> +#define PRINT_FS     0
> +#define PRINT_DRIVE  1
> +#define PRINT_DEVICE 2
> +
> +int print = PRINT_FS;
> +
>  void
>  grub_putchar (int c)
>  {
> @@ -74,13 +80,33 @@
>  probe (const char *path)
>  {
>    char *device_name;
> +  char *drive_name;
>    grub_device_t dev;
>    grub_fs_t fs;
>    
>    device_name = grub_guess_root_device (path);
>    if (! device_name)
>      {
> -      fprintf (stderr, "cannot find a GRUB device for %s.\n", path);
> +      fprintf (stderr, "cannot find a device for %s.\n", path);
> +      return;
> +    }
> +
> +  if (print == PRINT_DEVICE)
> +    {
> +      printf ("%s\n", device_name);
> +      return;
> +    }
> +
> +  drive_name = grub_util_biosdisk_get_grub_dev (device_name);
> +  if (! drive_name)
> +    {
> +      fprintf (stderr, "cannot find a GRUB drive for %s.\n", drive_name);
> +      return;
> +    }
> +
> +  if (print == PRINT_DRIVE)
> +    {
> +      printf ("(%s)\n", drive_name);
>        return;
>      }
>  
> @@ -102,6 +128,7 @@
>  static struct option options[] =
>    {
>      {"device-map", required_argument, 0, 'm'},
> +    {"target", required_argument, 0, 't'},
>      {"help", no_argument, 0, 'h'},
>      {"version", no_argument, 0, 'V'},
>      {"verbose", no_argument, 0, 'v'},
> @@ -113,14 +140,15 @@
>  {
>    if (status)
>      fprintf (stderr,
> -          "Try ``grub-probefs --help'' for more information.\n");
> +          "Try ``grub-probe --help'' for more information.\n");
>    else
>      printf ("\
> -Usage: grub-probefs [OPTION]... PATH\n\
> +Usage: grub-probe [OPTION]... PATH\n\
>  \n\
> -Probe a filesystem module for a given path.\n\
> +Probe device information for a given path.\n\
>  \n\
>    -m, --device-map=FILE     use FILE as the device map [default=%s]\n\
> +  -t, --target=(fs|drive|device)  print filesystem module, GRUB drive or 
> system device [default=fs]\n\
>    -h, --help                display this message and exit\n\
>    -V, --version             print version information and exit\n\
>    -v, --verbose             print verbose messages\n\
> @@ -138,12 +166,12 @@
>    char *dev_map = 0;
>    char *path;
>    
> -  progname = "grub-probefs";
> +  progname = "grub-probe";
>    
>    /* Check for options.  */
>    while (1)
>      {
> -      int c = getopt_long (argc, argv, "m:hVv", options, 0);
> +      int c = getopt_long (argc, argv, "m:t:hVv", options, 0);
>        
>        if (c == -1)
>       break;
> @@ -157,6 +185,17 @@
>           dev_map = xstrdup (optarg);
>           break;
>  
> +       case 't':
> +         if (!strcmp (optarg, "fs"))
> +           print = PRINT_FS;
> +         else if (!strcmp (optarg, "drive"))
> +           print = PRINT_DRIVE;
> +         else if (!strcmp (optarg, "device"))
> +           print = PRINT_DEVICE;
> +         else
> +           usage (1);
> +         break;
> +
>         case 'h':
>           usage (0);
>           break;
> Index: util/i386/pc/grub-setup.c
> ===================================================================
> RCS file: /sources/grub/grub2/util/i386/pc/grub-setup.c,v
> retrieving revision 1.18
> diff -u -r1.18 grub-setup.c
> --- util/i386/pc/grub-setup.c 4 Jun 2006 15:56:55 -0000       1.18
> +++ util/i386/pc/grub-setup.c 23 Sep 2006 07:16:32 -0000
> @@ -669,7 +669,7 @@
>      }
>    else
>      {
> -      root_dev = grub_guess_root_device (dir ? : DEFAULT_DIRECTORY);
> +      root_dev = grub_util_biosdisk_get_grub_dev (grub_guess_root_device 
> (dir ? : DEFAULT_DIRECTORY));
>        if (! root_dev)
>       {
>         grub_util_info ("guessing the root device failed, because of `%s'",