grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Fix verbose error output when device-mapper not supported by


From: Vladimir 'φ-coder/phcoder' Serbinenko
Subject: Re: [PATCH] Fix verbose error output when device-mapper not supported by kernel
Date: Mon, 07 Jun 2010 22:57:27 +0200
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.9) Gecko/20100515 Icedove/3.0.4

On 06/03/2010 06:23 PM, Colin Watson wrote:
> Following the merge of my dmraid-probe branch, several people have
> reported extremely verbose error output in the event that the kernel
> doesn't have device-mapper support (perhaps because the module isn't
> loaded; it's modular in the stock Debian kernel).  See e.g.
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=584196.  The attached
> patch fixes this.  OK for trunk?
>
>   
Go ahead.
> (The long patch to util/deviceiter.c is almost entirely due to the extra
> 'if' causing an indentation change.)
>   
For such cases please send a patch with --diff-options=-bpB
> 2010-06-03  Colin Watson  <address@hidden>
>
>       * kern/emu/misc.c (device_mapper_null_log): New function.
>       (grub_device_mapper_supported): New function.
>       * include/grub/emu/misc.h (grub_device_mapper_supported): Add
>       prototype.
>       * kern/emu/hostdisk.c (find_partition_start): Check whether
>       device-mapper is supported before trying to use it.
>       * util/deviceiter.c (grub_util_iterate_devices): Likewise.
>
> === modified file 'include/grub/emu/misc.h'
> --- include/grub/emu/misc.h   2010-05-28 13:48:45 +0000
> +++ include/grub/emu/misc.h   2010-06-03 16:00:06 +0000
> @@ -48,4 +48,8 @@ int EXPORT_FUNC(asprintf) (char **buf, c
>  char * EXPORT_FUNC(xasprintf) (const char *fmt, ...);
>  extern char * canonicalize_file_name (const char *path);
>  
> +#ifdef HAVE_DEVICE_MAPPER
> +int grub_device_mapper_supported (void);
> +#endif
> +
>  #endif /* GRUB_EMU_MISC_H */
>
> === modified file 'kern/emu/hostdisk.c'
> --- kern/emu/hostdisk.c       2010-06-02 22:47:22 +0000
> +++ kern/emu/hostdisk.c       2010-06-03 16:00:06 +0000
> @@ -342,7 +342,7 @@ find_partition_start (const char *dev)
>  # endif /* !defined(__NetBSD__) */
>  
>  # ifdef HAVE_DEVICE_MAPPER
> -  if (device_is_mapped (dev)) {
> +  if (grub_device_mapper_supported () && device_is_mapped (dev)) {
>      struct dm_task *task = NULL;
>      grub_uint64_t start, length;
>      char *target_type, *params, *space;
>
> === modified file 'kern/emu/misc.c'
> --- kern/emu/misc.c   2010-05-27 14:45:41 +0000
> +++ kern/emu/misc.c   2010-06-03 16:00:06 +0000
> @@ -22,6 +22,10 @@
>  #include <grub/time.h>
>  #include <grub/emu/misc.h>
>  
> +#ifdef HAVE_DEVICE_MAPPER
> +# include <libdevmapper.h>
> +#endif
> +
>  int verbosity;
>  
>  void
> @@ -311,3 +315,38 @@ grub_make_system_path_relative_to_its_ro
>  
>    return buf3;
>  }
> +
> +#ifdef HAVE_DEVICE_MAPPER
> +static void device_mapper_null_log (int level __attribute__ ((unused)),
> +                                 const char *file __attribute__ ((unused)),
> +                                 int line __attribute__ ((unused)),
> +                                 int dm_errno __attribute__ ((unused)),
> +                                 const char *f __attribute__ ((unused)),
> +                                 ...)
> +{
> +}
> +
> +int
> +grub_device_mapper_supported (void)
> +{
> +  static int supported = -1;
> +
> +  if (supported == -1)
> +    {
> +      struct dm_task *dmt;
> +
> +      /* Suppress annoying log messages.  */
> +      dm_log_with_errno_init (&device_mapper_null_log);
> +
> +      dmt = dm_task_create (DM_DEVICE_VERSION);
> +      supported = (dmt != NULL);
> +      if (dmt)
> +     dm_task_destroy (dmt);
> +
> +      /* Restore the original logger.  */
> +      dm_log_with_errno_init (NULL);
> +    }
> +
> +  return supported;
> +}
> +#endif /* HAVE_DEVICE_MAPPER */
>
> === modified file 'util/deviceiter.c'
> --- util/deviceiter.c 2010-01-26 14:26:16 +0000
> +++ util/deviceiter.c 2010-06-03 16:00:06 +0000
> @@ -33,6 +33,7 @@
>  #include <grub/util/deviceiter.h>
>  #include <grub/list.h>
>  #include <grub/misc.h>
> +#include <grub/emu/misc.h>
>  
>  #ifdef __linux__
>  # if !defined(__GLIBC__) || \
> @@ -676,112 +677,113 @@ grub_util_iterate_devices (int NESTED_FU
>      }
>  
>    /* DM-RAID.  */
> -  {
> -    struct dm_tree *tree = NULL;
> -    struct dm_task *task = NULL;
> -    struct dm_names *names = NULL;
> -    unsigned int next = 0;
> -    void *top_handle, *second_handle;
> -    struct dm_tree_node *root, *top, *second;
> -    struct dmraid_seen *seen = NULL;
> -
> -    /* Build DM tree for all devices.  */
> -    tree = dm_tree_create ();
> -    dmraid_check (tree, "dm_tree_create failed\n");
> -    task = dm_task_create (DM_DEVICE_LIST);
> -    dmraid_check (task, "dm_task_create failed\n");
> -    dmraid_check (dm_task_run (task), "dm_task_run failed\n");
> -    names = dm_task_get_names (task);
> -    dmraid_check (names, "dm_task_get_names failed\n");
> -    dmraid_check (names->dev, "No DM devices found\n");
> -    do
> -      {
> -     names = (void *) names + next;
> -     dmraid_check (dm_tree_add_dev (tree, MAJOR (names->dev),
> -                                    MINOR (names->dev)),
> -                      "dm_tree_add_dev (%s) failed\n", names->name);
> -     next = names->next;
> -      }
> -    while (next);
> -
> -    /* Walk the second-level children of the inverted tree; that is, devices
> -       which are directly composed of non-DM devices such as hard disks.
> -       This class includes all DM-RAID disks and excludes all DM-RAID
> -       partitions.  */
> -    root = dm_tree_find_node (tree, 0, 0);
> -    top_handle = NULL;
> -    top = dm_tree_next_child (&top_handle, root, 1);
> -    while (top)
> -      {
> -     second_handle = NULL;
> -     second = dm_tree_next_child (&second_handle, top, 1);
> -     while (second)
> -       {
> -         const char *node_name, *node_uuid;
> -         char *name;
> -         struct dmraid_seen *seen_elt;
> -
> -         node_name = dm_tree_node_get_name (second);
> -         dmraid_check (node_name, "dm_tree_node_get_name failed\n");
> -         node_uuid = dm_tree_node_get_uuid (second);
> -         dmraid_check (node_uuid, "dm_tree_node_get_uuid failed\n");
> -         if (strncmp (node_uuid, "DMRAID-", 7) != 0)
> -           {
> -             grub_dprintf ("deviceiter", "%s is not DM-RAID\n", node_name);
> -             goto dmraid_next_child;
> -           }
> -
> -         /* Have we already seen this node?  There are typically very few
> -            DM-RAID disks, so a list should be fast enough.  */
> -         if (grub_named_list_find (GRUB_AS_NAMED_LIST (seen), node_name))
> -           {
> -             grub_dprintf ("deviceiter", "Already seen DM device %s\n",
> -                           node_name);
> -             goto dmraid_next_child;
> -           }
> -
> -         name = xasprintf ("/dev/mapper/%s", node_name);
> -         if (check_device (name))
> -           {
> -             if (hook (name, 0))
> -               {
> -                 free (name);
> -                 while (seen)
> -                   {
> -                     struct dmraid_seen *seen_elt =
> -                       grub_list_pop (GRUB_AS_LIST_P (&seen));
> -                     free (seen_elt);
> -                   }
> -                 if (task)
> -                   dm_task_destroy (task);
> -                 if (tree)
> -                   dm_tree_free (tree);
> -                 return;
> -               }
> -           }
> -         free (name);
> +  if (grub_device_mapper_supported ())
> +    {
> +      struct dm_tree *tree = NULL;
> +      struct dm_task *task = NULL;
> +      struct dm_names *names = NULL;
> +      unsigned int next = 0;
> +      void *top_handle, *second_handle;
> +      struct dm_tree_node *root, *top, *second;
> +      struct dmraid_seen *seen = NULL;
> +
> +      /* Build DM tree for all devices.  */
> +      tree = dm_tree_create ();
> +      dmraid_check (tree, "dm_tree_create failed\n");
> +      task = dm_task_create (DM_DEVICE_LIST);
> +      dmraid_check (task, "dm_task_create failed\n");
> +      dmraid_check (dm_task_run (task), "dm_task_run failed\n");
> +      names = dm_task_get_names (task);
> +      dmraid_check (names, "dm_task_get_names failed\n");
> +      dmraid_check (names->dev, "No DM devices found\n");
> +      do
> +     {
> +       names = (void *) names + next;
> +       dmraid_check (dm_tree_add_dev (tree, MAJOR (names->dev),
> +                                      MINOR (names->dev)),
> +                        "dm_tree_add_dev (%s) failed\n", names->name);
> +       next = names->next;
> +     }
> +      while (next);
>  
> -         seen_elt = xmalloc (sizeof *seen_elt);
> -         seen_elt->name = node_name;
> -         grub_list_push (GRUB_AS_LIST_P (&seen), GRUB_AS_LIST (seen_elt));
> +      /* Walk the second-level children of the inverted tree; that is, 
> devices
> +      which are directly composed of non-DM devices such as hard disks.
> +      This class includes all DM-RAID disks and excludes all DM-RAID
> +      partitions.  */
> +      root = dm_tree_find_node (tree, 0, 0);
> +      top_handle = NULL;
> +      top = dm_tree_next_child (&top_handle, root, 1);
> +      while (top)
> +     {
> +       second_handle = NULL;
> +       second = dm_tree_next_child (&second_handle, top, 1);
> +       while (second)
> +         {
> +           const char *node_name, *node_uuid;
> +           char *name;
> +           struct dmraid_seen *seen_elt;
> +
> +           node_name = dm_tree_node_get_name (second);
> +           dmraid_check (node_name, "dm_tree_node_get_name failed\n");
> +           node_uuid = dm_tree_node_get_uuid (second);
> +           dmraid_check (node_uuid, "dm_tree_node_get_uuid failed\n");
> +           if (strncmp (node_uuid, "DMRAID-", 7) != 0)
> +             {
> +               grub_dprintf ("deviceiter", "%s is not DM-RAID\n", node_name);
> +               goto dmraid_next_child;
> +             }
> +
> +           /* Have we already seen this node?  There are typically very few
> +              DM-RAID disks, so a list should be fast enough.  */
> +           if (grub_named_list_find (GRUB_AS_NAMED_LIST (seen), node_name))
> +             {
> +               grub_dprintf ("deviceiter", "Already seen DM device %s\n",
> +                             node_name);
> +               goto dmraid_next_child;
> +             }
> +
> +           name = xasprintf ("/dev/mapper/%s", node_name);
> +           if (check_device (name))
> +             {
> +               if (hook (name, 0))
> +                 {
> +                   free (name);
> +                   while (seen)
> +                     {
> +                       struct dmraid_seen *seen_elt =
> +                         grub_list_pop (GRUB_AS_LIST_P (&seen));
> +                       free (seen_elt);
> +                     }
> +                   if (task)
> +                     dm_task_destroy (task);
> +                   if (tree)
> +                     dm_tree_free (tree);
> +                   return;
> +                 }
> +             }
> +           free (name);
> +
> +           seen_elt = xmalloc (sizeof *seen_elt);
> +           seen_elt->name = node_name;
> +           grub_list_push (GRUB_AS_LIST_P (&seen), GRUB_AS_LIST (seen_elt));
>  
>  dmraid_next_child:
> -         second = dm_tree_next_child (&second_handle, top, 1);
> -       }
> -     top = dm_tree_next_child (&top_handle, root, 1);
> -      }
> +           second = dm_tree_next_child (&second_handle, top, 1);
> +         }
> +       top = dm_tree_next_child (&top_handle, root, 1);
> +     }
>  
>  dmraid_end:
> -    while (seen)
> -      {
> -     struct dmraid_seen *seen_elt = grub_list_pop (GRUB_AS_LIST_P (&seen));
> -     free (seen_elt);
> -      }
> -    if (task)
> -      dm_task_destroy (task);
> -    if (tree)
> -      dm_tree_free (tree);
> -  }
> +      while (seen)
> +     {
> +       struct dmraid_seen *seen_elt = grub_list_pop (GRUB_AS_LIST_P (&seen));
> +       free (seen_elt);
> +     }
> +      if (task)
> +     dm_task_destroy (task);
> +      if (tree)
> +     dm_tree_free (tree);
> +    }
>  # endif /* HAVE_DEVICE_MAPPER */
>  #endif /* __linux__ */
>  }
>
> Thanks,
>
>   


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


Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]