Improve `zpool import` search behavior

The goal of this change is to make 'zpool import' prefer to use
the peristent /dev/mapper or /dev/disk/by-* paths.  These are far
preferable to the devices in /dev/ whos names are not persistent
and are determined by the order in which a device is detected.

This patch improves things by changing the default search path from
just to the top level /dev/ directory to (in order):

  /dev/disk/by-vdev   - Custom rules, use first if they exist
  /dev/disk/zpool     - Custom rules, use first if they exist
  /dev/mapper         - Use multipath devices before components
  /dev/disk/by-uuid   - Single unique entry and persistent
  /dev/disk/by-id     - May be multiple entries and persistent
  /dev/disk/by-path   - Encodes physical location and persistent
  /dev/disk/by-label  - Custom persistent labels
  /dev                - UNSAFE device names will change

The default search path can be overriden by setting the
ZPOOL_IMPORT_PATH environment variable.  This must be a colon
delimited list of paths which are searched for vdevs.  If the
'zpool import -d' option is specified only those listed paths
will be searched.

Finally, when multiple paths to the same device are found.  If one
of the paths is an exact match for the path used last time to import
the pool it will be used.  When there are no exact matches the
prefered path will be determined by the provided search order.

This means you can still import a pool and force specific names by
providing the -d <path> option.  And the prefered names will persist
as long as those paths exist on your system.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #965

diff --git a/cmd/zpool/zpool_main.c b/cmd/zpool/zpool_main.c
index 312f157..cf8d5ed 100644 (file)
--- a/cmd/zpool/zpool_main.c
+++ b/cmd/zpool/zpool_main.c
@@ -1647,6 +1647,7 @@ int
 zpool_do_import(int argc, char **argv)
 {
        char **searchdirs = NULL;
+       char *env, *envdup = NULL;
        int nsearch = 0;
        int c;
        int err = 0;
@@ -1846,6 +1847,30 @@ zpool_do_import(int argc, char **argv)
                idata.unique = B_TRUE;
        }
 
+       /*
+        * Check the environment for the preferred search path.
+        */
+       if ((searchdirs == NULL) && (env = getenv("ZPOOL_IMPORT_PATH"))) {
+               char *dir;
+
+               envdup = strdup(env);
+
+               dir = strtok(envdup, ":");
+               while (dir != NULL) {
+                       if (searchdirs == NULL) {
+                               searchdirs = safe_malloc(sizeof (char *));
+                       } else {
+                               char **tmp = safe_malloc((nsearch + 1) *
+                                   sizeof (char *));
+                               bcopy(searchdirs, tmp, nsearch *
+                                   sizeof (char *));
+                               free(searchdirs);
+                               searchdirs = tmp;
+                       }
+                       searchdirs[nsearch++] = dir;
+                       dir = strtok(NULL, ":");
+               }
+       }
 
        idata.path = searchdirs;
        idata.paths = nsearch;
@@ -1882,6 +1907,8 @@ zpool_do_import(int argc, char **argv)
        if (err == 1) {
                if (searchdirs != NULL)
                        free(searchdirs);
+               if (envdup != NULL)
+                       free(envdup);
                nvlist_free(policy);
                return (1);
        }
@@ -1984,6 +2011,8 @@ error:
        nvlist_free(policy);
        if (searchdirs != NULL)
                free(searchdirs);
+       if (envdup != NULL)
+               free(envdup);
 
        return (err ? 1 : 0);
 }

diff --git a/lib/libzfs/libzfs_import.c b/lib/libzfs/libzfs_import.c
index d12dc86..c2d87f8 100644 (file)
--- a/lib/libzfs/libzfs_import.c
+++ b/lib/libzfs/libzfs_import.c
@@ -87,6 +87,7 @@ typedef struct pool_entry {
 typedef struct name_entry {
        char                    *ne_name;
        uint64_t                ne_guid;
+       uint64_t                ne_order;
        struct name_entry       *ne_next;
 } name_entry_t;
 
@@ -132,7 +133,6 @@ fix_paths(nvlist_t *nv, name_entry_t *names)
        uint64_t guid;
        name_entry_t *ne, *best;
        char *path, *devid;
-       int matched;
 
        if (nvlist_lookup_nvlist_array(nv, ZPOOL_CONFIG_CHILDREN,
            &child, &children) == 0) {
@@ -148,44 +148,33 @@ fix_paths(nvlist_t *nv, name_entry_t *names)
         * the path and see if we can calculate a new devid.
         *
         * There may be multiple names associated with a particular guid, in
-        * which case we have overlapping slices or multiple paths to the same
-        * disk.  If this is the case, then we want to pick the path that is
-        * the most similar to the original, where "most similar" is the number
-        * of matching characters starting from the end of the path.  This will
-        * preserve slice numbers even if the disks have been reorganized, and
-        * will also catch preferred disk names if multiple paths exist.
+        * which case we have overlapping partitions or multiple paths to the
+        * same disk.  In this case we prefer to use the path name which
+        * matches the ZPOOL_CONFIG_PATH.  If no matching entry is found we
+        * use the lowest order device which corresponds to the first match
+        * while traversing the ZPOOL_IMPORT_PATH search path.
         */
        verify(nvlist_lookup_uint64(nv, ZPOOL_CONFIG_GUID, &guid) == 0);
        if (nvlist_lookup_string(nv, ZPOOL_CONFIG_PATH, &path) != 0)
                path = NULL;
 
-       matched = 0;
        best = NULL;
        for (ne = names; ne != NULL; ne = ne->ne_next) {
                if (ne->ne_guid == guid) {
-                       const char *src, *dst;
-                       int count;
 
                        if (path == NULL) {
                                best = ne;
                                break;
                        }
 
-                       src = ne->ne_name + strlen(ne->ne_name) - 1;
-                       dst = path + strlen(path) - 1;
-                       for (count = 0; src >= ne->ne_name && dst >= path;
-                           src--, dst--, count++)
-                               if (*src != *dst)
-                                       break;
-
-                       /*
-                        * At this point, 'count' is the number of characters
-                        * matched from the end.
-                        */
-                       if (count > matched || best == NULL) {
+                       if ((strlen(path) == strlen(ne->ne_name)) &&
+                           !strncmp(path, ne->ne_name, strlen(path))) {
                                best = ne;
-                               matched = count;
+                               break;
                        }
+
+                       if (ne->ne_order < best->ne_order || best == NULL)
+                               best = ne;
                }
        }
 
@@ -211,7 +200,7 @@ fix_paths(nvlist_t *nv, name_entry_t *names)
  */
 static int
 add_config(libzfs_handle_t *hdl, pool_list_t *pl, const char *path,
-    nvlist_t *config)
+    int order, nvlist_t *config)
 {
        uint64_t pool_guid, vdev_guid, top_guid, txg, state;
        pool_entry_t *pe;
@@ -236,6 +225,7 @@ add_config(libzfs_handle_t *hdl, pool_list_t *pl, const char *path,
                        return (-1);
                }
                ne->ne_guid = vdev_guid;
+               ne->ne_order = order;
                ne->ne_next = pl->names;
                pl->names = ne;
                return (0);
@@ -337,6 +327,7 @@ add_config(libzfs_handle_t *hdl, pool_list_t *pl, const char *path,
        }
 
        ne->ne_guid = vdev_guid;
+       ne->ne_order = order;
        ne->ne_next = pl->names;
        pl->names = ne;
 
@@ -978,7 +969,7 @@ zpool_find_import_blkid(libzfs_handle_t *hdl, pool_list_t *pools)
                }
 
                if (config != NULL) {
-                       err = add_config(hdl, pools, devname, config);
+                       err = add_config(hdl, pools, devname, 0, config);
                        if (err != 0)
                                goto err_blkid3;
                }
@@ -993,6 +984,20 @@ err_blkid1:
 }
 #endif /* HAVE_LIBBLKID */
 
+#define DEFAULT_IMPORT_PATH_SIZE       8
+
+static char *
+zpool_default_import_path[DEFAULT_IMPORT_PATH_SIZE] = {
+       "/dev/disk/by-vdev",    /* Custom rules, use first if they exist */
+       "/dev/disk/zpool",      /* Custom rules, use first if they exist */
+       "/dev/mapper",          /* Use multipath devices before components */
+       "/dev/disk/by-uuid",    /* Single unique entry and persistent */
+       "/dev/disk/by-id",      /* May be multiple entries and persistent */
+       "/dev/disk/by-path",    /* Encodes physical location and persistent */
+       "/dev/disk/by-label",   /* Custom persistent labels */
+       "/dev"                  /* UNSAFE device names will change */
+};
+
 /*
  * Given a list of directories to search, find all pools stored on disk.  This
  * includes partial pools which are not available to import.  If no args are
@@ -1011,7 +1016,6 @@ zpool_find_import_impl(libzfs_handle_t *hdl, importargs_t *iarg)
        size_t pathleft;
        struct stat64 statbuf;
        nvlist_t *ret = NULL, *config;
-       static char *default_dir = DISK_ROOT;
        int fd;
        pool_list_t pools = { 0 };
        pool_entry_t *pe, *penext;
@@ -1031,8 +1035,9 @@ zpool_find_import_impl(libzfs_handle_t *hdl, importargs_t *iarg)
                    dgettext(TEXT_DOMAIN, "blkid failure falling back "
                    "to manual probing"));
 #endif /* HAVE_LIBBLKID */
-               dirs = 1;
-               dir = &default_dir;
+
+               dir = zpool_default_import_path;
+               dirs = DEFAULT_IMPORT_PATH_SIZE;
        }
 
        /*
@@ -1046,6 +1051,12 @@ zpool_find_import_impl(libzfs_handle_t *hdl, importargs_t *iarg)
 
                /* use realpath to normalize the path */
                if (realpath(dir[i], path) == 0) {
+
+                       /* it is safe to skip missing search paths */
+                       if (errno == ENOENT)
+                               continue;
+
+                       zfs_error_aux(hdl, strerror(errno));
                        (void) zfs_error_fmt(hdl, EZFS_BADPATH,
                            dgettext(TEXT_DOMAIN, "cannot open '%s'"), dir[i]);
                        goto error;
@@ -1156,7 +1167,7 @@ zpool_find_import_impl(libzfs_handle_t *hdl, importargs_t *iarg)
                                }
                                /* use the non-raw path for the config */
                                (void) strlcpy(end, name, pathleft);
-                               if (add_config(hdl, &pools, path, config) != 0)
+                               if (add_config(hdl, &pools, path, i+1, config))
                                        goto error;
                        }
                }