[PATCH] Fix LVM/RAID probing without device.map

[PATCH] Fix LVM/RAID probing without device.map

[Colin Watson]

We're trying to get rid of our reliance on device.map, for all the
well-known reasons: it causes problems when OS device names change, etc.
For the most part, GRUB 1.98 works fine without it, because if you run
grub-probe or grub-setup in a way that requires them to look at e.g.
/dev/sda, then they'll generate a temporary mapping to a GRUB device
name so that internal functions work.

However, LVM and RAID don't work in this configuration.  This is because
they rely on disk/lvm.c and disk/raid.c being able to iterate over all
devices to scan for LVM and RAID physical volumes, which only works if
all the plausible devices on which those PVs might live have already
been probed.

The following patch arranges to probe all the underlying devices if
device.map doesn't exist.  In the case of grub-probe, we only do so if
the device name indicates that it's LVM or RAID, to avoid slowing down
update-grub unnecessarily when those device abstractions aren't
involved.

2010-03-22  Colin Watson  <cjwatson@ubuntu.com>

	* util/hostdisk.c (store_grub_dev): New function.
	(grub_util_biosdisk_probe_device): Likewise.
	* include/grub/util/hostdisk.h (grub_util_biosdisk_probe_device):
	New prototype.
	* util/grub-probe.c (probe): Accept new dev_map parameter.  If this
	cannot be statted and the target device is LVM or RAID, probe all
	devices and reinitialise the LVM and RAID modules.
	(main): Update calls to probe.
	* util/i386/pc/grub-setup.c (main): If the device map cannot be
	statted, probe all devices.
	* util/sparc64/ieee1275/grub-setup.c (main): Likewise.
	* conf/common.rmk (grub_probe_SOURCES): Add util/deviceiter.c.
	* conf/i386-pc.rmk (grub_setup_SOURCES): Likewise.
	* conf/sparc64-ieee1275.rmk (grub_setup_SOURCES): Likewise.

=== modified file 'conf/common.rmk'
--- conf/common.rmk	2010-03-14 16:50:55 +0000
+++ conf/common.rmk	2010-03-22 14:21:18 +0000
@@ -26,6 +26,7 @@ sbin_UTILITIES += grub-probe
 util/grub-probe.c_DEPENDENCIES = grub_probe_init.h
 grub_probe_SOURCES = gnulib/progname.c util/grub-probe.c	\
 	util/hostdisk.c	util/misc.c util/getroot.c		\
+	util/deviceiter.c					\
 	kern/device.c kern/disk.c kern/err.c kern/misc.c	\
 	kern/parser.c kern/partition.c kern/file.c kern/list.c	\
 	\

=== modified file 'conf/i386-pc.rmk'
--- conf/i386-pc.rmk	2010-03-14 16:50:55 +0000
+++ conf/i386-pc.rmk	2010-03-22 14:21:18 +0000
@@ -75,7 +75,8 @@ util/grub-mkrawimage.c_DEPENDENCIES = Ma
 util/i386/pc/grub-setup.c_DEPENDENCIES = grub_setup_init.h
 grub_setup_SOURCES = gnulib/progname.c \
 	util/i386/pc/grub-setup.c util/hostdisk.c	\
-	util/misc.c util/getroot.c kern/device.c kern/disk.c	\
+	util/misc.c util/getroot.c util/deviceiter.c		\
+	kern/device.c kern/disk.c				\
 	kern/err.c kern/misc.c kern/parser.c kern/partition.c	\
 	kern/file.c kern/fs.c kern/env.c kern/list.c		\
 	fs/fshelp.c						\

=== modified file 'conf/sparc64-ieee1275.rmk'
--- conf/sparc64-ieee1275.rmk	2010-03-14 16:50:55 +0000
+++ conf/sparc64-ieee1275.rmk	2010-03-22 14:21:18 +0000
@@ -50,7 +50,8 @@ grub_mkimage_SOURCES = util/grub-mkrawim
 # For grub-setup.
 util/sparc64/ieee1275/grub-setup.c_DEPENDENCIES = grub_setup_init.h
 grub_setup_SOURCES = util/sparc64/ieee1275/grub-setup.c util/hostdisk.c	\
-	util/misc.c util/getroot.c kern/device.c kern/disk.c	\
+	util/misc.c util/getroot.c util/deviceiter.c		\
+	kern/device.c kern/disk.c				\
 	kern/err.c kern/misc.c kern/parser.c kern/partition.c	\
 	kern/file.c kern/fs.c kern/env.c kern/list.c		\
 	fs/fshelp.c						\

=== modified file 'include/grub/util/hostdisk.h'
--- include/grub/util/hostdisk.h	2008-09-08 13:52:30 +0000
+++ include/grub/util/hostdisk.h	2010-03-22 14:21:18 +0000
@@ -22,6 +22,7 @@
 
 void grub_util_biosdisk_init (const char *dev_map);
 void grub_util_biosdisk_fini (void);
+int grub_util_biosdisk_probe_device (const char *name, int is_floppy);
 char *grub_util_biosdisk_get_grub_dev (const char *os_dev);
 
 #endif /* ! GRUB_BIOSDISK_MACHINE_UTIL_HEADER */

=== modified file 'util/grub-probe.c'
--- util/grub-probe.c	2010-01-20 02:11:07 +0000
+++ util/grub-probe.c	2010-03-22 14:21:18 +0000
@@ -28,6 +28,7 @@
 #include <grub/msdos_partition.h>
 #include <grub/util/hostdisk.h>
 #include <grub/util/getroot.h>
+#include <grub/util/deviceiter.h>
 #include <grub/term.h>
 #include <grub/env.h>
 #include <grub/raid.h>
@@ -106,13 +107,14 @@ probe_raid_level (grub_disk_t disk)
 }
 
 static void
-probe (const char *path, char *device_name)
+probe (const char *path, char *device_name, const char *dev_map)
 {
   char *drive_name = NULL;
   char *grub_path = NULL;
   char *filebuf_via_grub = NULL, *filebuf_via_sys = NULL;
   grub_device_t dev = NULL;
   grub_fs_t fs;
+  struct stat dev_map_stat;
 
   if (path == NULL)
     {
@@ -136,6 +138,22 @@ probe (const char *path, char *device_na
       goto end;
     }
 
+  if (stat (dev_map, &dev_map_stat) == -1 &&
+      grub_util_get_dev_abstraction (device_name) != GRUB_DEV_ABSTRACTION_NONE)
+    {
+      /* If we don't have a device map, then we won't yet know about the
+         physical volumes underlying this device, so probe all devices.  */
+      grub_util_iterate_devices (grub_util_biosdisk_probe_device, 0);
+
+      /* Now reinitialise the higher layers.  */
+      grub_lvm_fini ();
+      grub_mdraid_fini ();
+      grub_raid_fini ();
+      grub_raid_init ();
+      grub_mdraid_init ();
+      grub_lvm_init ();
+    }
+
   drive_name = grub_util_get_grub_dev (device_name);
   if (! drive_name)
     grub_util_error ("cannot find a GRUB drive for %s.  Check your device.map", device_name);
@@ -428,9 +446,9 @@ main (int argc, char *argv[])
 
   /* Do it.  */
   if (argument_is_device)
-    probe (NULL, argument);
+    probe (NULL, argument, dev_map ? : DEFAULT_DEVICE_MAP);
   else
-    probe (argument, NULL);
+    probe (argument, NULL, dev_map ? : DEFAULT_DEVICE_MAP);
 
   /* Free resources.  */
   grub_fini_all ();

=== modified file 'util/hostdisk.c'
--- util/hostdisk.c	2010-03-14 15:39:14 +0000
+++ util/hostdisk.c	2010-03-22 14:21:18 +0000
@@ -1024,6 +1024,48 @@ find_system_device (const char *os_dev)
   return i;
 }
 
+static void
+store_grub_dev (const char *grub_disk, const char *os_disk)
+{
+  unsigned int i;
+
+  for (i = 0; i < ARRAY_SIZE (map); i++)
+    if (! map[i].device)
+      break;
+    else if (strcmp (map[i].drive, grub_disk) == 0)
+      {
+	if (strcmp (map[i].device, os_disk) == 0)
+	  return;
+	grub_util_error (_("drive `%s' already mapped to `%s'"),
+			 map[i].drive, map[i].device);
+      }
+
+  if (i == ARRAY_SIZE (map))
+    grub_util_error (_("device count exceeds limit"));
+
+  map[i].drive = xstrdup (grub_disk);
+  map[i].device = xstrdup (os_disk);
+}
+
+static int num_hd = 0;
+static int num_fd = 0;
+
+int
+grub_util_biosdisk_probe_device (const char *name, int is_floppy)
+{
+  char *grub_disk;
+
+  if (is_floppy)
+    grub_disk = xasprintf ("fd%d", num_fd++);
+  else
+    grub_disk = xasprintf ("hd%d", num_hd++);
+
+  store_grub_dev (grub_disk, name);
+  free (grub_disk);
+
+  return 0;
+}
+
 char *
 grub_util_biosdisk_get_grub_dev (const char *os_dev)
 {

=== modified file 'util/i386/pc/grub-setup.c'
--- util/i386/pc/grub-setup.c	2010-03-08 22:20:02 +0000
+++ util/i386/pc/grub-setup.c	2010-03-22 14:21:18 +0000
@@ -36,6 +36,7 @@
 #include <grub/util/raid.h>
 #include <grub/util/lvm.h>
 #include <grub/util/getroot.h>
+#include <grub/util/deviceiter.h>
 
 static const grub_gpt_part_type_t grub_gpt_partition_type_bios_boot = GRUB_GPT_PARTITION_TYPE_BIOS_BOOT;
 
@@ -631,6 +632,7 @@ main (int argc, char *argv[])
   char *core_file = 0;
   char *dir = 0;
   char *dev_map = 0;
+  struct stat dev_map_stat;
   char *root_dev = 0;
   char *dest_dev;
   int must_embed = 0, force = 0, fs_probe = 1;
@@ -729,6 +731,9 @@ main (int argc, char *argv[])
   /* Initialize the emulated biosdisk driver.  */
   grub_util_biosdisk_init (dev_map ? : DEFAULT_DEVICE_MAP);
 
+  if (stat (dev_map ? : DEFAULT_DEVICE_MAP, &dev_map_stat) == -1)
+    grub_util_iterate_devices (grub_util_biosdisk_probe_device, 0);
+
   /* Initialize all modules. */
   grub_init_all ();
 

=== modified file 'util/sparc64/ieee1275/grub-setup.c'
--- util/sparc64/ieee1275/grub-setup.c	2010-01-16 00:26:52 +0000
+++ util/sparc64/ieee1275/grub-setup.c	2010-03-22 14:21:18 +0000
@@ -46,6 +46,7 @@
 #include <sys/stat.h>
 #include <dirent.h>
 #include <grub/util/getroot.h>
+#include <grub/util/deviceiter.h>
 
 #define _GNU_SOURCE	1
 #include <getopt.h>
@@ -618,6 +619,7 @@ int
 main (int argc, char *argv[])
 {
   struct grub_setup_info ginfo;
+  struct stat dev_map_stat;
 
   set_program_name (argv[0]);
 
@@ -630,6 +632,9 @@ main (int argc, char *argv[])
   /* Initialize the emulated biosdisk driver.  */
   grub_util_biosdisk_init (ginfo.dev_map ? ginfo.dev_map : DEFAULT_DEVICE_MAP);
 
+  if (stat (ginfo.dev_map ? : DEFAULT_DEVICE_MAP, &dev_map_stat) == -1)
+    grub_util_iterate_devices (grub_util_biosdisk_probe_device, 0);
+
   /* Initialize all modules. */
   grub_init_all ();
 

Comments?

-- 
Colin Watson                                       [cjwatson@ubuntu.com]

Re: [PATCH] Fix LVM/RAID probing without device.map

[Vladimir 'φ-coder/phcoder' Serbinenko]

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


util/deviceiter.c is missing from your patch.
> @@ -136,6 +138,22 @@ probe (const char *path, char *device_na
>        goto end;
>      }
>  
> +  if (stat (dev_map, &dev_map_stat) == -1 &&
> +      grub_util_get_dev_abstraction (device_name) != GRUB_DEV_ABSTRACTION_NONE)
>   
Looks like it may call stat with NULL argument. I would rather avoid
doing this than rely on stat to detect it cleanly and not just segfault
> +    {
> +      /* If we don't have a device map, then we won't yet know about the
> +         physical volumes underlying this device, so probe all devices.  */
> +      grub_util_iterate_devices (grub_util_biosdisk_probe_device, 0);
> +
> +      /* Now reinitialise the higher layers.  */
> +      grub_lvm_fini ();
> +      grub_mdraid_fini ();
> +      grub_raid_fini ();
> +      grub_raid_init ();
> +      grub_mdraid_init ();
> +      grub_lvm_init ();
>   
Can we not to initialise those levels before they should be inited
rather than reinit? If it requires unclean workarounds I would rather
prefer this since it's relatively clean (approx rmmod+insmod)
> +int
> +grub_util_biosdisk_probe_device (const char *name, int is_floppy)
> +{
> +  char *grub_disk;
> +
> +  if (is_floppy)
> +    grub_disk = xasprintf ("fd%d", num_fd++);
> +  else
> +    grub_disk = xasprintf ("hd%d", num_hd++);
> +
>   
You can also choose something more straightforward for name generation.
Like just use 'name'.




-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 293 bytes --]

Re: [PATCH] Fix LVM/RAID probing without device.map

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> util/deviceiter.c is missing from your patch.
>   
Sorry, I had an impression you were adding this file. Ignore this

-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 293 bytes --]