Re: zfs pool devices detection in core/osdep/unix/getroot.c

[Andrei Borzenkov <arvidjaar@gmail.com>]

В Mon, 13 Apr 2015 21:00:20 +0300
Toomas Soome <tsoome@me.com> пишет:

> hi!
> 
> this update is written by Richard Yao <ryao@gentoo.org> and is addressing the issue that device names are not correctly discovered in more complex pool setups. This code walks over pool vdev list and will collect device names from vdev children, ensuring the all device names will be presented to caller.
> 
> Since Richard has been busy, Im posting this update on his behalf and based on fact that the code is been quite extensively tested by me and few other people anyhow:) 
> 
> rgds,
> toomas
> 
> 

Could you please send git format-patch (or git send-email) ready to
apply? As far as I know policy, patches on behalf of others are rather
frowned upon.

Anyway minor nitpicks

> diff --git a/grub-core/osdep/unix/getroot.c b/grub-core/osdep/unix/getroot.c
> index b98b2df..329938a 100644
> --- a/grub-core/osdep/unix/getroot.c
> +++ b/grub-core/osdep/unix/getroot.c
> @@ -159,43 +159,17 @@ xgetcwd (void)
>    return path;
>  }
>  
> -char **
> -grub_util_find_root_devices_from_poolname (char *poolname)
> -{
> -  char **devices = 0;
> -  size_t ndevices = 0;
> -  size_t devices_allocated = 0;
> -
>  #if defined(HAVE_LIBZFS) && defined(HAVE_LIBNVPAIR)
> -  zpool_handle_t *zpool;
> -  libzfs_handle_t *libzfs;
> -  nvlist_t *config, *vdev_tree;
> +static void
> +grub_util_find_child_vdevs(nvlist_t *vdev_tree, char ***devices, size_t *ndevices, size_t *devices_allocated)
> +{
>    nvlist_t **children;
>    unsigned int nvlist_count;
>    unsigned int i;
>    char *device = 0;
>  
> -  libzfs = grub_get_libzfs_handle ();
> -  if (! libzfs)
> -    return NULL;
> -
> -  zpool = zpool_open (libzfs, poolname);
> -  config = zpool_get_config (zpool, NULL);
> -
> -  if (nvlist_lookup_nvlist (config, "vdev_tree", &vdev_tree) != 0)
> -    error (1, errno, "nvlist_lookup_nvlist (\"vdev_tree\")");
> -
> -  if (nvlist_lookup_nvlist_array (vdev_tree, "children", &children, &nvlist_count) != 0)
> -    error (1, errno, "nvlist_lookup_nvlist_array (\"children\")");
> -  assert (nvlist_count > 0);
> -
> -  while (nvlist_lookup_nvlist_array (children[0], "children",
> -				     &children, &nvlist_count) == 0)
> -    assert (nvlist_count > 0);
> -
> -  for (i = 0; i < nvlist_count; i++)
> -    {
> -      if (nvlist_lookup_string (children[i], "path", &device) != 0)
> +  if (nvlist_lookup_nvlist_array (vdev_tree, "children", &children, &nvlist_count) != 0){

curly braces style

> +      if (nvlist_lookup_string (vdev_tree, "path", &device) != 0)
>  	error (1, errno, "nvlist_lookup_string (\"path\")");
>  
>        struct stat st;
> @@ -214,17 +188,50 @@ grub_util_find_root_devices_from_poolname (char *poolname)
>  	  else
>  #endif
>  	    device = xstrdup (device);
> -	  if (ndevices >= devices_allocated)
> +	  if (*ndevices >= *devices_allocated)
>  	    {
> -	      devices_allocated = 2 * (devices_allocated + 8);
> -	      devices = xrealloc (devices, sizeof (devices[0])
> -				  * devices_allocated);
> +	      *devices_allocated = 2 * (*devices_allocated + 8);
> +	      *devices = xrealloc (*devices, sizeof ((*devices)[0])
> +				  * *devices_allocated);
>  	    }
> -	  devices[ndevices++] = device;
> +	  (*devices)[(*ndevices)++] = device;
>  	}
>  
>        device = NULL;
> -    }
> +
> +  } else {

curly braces style

> +    for (i = 0; i < nvlist_count; i++)
> +      {
> +        grub_util_find_child_vdevs(children[i], devices, ndevices, devices_allocated);
> +      }
> +  }

all braces in this branch are reundant.