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

[Colin Watson]

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?

(The long patch to util/deviceiter.c is almost entirely due to the extra
'if' causing an indentation change.)

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,

-- 
Colin Watson                                       address@hidden