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

[Raphael Bossek]

2010/7/20 Vladimir 'φ-coder/phcoder' Serbinenko <address@hidden>:
> Please avoid compressing patches. Perhaps it saved you 0.1 seconds of
> transfer time but it delayed the review because it's so annoying to have
> to exit from mailer for this.
Ok, next time.

>  static int
> -grub_util_is_dmraid (const char *os_dev)
> +grub_util_is_lvm (const char *os_dev)
>  {
>   if (! strncmp (os_dev, "/dev/mapper/nvidia_", 19))
>     return 1;
> nvidia_ isn't lvm-related. Why do you want to name it lvm?
confirmed

> -  if (!strncmp (os_dev, "/dev/md", 7))
> +  if (grub_util_is_mdraid (os_dev))
> Since this function isn't reused it only increases number of lines of code.
confirmed

> +  if (! realpath (os_dev, path))
> +    return 1;
> We use realpath (os_dev, NULL);
Ok, I'll change this.

> +
> +  return ! strncmp (path, "/dev/dm-", 8);
> This check fails if devices in mapper aren't symlinks. Aren't there any
> better way using libraries?
I'll check the API from devicemapper library. Maybe udev, I do not
know right yet.

> +#ifdef HAVE_DEVICE_MAPPER
> +  GRUB_DEV_ABSTRACTION_DM,
> +#else
> +  GRUB_DEV_ABSTRACTION_DM_RESERVE_THIS_VALUE_NOTFORUSE,
> +#endif
> Why do you need distinguish dm from mdraid. In any case #if is useless:
> just use:
> +  GRUB_DEV_ABSTRACTION_DM,
Ok, I'll remove this.

I intent to recreate the patch based on an a new release of grub2 for Debian.

Greetings,
Raphael