[PATCH] Re: Multiple partition maps

[phcoder <phcoder@gmail.com>]

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

Robert Millan wrote:
> On Mon, Sep 08, 2008 at 08:27:05PM +0200, phcoder wrote:
>> Robert Millan wrote:
>>> On Thu, Sep 04, 2008 at 11:54:43PM +0200, phcoder wrote:
>>>> BTW GPT module checks the protective MBR. In some cases when legay OS
>>>> modified the MBR it's no longer "protective MBR". And in theese cases
>>>> GRUB will refuse to boot. Isn't the magic number check enough?
>>> If there's at least one protective GPT partition (0xee), I think this should
>>> be considered enough to accept the partmap as GPT.
>>>
>> In GPT module if first partition is not of type 0xee then it's
>> considered that no GPT is present. Is think that this check is
>> error-prone (with e.g. bootcamp) and unnecessary
> 
> Agreed.  Can you fix this?
> 

I send a patch for it. However I couldn't test it because of the bug in
the make system. About the bug I'll post in appropriate thread. In that
patch the pc module explicitely checks for the absence of GPT table. IMO
it's ugly. Another alternative would be to assign priorities to the
partition tables. I also tried this way and send patch for it. Again I
couldn't test it.

Vladimir 'phcoder' Serbinenko


[-- Attachment #2: parts.diff --]
[-- Type: text/x-diff, Size: 4704 bytes --]

Index: kern/partition.c
===================================================================
--- kern/partition.c	(revision 1861)
+++ kern/partition.c	(working copy)
@@ -25,8 +25,20 @@
 void
 grub_partition_map_register (grub_partition_map_t partmap)
 {
-  partmap->next = grub_partition_map_list;
-  grub_partition_map_list = partmap;
+  grub_partition_map_t cur, prev=0;
+  for (cur=grub_partition_map_list;cur && cur->priority>partmap->priority; 
+       cur=cur->next)
+    prev=cur;
+  if (!prev)
+    {
+      partmap->next = grub_partition_map_list;
+      grub_partition_map_list = partmap;
+    }
+  else
+    {
+      partmap->next=prev->next;
+      prev->next=partmap;
+    }
 }
 
 void
Index: include/grub/partition.h
===================================================================
--- include/grub/partition.h	(revision 1861)
+++ include/grub/partition.h	(working copy)
@@ -42,6 +42,9 @@
   
   /* Return the name of the partition PARTITION.  */
   char *(*get_name) (const grub_partition_t partition);
+
+  /* Partmaps with higher priorities are checked first */
+  int priority;
   
   /* The next partition map type.  */
   struct grub_partition_map *next;
Index: partmap/amiga.c
===================================================================
--- partmap/amiga.c	(revision 1861)
+++ partmap/amiga.c	(working copy)
@@ -205,7 +205,8 @@
     .name = "amiga_partition_map",
     .iterate = amiga_partition_map_iterate,
     .probe = amiga_partition_map_probe,
-    .get_name = amiga_partition_map_get_name
+    .get_name = amiga_partition_map_get_name,
+    .priority = 0
   };
 
 GRUB_MOD_INIT(amiga_partition_map)
Index: partmap/apple.c
===================================================================
--- partmap/apple.c	(revision 1861)
+++ partmap/apple.c	(working copy)
@@ -241,7 +241,8 @@
     .name = "apple_partition_map",
     .iterate = apple_partition_map_iterate,
     .probe = apple_partition_map_probe,
-    .get_name = apple_partition_map_get_name
+    .get_name = apple_partition_map_get_name,
+    .priority = 0
   };
 
 GRUB_MOD_INIT(apple_partition_map)
Index: partmap/acorn.c
===================================================================
--- partmap/acorn.c	(revision 1861)
+++ partmap/acorn.c	(working copy)
@@ -192,7 +192,8 @@
   .name = "Linux/ADFS partition map",
   .iterate = acorn_partition_map_iterate,
   .probe = acorn_partition_map_probe,
-  .get_name = acorn_partition_map_get_name
+  .get_name = acorn_partition_map_get_name,
+  .priority = 0
 };
 
 GRUB_MOD_INIT(acorn_partition_map)
Index: partmap/pc.c
===================================================================
--- partmap/pc.c	(revision 1861)
+++ partmap/pc.c	(working copy)
@@ -111,7 +111,7 @@
   pcdata.dos_part = -1;
   p.data = &pcdata;
   p.partmap = &grub_pc_partition_map;
-  
+
   while (1)
     {
       int i;
@@ -303,7 +303,8 @@
     .name = "pc_partition_map",
     .iterate = pc_partition_map_iterate,
     .probe = pc_partition_map_probe,
-    .get_name = pc_partition_map_get_name
+    .get_name = pc_partition_map_get_name,
+    .priority = -10
   };
 
 GRUB_MOD_INIT(pc_partition_map)
Index: partmap/sun.c
===================================================================
--- partmap/sun.c	(revision 1861)
+++ partmap/sun.c	(working copy)
@@ -205,7 +205,8 @@
     .name = "sun_partition_map",
     .iterate = sun_partition_map_iterate,
     .probe = sun_partition_map_probe,
-    .get_name = sun_partition_map_get_name
+    .get_name = sun_partition_map_get_name,
+    .priority = 0
   };
 
 GRUB_MOD_INIT(sun_partition_map)
Index: partmap/gpt.c
===================================================================
--- partmap/gpt.c	(revision 1861)
+++ partmap/gpt.c	(working copy)
@@ -58,18 +58,6 @@
   raw = *disk;
   raw.partition = 0;
 
-  /* Read the protective MBR.  */
-  if (grub_disk_read (&raw, 0, 0, sizeof (mbr), (char *) &mbr))
-    return grub_errno;
-
-  /* Check if it is valid.  */
-  if (mbr.signature != grub_cpu_to_le16 (GRUB_PC_PARTITION_SIGNATURE))
-    return grub_error (GRUB_ERR_BAD_PART_TABLE, "no signature");
-
-  /* Make sure the MBR is a protective MBR and not a normal MBR.  */
-  if (mbr.entries[0].type != GRUB_PC_PARTITION_TYPE_GPT_DISK)
-    return grub_error (GRUB_ERR_BAD_PART_TABLE, "no GPT partition map found");
-
   /* Read the GPT header.  */
   if (grub_disk_read (&raw, 1, 0, sizeof (gpt), (char *) &gpt))
     return grub_errno;
@@ -183,7 +171,8 @@
     .name = "gpt_partition_map",
     .iterate = gpt_partition_map_iterate,
     .probe = gpt_partition_map_probe,
-    .get_name = gpt_partition_map_get_name
+    .get_name = gpt_partition_map_get_name,
+    .priority = 0
   };
 
 GRUB_MOD_INIT(gpt_partition_map)

[-- Attachment #3: partsb.diff --]
[-- Type: text/x-diff, Size: 1764 bytes --]

Index: pc.c
===================================================================
--- pc.c        (revision 1861)
+++ pc.c        (working copy)
@@ -19,11 +19,17 @@

 #include <grub/partition.h>
 #include <grub/pc_partition.h>
+#include <grub/gpt_partition.h>
 #include <grub/disk.h>
 #include <grub/mm.h>
 #include <grub/misc.h>
 #include <grub/dl.h>

+static grub_uint8_t grub_gpt_magic[8] =
+  {
+    0x45, 0x46, 0x49, 0x20, 0x50, 0x41, 0x52, 0x54
+  };
+
 static struct grub_partition_map grub_pc_partition_map;

 #ifndef GRUB_UTIL
@@ -111,6 +117,13 @@
   pcdata.dos_part = -1;
   p.data = &pcdata;
   p.partmap = &grub_pc_partition_map;
+
+  /* Read the GPT header.  */
+  if (grub_disk_read (&raw, 1, 0, sizeof (gpt), (char *) &gpt))
+    return grub_errno;
+
+  if (!grub_memcmp (gpt.magic, grub_gpt_magic, sizeof (grub_gpt_magic)))
+    return grub_error (GRUB_ERR_BAD_PART_TABLE, "valid GPT header");

   while (1)
     {
Index: gpt.c
===================================================================
--- gpt.c       (revision 1861)
+++ gpt.c       (working copy)
@@ -58,18 +58,6 @@
   raw = *disk;
   raw.partition = 0;

-  /* Read the protective MBR.  */
-  if (grub_disk_read (&raw, 0, 0, sizeof (mbr), (char *) &mbr))
-    return grub_errno;
-
-  /* Check if it is valid.  */
-  if (mbr.signature != grub_cpu_to_le16 (GRUB_PC_PARTITION_SIGNATURE))
-    return grub_error (GRUB_ERR_BAD_PART_TABLE, "no signature");
-
-  /* Make sure the MBR is a protective MBR and not a normal MBR.  */
-  if (mbr.entries[0].type != GRUB_PC_PARTITION_TYPE_GPT_DISK)
-    return grub_error (GRUB_ERR_BAD_PART_TABLE, "no GPT partition map found");
-
   /* Read the GPT header.  */
   if (grub_disk_read (&raw, 1, 0, sizeof (gpt), (char *) &gpt))
     return grub_errno;