Re: grub RAID heuristics (how can we avoid "superfluous RAID member (2 found)")

["Vladimir 'φ-coder/phcoder' Serbinenko" <phcoder@gmail.com>]

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

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


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

=== 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))