[Top][All Lists]
[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
signature.asc
Description: OpenPGP digital signature