From efd5dc02f150bee9846e05d7e9b8960326a3222f Mon Sep 17 00:00:00 2001 From: delphij Date: Fri, 5 Jul 2013 03:49:52 +0000 Subject: [PATCH] MFC r251634: illumos #3745 zpool create should treat -O mountpoint and -m the same cddl/contrib/opensolaris/cmd/zpool/zpool_main.c: (change 644608) This allows specifying a mountpoint using the latter form and having its value checked and used as it would be using the former form. As a consequence of this change: 1. The mountpoint property is set in the fsprops nvlist prior to creating the pool, rather than being set after creating the pool. To me, this is the proper approach, since it avoids creating the pool if the mountpoint setting would cause the command to fail. 2. The mountpoint property, unlike all others, can be specified more than once. Only the last setting takes effect. This is to avoid breaking potential existing users that specify -m more than once. Submitted by: will cddl/contrib/opensolaris/lib/libzfs/common/libzfs_pool.c Fix "zpool create -R -m ". Ever since change 644608, this has been broken. The problem is that some old code in libzfs_pool.c would force a pool's mountpoint to "/" when creating a pool with an altroot. That probably implemented some old policy decision regarding altroots, but it conflicts with the current manpage. It also had no effect until 644608, because the zpool command would _always_ change the pool's mountpoint after creating it. The solution is to delete the old code from libzfs_pool.c. Submitted by: asomers Reviewed by: Matthew Ahrens , Christopher Siden Sponsored by: Spectra Logic git-svn-id: svn://svn.freebsd.org/base/stable/9@252758 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f --- .../opensolaris/cmd/zpool/zpool_main.c | 37 +++++++++++++++---- .../lib/libzfs/common/libzfs_pool.c | 16 -------- 2 files changed, 29 insertions(+), 24 deletions(-) diff --git a/cddl/contrib/opensolaris/cmd/zpool/zpool_main.c b/cddl/contrib/opensolaris/cmd/zpool/zpool_main.c index 078447479..dfa8cbdee 100644 --- a/cddl/contrib/opensolaris/cmd/zpool/zpool_main.c +++ b/cddl/contrib/opensolaris/cmd/zpool/zpool_main.c @@ -804,6 +804,7 @@ zpool_do_create(int argc, char **argv) goto errout; break; case 'm': + /* Equivalent to -O mountpoint=optarg */ mountpoint = optarg; break; case 'o': @@ -842,8 +843,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 " @@ -960,6 +971,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 @@ -994,21 +1017,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/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_pool.c b/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_pool.c index 9b04e08c5..3ce962310 100644 --- a/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_pool.c +++ b/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_pool.c @@ -1112,7 +1112,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, @@ -1211,21 +1210,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); -- 2.45.0