register dummy drive

There's no reason grub-probe should fail if it can't resolve drive, when we just asked for -t fs, -t fs_uuid or -t partmap.

This patch solves the problem by spliting device/drive map[] entry registration into a separate function, and using that from grub-probe.c to register a dummy drive that will last during the current execution.

Quoted 6 lines · Robert Millan

There's no reason grub-probe should fail if it can't resolve drive, when we just asked for -t fs, -t fs_uuid or -t partmap.

This patch solves the problem by spliting device/drive map[] entry registration into a separate function, and using that from grub-probe.c to register a dummy drive that will last during the current execution.

Cannot hostfs be used by default for all userspace utilities?

Quoted 8 lines · Pavel Roskin

Quoted 6 lines · Robert Millan

There's no reason grub-probe should fail if it can't resolve drive, when we just asked for -t fs, -t fs_uuid or -t partmap.

This patch solves the problem by spliting device/drive map[] entry registration into a separate function, and using that from grub-probe.c to register a dummy drive that will last during the current execution.

Quoted 1 line

Cannot hostfs be used by default for all userspace utilities?

Part of what makes grub-probe interesting is that it shares a lot of code with the freestanding GRUB you will run later, so when it is used during grub-install & update-grub, it is very useful to catch possible problems. I think hostfs would defeat that purpose.

Quoted 5 lines

Quoted 4 lines

Quoted 4 lines

This patch solves the problem by spliting device/drive map[] entry registration into a separate function, and using that from grub-probe.c to register a dummy drive that will last during the current execution.

Quoted 4 lines

Part of what makes grub-probe interesting is that it shares a lot of code with the freestanding GRUB you will run later, so when it is used during grub-install & update-grub, it is very useful to catch possible problems. I think hostfs would defeat that purpose.

Well, then we probably don't want to ignore any errors. When do you have the situation that the drive cannot be resolved?

My concern is that introducing a dummy object for non-testing purposes could indicate bad code quality.

Quoted 5 lines · Pavel Roskin

Quoted 5 lines

Quoted 4 lines

Quoted 4 lines

This patch solves the problem by spliting device/drive map[] entry registration into a separate function, and using that from grub-probe.c to register a dummy drive that will last during the current execution.

Quoted 7 lines

Quoted 4 lines

Part of what makes grub-probe interesting is that it shares a lot of code with the freestanding GRUB you will run later, so when it is used during grub-install & update-grub, it is very useful to catch possible problems. I think hostfs would defeat that purpose.

Well, then we probably don't want to ignore any errors. When do you have the situation that the drive cannot be resolved?

For example, when the next version of Linux decides to rename all devices (it tends to do that often) and suddenly none of them match any device.map entry.

The thing is, for what we're trying to do (-t fs, -t fs_uuid, -t partmap) we don't care how will GRUB identify those devices when it's running on the BIOS, so just any string that uniquely identifies them will be enough so we can run our filesystem / partmap probes.

There's no reason grub-probe should fail if it can't resolve drive, when we just asked for -t fs, -t fs_uuid or -t partmap.

This patch solves the problem by spliting device/drive map[] entry registration into a separate function, and using that from grub-probe.c to register a dummy drive that will last during the current execution.

It's diffed relative to my previous patch for device function names (but if we need to change something in that one, not a problem for me to readapt it, of course).

Signature · 21 lines

Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What good is a phone call… if you are unable to speak?
(as seen on /.)

2008-06-02  Robert Millan  <[email protected]>

	* include/grub/util/biosdisk.h (grub_util_biosdisk_register): New
	function declaration.
	* util/biosdisk.c (open_device): Remove first grub_util_info() call
	(too verbose).
	(grub_util_biosdisk_register): New function.
	(read_device_map): Use grub_util_biosdisk_register() for registration
	of drive/device entries rather than doing it ourselves.
	* util/grub-probe.c (probe): If we weren't asked to -t drive, allow
	grub_util_get_grub_dev() to fail by registering a dummy drive entry,
	so that other options like -t fs, -t fs_uuid or -t partmap will work
	regardless.

diff -x configure -x config.h.in -x CVS -x '*~' -x '*.mk' -urp ../tmp.old/include/grub/util/biosdisk.h ./include/grub/util/biosdisk.h

include/grub/util/biosdisk.h renamed from tmp.old/include/grub/util/biosdisk.h +1 -0

@@ -23,5 +23,6 @@
 void grub_util_biosdisk_init (const char *dev_map);
 void grub_util_biosdisk_fini (void);
 char *grub_util_biosdisk_get_grub_dev (const char *os_dev);
+int grub_util_biosdisk_register (char *device, char *drive);
 
 #endif /* ! GRUB_BIOSDISK_MACHINE_UTIL_HEADER */

diff -x configure -x config.h.in -x CVS -x '*~' -x '*.mk' -urp ../tmp.old/util/biosdisk.c ./util/biosdisk.c

util/biosdisk.c renamed from tmp.old/util/biosdisk.c +22 -13

@@ -288,7 +288,6 @@ open_device (const grub_disk_t disk, gru
       is_partition = linux_find_partition (dev, disk->partition->start);
 
     /* Open the partition.  */
-    grub_util_info ("opening the device `%s'", dev);
     fd = open (dev, flags);
     if (fd < 0)
       {

@@ -464,6 +463,20 @@ static struct grub_disk_dev grub_util_bi
     .next = 0
   };
 
+/* Registers a device/drive map.  */
+int
+grub_util_biosdisk_register (char *device, char *drive)
+{
+  int index;
+  /* Find a free slot.  */
+  index = find_grub_drive (NULL);
+  if (index < 0)
+    return -1;
+  map[index].device = device;
+  map[index].drive = drive;
+  return 0;
+}
+
 static void
 read_device_map (const char *dev_map)
 {

@@ -486,7 +499,7 @@ read_device_map (const char *dev_map)
     {
       char *p = buf;
       char *e;
-      int drive;
+      char *device, *drive;
 
       lineno++;
 

@@ -502,19 +515,14 @@ read_device_map (const char *dev_map)
 	show_error ("No open parenthesis found");
 
       p++;
-      /* Find a free slot.  */
-      drive = find_grub_drive (NULL);
-      if (drive < 0)
-	show_error ("Map table size exceeded");
-
       e = p;
       p = strchr (p, ')');
       if (! p)
 	show_error ("No close parenthesis found");
 
-      map[drive].drive = xmalloc (p - e + sizeof ('\0'));
-      strncpy (map[drive].drive, e, p - e + sizeof ('\0'));
-      map[drive].drive[p - e] = '\0';
+      drive = xmalloc (p - e + sizeof ('\0'));
+      strncpy (drive, e, p - e + sizeof ('\0'));
+      drive[p - e] = '\0';
 
       p++;
       /* Skip leading spaces.  */

@@ -540,12 +548,13 @@ read_device_map (const char *dev_map)
       /* On Linux, the devfs uses symbolic links horribly, and that
 	 confuses the interface very much, so use realpath to expand
 	 symbolic links.  */
-      map[drive].device = xmalloc (PATH_MAX);
-      if (! realpath (p, map[drive].device))
+      device = xmalloc (PATH_MAX);
+      if (! realpath (p, device))
 	grub_util_error ("Cannot get the real path of `%s'", p);
 #else
-      map[drive].device = xstrdup (p);
+      device = xstrdup (p);
 #endif
+      grub_util_biosdisk_register (device, drive);
     }
 
   fclose (fp);

diff -x configure -x config.h.in -x CVS -x '*~' -x '*.mk' -urp ../tmp.old/util/grub-probe.c ./util/grub-probe.c

util/grub-probe.c renamed from tmp.old/util/grub-probe.c +12 -2

@@ -153,15 +153,25 @@ probe (const char *path, char *device_na
     }
 
   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.\n", device_name);
 
   if (print == PRINT_DRIVE)
     {
+      if (! drive_name)
+	grub_util_error ("Cannot find a GRUB drive for %s.  Check your device.map.\n", device_name);
+
       printf ("(%s)\n", drive_name);
       goto end;
     }
 
+  /* For the purpose of checks that follow, a dummy drive name will do.  */
+  if (! drive_name)
+    {
+      drive_name = "dummy-grub-drive";
+      grub_util_info ("Cannot find GRUB drive for %s.  Registering \"(%s)\t%s\".",
+		      device_name, drive_name, device_name);
+      grub_util_biosdisk_register (xstrdup (device_name), xstrdup (drive_name));
+    }
+
   grub_util_info ("opening %s", drive_name);
   dev = grub_device_open (drive_name);
   if (! dev)