On 02.02.2012 01:55, Daniel Kahn Gillmor wrote:
hi folks--

i was speaking with phcoder today on #grub, about getting messages like
this from grub when a partition that is part of a Linux SW RAID set
(with metadata 0.9x) is close to the end of its containing block device:

   superfluous RAID member (2 found)
Try attached patch
Here's phcoder's explanation of the problem:

16:08<  phcoder>  if you have<  64KiB between end of disk and end of partition
                  the metadata is exactly in the same place for either if the
                  whole disks are raided or only partitions. And no field which
                  allows to distinguish those
  [...]
16:09<  phcoder>  thing is that mdraid format rounds it down to a 64K aligned
                  boundary
16:10<  dkg0>  64KiB aligned to the parent disk, or to the partition itself?
16:10<  phcoder>  to whatever the host of the raid is. For your error to occur
                  it has to be both
16:10<  dkg0>  hm, i suppose if the partition itself starts evenly aligned with
               a 64KiB boundary, it'd be the same thing.

It sounds like there might be some circumstances (e.g. a RAID0 set of
sda and sdb, creating md0, and a partition table on top of md0) where it
is legitimately difficult to decide the correct interpretation.

However, i think there might be a reasonable heuristic that can be used
in the case of RAID1 sets that would avoid the ambiguity (and the scary
messages/warnings to the user:

Select the smallest known block device that can completely enclose the
RAID member.  The larger block device(s) should not be considered to be
exporting that RAID.

these heuristics would mean that RAID1 sets with 0.9x metadata on any
sort of disklabel would only have their member components show up once
during a scan, rather than treating them as a duplicate.

phcoder mentioned that the RAID code was undergoing something of an
overhaul.  perhaps these heuristics could play into that update?

I'm not sure how to address it with RAID0 or RAID10, but if we could fix
the RAID1 case, that would be a big win for a lot of users.

Regards,

        --dkg

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel



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

=== modified file 'grub-core/disk/diskfilter.c'
--- grub-core/disk/diskfilter.c	2012-02-12 14:25:25 +0000
+++ grub-core/disk/diskfilter.c	2012-02-23 05:42:14 +0000
@@ -972,35 +972,40 @@
 	: (pv->id.id == id->id))
       {
 	struct grub_diskfilter_lv *lv;
+	grub_disk_t disk;
 	/* FIXME: Check whether the update time of the superblocks are
 	   the same.  */
+	disk = grub_disk_open (disk->name);
+	if (!disk)
+	  return grub_errno;
+	if (disk && pv->disk && grub_disk_get_size (disk) >= pv->part_size)
+	  {
+	    grub_disk_close (disk);
+	    return GRUB_ERR_NONE;
+	  }
+	pv->disk = disk;
 	/* This could happen to LVM on RAID, pv->disk points to the
 	   raid device, we shouldn't change it.  */
-	if (! pv->disk)
-	  {
-	    pv->disk = grub_disk_open (disk->name);
-	    if (! pv->disk)
-	      return grub_errno;
-	    pv->part_start = grub_partition_get_start (disk->partition);
-	    pv->part_size = grub_disk_get_size (disk);
+	pv->start_sector -= pv->part_start;
+	pv->part_start = grub_partition_get_start (disk->partition);
+	pv->part_size = grub_disk_get_size (disk);
 
 #ifdef GRUB_UTIL
-	    {
-	      grub_size_t s = 1;
-	      grub_partition_t p;
-	      for (p = disk->partition; p; p = p->parent)
-		s++;
-	      pv->partmaps = xmalloc (s * sizeof (pv->partmaps[0]));
-	      s = 0;
-	      for (p = disk->partition; p; p = p->parent)
-		pv->partmaps[s++] = xstrdup (p->partmap->name);
-	      pv->partmaps[s++] = 0;
-	    }
+	{
+	  grub_size_t s = 1;
+	  grub_partition_t p;
+	  for (p = disk->partition; p; p = p->parent)
+	    s++;
+	  pv->partmaps = xmalloc (s * sizeof (pv->partmaps[0]));
+	  s = 0;
+	  for (p = disk->partition; p; p = p->parent)
+	    pv->partmaps[s++] = xstrdup (p->partmap->name);
+	  pv->partmaps[s++] = 0;
+	}
 #endif
-	    if (start_sector != (grub_uint64_t)-1)
-	      pv->start_sector = start_sector;
-	    pv->start_sector += pv->part_start;
-	  }
+	if (start_sector != (grub_uint64_t)-1)
+	  pv->start_sector = start_sector;
+	pv->start_sector += pv->part_start;
 	/* Add the device to the array. */
 	for (lv = array->lvs; lv; lv = lv->next)
 	  if (!lv->became_readable_at && lv->fullname && is_lv_readable (lv))

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Reply via email to