Re: [PATCH] Software RAID (fakeraid) support for Linux device mapper

[Colin Watson]

On Thu, Aug 05, 2010 at 10:34:48AM +0200, Raphael Bossek wrote:
> here an update of my patch which apply clean with Colin's Debian
> 1.98+20100804 release.
> Coments are welcome in hope my modification will be applied upstream
> because I've to do this for my confing each time Colin release a new
> version of grub2 for Debian ;-)

I can't work out how this actually fixes anything for you, to be honest.

> diff -r 1de1becf5f1b kern/emu/getroot.c
> --- a/kern/emu/getroot.c      Thu Aug 05 10:27:19 2010 +0200
> +++ b/kern/emu/getroot.c      Thu Aug 05 10:29:04 2010 +0200
> @@ -147,9 +147,6 @@
>        free (ret);
>        ret = NULL;
> 
> -      if (major != 0)
> -     continue; /* not a virtual device */
> -
>        sep = strstr (buf + count, " - ");
>        if (!sep)
>       continue;
> @@ -164,6 +161,9 @@
>        if (!S_ISBLK (st.st_mode))
>       continue; /* not a block device */
> 
> +      if (major != 0 && ! device_is_mapped (device))
> +     continue; /* not a virtual device */
> +
>        ret = strdup (device);
>      }
> 

This is in the function find_root_device_from_mountinfo, which is only
supposed to be used for btrfs.  The restriction to virtual devices is
deliberate, because it's hard to find anything else for btrfs.  Allowing
mapped devices as well is a significant change in behaviour, and I would
rather not do this.  Mapped devices should be handled using
find_root_device, not find_root_device_from_mountinfo.  If
find_root_device isn't working, please fix that instead.

>  #ifdef HAVE_DEVICE_MAPPER
>        /* If this is a DM-RAID device.  */
> -      if ((strncmp ("mapper/", p, 7) == 0))
> +      if (device_is_mapped (path))
>       {
>         static struct dm_tree *tree = NULL;
>         uint32_t maj, min;

I don't know if I'd call this wrong as such, but the rest of this
sequence of conditionals operates on filenames so it seems rather out of
style.  I'd prefer to stick to the previous version.

Please include ChangeLog entries with your patches in future so that we
have an explanation of what they're trying to fix.  I think this one
should be done differently, though.

(There are a couple of other DM-RAID-related patches floating around
too.)

-- 
Colin Watson                                       address@hidden