Re: [PATCH] Support RAID on virtio devices, and others

[Vladimir 'phcoder' Serbinenko]

Colin Watson wrote:
> GRUB only supports RAID on a relatively small number of device types, as
> implemented by grub_util_getdiskname. I received a bug report noting
> that this doesn't work for RAID arrays with virtio block devices (often
> used in kvm) as components. This is difficult to support using the
> approach taken by grub_util_getdiskname, as virtio devices use dynamic
> major numbers.
>
> find_root_device in util/getroot.c seemed to be exactly what I wanted:
> it just trawls /dev for the appropriate major and minor numbers. This
> code is not performance-critical, so that should be fine. 
Not true. Even in current state grub-mkconfig is taking considerable
time to complete on my system with numerous kernels. Unless someone
implements a cache (it can be invalidated after 5 minutes) not to go
through the same probing procedure on every grub-probe call I object
against adding any additional delay in probing procedure
> I made the
> function naming more consistent, added support for a default directory
> in its interface (this may have problems on Cygwin; does anyone care
> about RAID on Cygwin? If so, perhaps they can propose improvements), and
> changed the RAID code to use it.
>
> Bazaar users can merge this from:
>
>   bzr+ssh://bzr.sv.gnu.org/grub/people/cjwatson/raid-virtio/
>
> === modified file 'ChangeLog'
> --- ChangeLog 2009-10-21 12:22:05 +0000
> +++ ChangeLog 2009-10-25 01:32:02 +0000
> @@ -1,3 +1,22 @@
> +2009-10-25  Colin Watson  <address@hidden>
> +
> +     Support RAID on virtio devices, and others.
> +
> +     * util/getroot.c [__MINGW32__] (find_root_device): Rename to ...
> +     [__MINGW32__] (grub_find_device): ... this.
> +     [! __MINGW32__ && ! __CYGWIN__] (find_root_device): Rename to ...
> +     [! __MINGW32__ && ! __CYGWIN__] (grub_find_device): ... this.  Use a
> +     reasonable default if dir is NULL.
> +     [! __MINGW32__ && __CYGWIN__] (find_cygwin_root_device): Rename to
> +     ...
> +     [! __MINGW32__ && __CYGWIN__] (grub_find_device): ... this.
> +     (grub_guess_root_device): Update callers.
> +     * include/grub/util/getroot.h (grub_find_device): Add prototype.
> +
> +     * util/raid.c (grub_util_getdiskname): Remove.
> +     (grub_util_raid_getmembers): Use grub_find_device rather than
> +     grub_util_getdiskname.
> +
>  2009-10-21  Felix Zielcke  <address@hidden>
>  
>       * config.guess: Update to latest version from config git
>
> === modified file 'include/grub/util/getroot.h'
> --- include/grub/util/getroot.h       2009-04-11 09:40:39 +0000
> +++ include/grub/util/getroot.h       2009-10-25 01:22:15 +0000
> @@ -19,12 +19,15 @@
>  #ifndef GRUB_UTIL_GETROOT_HEADER
>  #define GRUB_UTIL_GETROOT_HEADER     1
>  
> +#include <sys/types.h>
> +
>  enum grub_dev_abstraction_types {
>    GRUB_DEV_ABSTRACTION_NONE,
>    GRUB_DEV_ABSTRACTION_LVM,
>    GRUB_DEV_ABSTRACTION_RAID,
>  };
>  
> +char *grub_find_device (const char *dir, dev_t dev);
>  char *grub_guess_root_device (const char *dir);
>  char *grub_get_prefix (const char *dir);
>  int grub_util_get_dev_abstraction (const char *os_dev);
>
> === modified file 'util/getroot.c'
> --- util/getroot.c    2009-07-20 20:03:18 +0000
> +++ util/getroot.c    2009-10-25 01:22:15 +0000
> @@ -172,8 +172,8 @@ grub_get_prefix (const char *dir)
>  
>  #ifdef __MINGW32__
>  
> -static char *
> -find_root_device (const char *dir __attribute__ ((unused)),
> +char *
> +grub_find_device (const char *dir __attribute__ ((unused)),
>                    dev_t dev __attribute__ ((unused)))
>  {
>    return 0;
> @@ -181,13 +181,22 @@ find_root_device (const char *dir __attr
>  
>  #elif ! defined(__CYGWIN__)
>  
> -static char *
> -find_root_device (const char *dir, dev_t dev)
> +char *
> +grub_find_device (const char *dir, dev_t dev)
>  {
>    DIR *dp;
>    char *saved_cwd;
>    struct dirent *ent;
>  
> +  if (! dir)
> +    {
> +#ifdef __CYGWIN__
> +      return NULL;
> +#else
> +      dir = "/dev";
> +#endif
> +    }
> +
>    dp = opendir (dir);
>    if (! dp)
>      return 0;
> @@ -225,7 +234,7 @@ find_root_device (const char *dir, dev_t
>         /* Find it recursively.  */
>         char *res;
>  
> -       res = find_root_device (ent->d_name, dev);
> +       res = grub_find_device (ent->d_name, dev);
>  
>         if (res)
>           {
> @@ -328,8 +337,8 @@ get_bootsec_serial (const char *os_dev, 
>    return serial;
>  }
>  
> -static char *
> -find_cygwin_root_device (const char *path, dev_t dev)
> +char *
> +grub_find_device (const char *path, dev_t dev)
>  {
>    /* No root device for /cygdrive.  */
>    if (dev == (DEV_CYGDRIVE_MAJOR << 16))
> @@ -350,7 +359,7 @@ find_cygwin_root_device (const char *pat
>  
>    /* Cygwin returns the partition serial number in stat.st_dev.
>       This is never identical to the device number of the emulated
> -     /dev/sdXN device, so above find_root_device () does not work.
> +     /dev/sdXN device, so above grub_find_device () does not work.
>       Search the partition with the same serial in boot sector instead.  */
>    char devpath[sizeof ("/dev/sda15") + 13]; /* Size + Paranoia.  */
>    int d;
> @@ -386,12 +395,12 @@ grub_guess_root_device (const char *dir)
>  
>  #ifdef __CYGWIN__
>    /* Cygwin specific function.  */
> -  os_dev = find_cygwin_root_device (dir, st.st_dev);
> +  os_dev = grub_find_device (dir, st.st_dev);
>  
>  #else
>  
>    /* This might be truly slow, but is there any better way?  */
> -  os_dev = find_root_device ("/dev", st.st_dev);
> +  os_dev = grub_find_device ("/dev", st.st_dev);
>  #endif
>  
>    return os_dev;
>
> === modified file 'util/raid.c'
> --- util/raid.c       2009-06-10 21:04:23 +0000
> +++ util/raid.c       2009-10-25 01:22:15 +0000
> @@ -21,40 +21,19 @@
>  #ifdef __linux__
>  #include <grub/util/misc.h>
>  #include <grub/util/raid.h>
> +#include <grub/util/getroot.h>
>  
>  #include <string.h>
>  #include <fcntl.h>
>  #include <sys/ioctl.h>
>  #include <errno.h>
> +#include <sys/types.h>
>  
>  #include <linux/types.h>
>  #include <linux/major.h>
>  #include <linux/raid/md_p.h>
>  #include <linux/raid/md_u.h>
>  
> -static char *
> -grub_util_getdiskname (int major, int minor)
> -{
> -  char *name = xmalloc (15);
> -
> -  if (major == LOOP_MAJOR)
> -    sprintf (name, "/dev/loop%d", minor);
> -  else if (major == IDE0_MAJOR)
> -    sprintf (name, "/dev/hd%c", 'a' + minor / 64);
> -  else if (major == IDE1_MAJOR)
> -    sprintf (name, "/dev/hd%c", 'c' + minor / 64);
> -  else if (major == IDE2_MAJOR)
> -    sprintf (name, "/dev/hd%c", 'e' + minor / 64);
> -  else if (major == IDE3_MAJOR)
> -    sprintf (name, "/dev/hd%c", 'g' + minor / 64);
> -  else if (major == SCSI_DISK0_MAJOR)
> -    sprintf (name, "/dev/sd%c", 'a' + minor / 16);
> -  else
> -    grub_util_error ("Unknown device number: %d, %d", major, minor);
> -
> -  return name;
> -}
> -
>  char **
>  grub_util_raid_getmembers (char *name)
>  {
> @@ -99,7 +78,8 @@ grub_util_raid_getmembers (char *name)
>  
>        if (disk.state & (1 << MD_DISK_ACTIVE))
>       {
> -       devicelist[j] = grub_util_getdiskname (disk.major, disk.minor);
> +       devicelist[j] = grub_find_device (NULL,
> +                                         makedev (disk.major, disk.minor));
>         j++;
>       }
>      }
>
>
>   


-- 
Regards
Vladimir 'phcoder' Serbinenko
Personal git repository: http://repo.or.cz/w/grub2/phcoder.git