Illumos #3745, #3811

3745 zpool create should treat -O mountpoint and -m the same
3811 zpool create -o altroot=/xyz -O mountpoint=/mnt ignores
     the mountpoint option
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Christopher Siden <christopher.siden@delphix.com>

References:
  https://www.illumos.org/issues/3745
  https://www.illumos.org/issues/3811
  illumos/illumos-gate@8b713775314bbbf24edd503b4869342d8711ce95

Ported-by: Richard Yao <ryao@gentoo.org>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Issue #1775

diff --git a/cmd/zpool/zpool_main.c b/cmd/zpool/zpool_main.c
index cc8fbaca8124c3a111f80b7e71c656e709631e22..0d21b7e73338d6cfa24abc7c942189b6042a765d 100644 (file)
--- a/cmd/zpool/zpool_main.c
+++ b/cmd/zpool/zpool_main.c
@@ -831,6 +831,7 @@ zpool_do_create(int argc, char **argv)
                                goto errout;
                        break;
                case 'm':
+                       /* Equivalent to -O mountpoint=optarg */
                        mountpoint = optarg;
                        break;
                case 'o':
@@ -869,8 +870,18 @@ zpool_do_create(int argc, char **argv)
                        *propval = '\0';
                        propval++;
 
-                       if (add_prop_list(optarg, propval, &fsprops, B_FALSE))
+                       /*
+                        * Mountpoints are checked and then added later.
+                        * Uniquely among properties, they can be specified
+                        * more than once, to avoid conflict with -m.
+                        */
+                       if (0 == strcmp(optarg,
+                           zfs_prop_to_name(ZFS_PROP_MOUNTPOINT))) {
+                               mountpoint = propval;
+                       } else if (add_prop_list(optarg, propval, &fsprops,
+                           B_FALSE)) {
                                goto errout;
+                       }
                        break;
                case ':':
                        (void) fprintf(stderr, gettext("missing argument for "
@@ -987,6 +998,18 @@ zpool_do_create(int argc, char **argv)
                }
        }
 
+       /*
+        * Now that the mountpoint's validity has been checked, ensure that
+        * the property is set appropriately prior to creating the pool.
+        */
+       if (mountpoint != NULL) {
+               ret = add_prop_list(zfs_prop_to_name(ZFS_PROP_MOUNTPOINT),
+                   mountpoint, &fsprops, B_FALSE);
+               if (ret != 0)
+                       goto errout;
+       }
+
+       ret = 1;
        if (dryrun) {
                /*
                 * For a dry run invocation, print out a basic message and run
@@ -1021,21 +1044,19 @@ zpool_do_create(int argc, char **argv)
                                if (nvlist_exists(props, propname))
                                        continue;
 
-                               if (add_prop_list(propname, ZFS_FEATURE_ENABLED,
-                                   &props, B_TRUE) != 0)
+                               ret = add_prop_list(propname,
+                                   ZFS_FEATURE_ENABLED, &props, B_TRUE);
+                               if (ret != 0)
                                        goto errout;
                        }
                }
+
+               ret = 1;
                if (zpool_create(g_zfs, poolname,
                    nvroot, props, fsprops) == 0) {
                        zfs_handle_t *pool = zfs_open(g_zfs, poolname,
                            ZFS_TYPE_FILESYSTEM);
                        if (pool != NULL) {
-                               if (mountpoint != NULL)
-                                       verify(zfs_prop_set(pool,
-                                           zfs_prop_to_name(
-                                           ZFS_PROP_MOUNTPOINT),
-                                           mountpoint) == 0);
                                if (zfs_mount(pool, NULL, 0) == 0)
                                        ret = zfs_shareall(pool);
                                zfs_close(pool);

diff --git a/lib/libzfs/libzfs_pool.c b/lib/libzfs/libzfs_pool.c
index 8b3edec168d867f52284aa6f33086acbf57dd08c..a221f9fc7fe7ba02b3f099869f7c01fea572389b 100644 (file)
--- a/lib/libzfs/libzfs_pool.c
+++ b/lib/libzfs/libzfs_pool.c
@@ -1159,7 +1159,6 @@ zpool_create(libzfs_handle_t *hdl, const char *pool, nvlist_t *nvroot,
        nvlist_t *zc_fsprops = NULL;
        nvlist_t *zc_props = NULL;
        char msg[1024];
-       char *altroot;
        int ret = -1;
 
        (void) snprintf(msg, sizeof (msg), dgettext(TEXT_DOMAIN,
@@ -1260,21 +1259,6 @@ zpool_create(libzfs_handle_t *hdl, const char *pool, nvlist_t *nvroot,
                }
        }
 
-       /*
-        * If this is an alternate root pool, then we automatically set the
-        * mountpoint of the root dataset to be '/'.
-        */
-       if (nvlist_lookup_string(props, zpool_prop_to_name(ZPOOL_PROP_ALTROOT),
-           &altroot) == 0) {
-               zfs_handle_t *zhp;
-
-               verify((zhp = zfs_open(hdl, pool, ZFS_TYPE_DATASET)) != NULL);
-               verify(zfs_prop_set(zhp, zfs_prop_to_name(ZFS_PROP_MOUNTPOINT),
-                   "/") == 0);
-
-               zfs_close(zhp);
-       }
-
 create_failed:
        zcmd_free_nvlists(&zc);
        nvlist_free(zc_props);