Skip to contentSkip to conversation

Do not embed with multiple partmaps

PATCH In review Grégoire Sutre 2 participants 3 messages Jun 17, 2010 – Jul 4, 2010

lore
Share & tools
lore

Grégoire Sutre

author
#0 lore raw

PATCH 1/1 Do not embed with multiple partmaps

Hi,

As a (temporary) solution to the problem of embedding with multiple top-level partmaps, the attached patch simply disables embedding when there are multiple partmaps (i.e. apply the same behavior as when there is no partmap).

This prevents grub-setup from overwriting e.g. a BSD label if it finds an msdos label first.

The only drawback I see is that grub-setup may complain about an old obsolete disklabel, but in that case the user simply has to destroy the obsolete label, and IMHO this is more acceptable than potentially destroying valuable data.

Grégoire

=== modified file 'ChangeLog'

ChangeLog +5 -0
@@ -1,3 +1,8 @@
+2010-06-17  Grégoire Sutre  <[email protected]>
+
+	* util/i386/pc/grub-setup.c (setup): Do not embed when there are
+	multiple (top-level) partmaps.
+
 2010-06-17  Colin Watson  <[email protected]>
 
 	* util/i386/pc/grub-setup.c (usage): Pass an extra `program_name'

=== modified file 'util/i386/pc/grub-setup.c'

i386/pc/grub-setup.c +15 -2
@@ -93,6 +93,7 @@
   grub_uint16_t core_sectors;
   grub_device_t root_dev, dest_dev;
   const char *dest_partmap;
+  int multiple_partmaps;
   grub_uint8_t *boot_drive;
   grub_disk_addr_t *kernel_sector;
   grub_uint16_t *boot_drive_check;

@@ -354,10 +355,17 @@
     {
       if (p->parent)
 	return 0;
-      dest_partmap = p->partmap->name;
-      return 1;
+      if (dest_partmap == NULL)
+	dest_partmap = p->partmap->name;
+      else if (strcmp (dest_partmap, p->partmap->name) != 0)
+	{
+	  multiple_partmaps = 1;
+	  return 1;
+	}
+      return 0;
     }
   dest_partmap = 0;
+  multiple_partmaps = 0;
   grub_partition_iterate (dest_dev->disk, identify_partmap);
 
   if (! dest_partmap)

@@ -365,6 +373,11 @@
       grub_util_warn (_("Attempting to install GRUB to a partitionless disk.  This is a BAD idea."));
       goto unable_to_embed;
     }
+  if (multiple_partmaps)
+    {
+      grub_util_warn (_("Attempting to install GRUB to a disk with multiple partition labels.  This is a BAD idea."));
+      goto unable_to_embed;
+    }
 
   if (strcmp (dest_partmap, "msdos") == 0)
     grub_partition_iterate (dest_dev->disk, find_usable_region_msdos);
#1 lore raw
Quoted 7 lines · Grégoire Sutre

Hi,

As a (temporary) solution to the problem of embedding with multiple top-level partmaps, the attached patch simply disables embedding when there are multiple partmaps (i.e. apply the same behavior as when there is no partmap).

Quoted 8 lines

This prevents grub-setup from overwriting e.g. a BSD label if it finds an msdos label first.

The only drawback I see is that grub-setup may complain about an old obsolete disklabel, but in that case the user simply has to destroy the obsolete label, and IMHO this is more acceptable than potentially destroying valuable data.

Go ahead. Just replace "BAD IDEA" with "not supported yet"

Quoted 8 lines

Grégoire

_______________________________________________
Grub-devel mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/grub-devel

Signature · 2 lines
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
author
#2 lore raw
Quoted 1 line · Vladimir 'φ-coder/phcoder' Serbinenko

Go ahead. Just replace "BAD IDEA" with "not supported yet"

Thanks, I replaced "BAD IDEA" with "not supported yet" and committed.

Grégoire.