Date: Fri, 5 Jul 2013 03:50:11 +0000 (UTC) From: Xin LI <delphij@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org Subject: svn commit: r252759 - in stable/8/cddl/contrib/opensolaris: cmd/zpool lib/libzfs/common Message-ID: <201307050350.r653oB7P068321@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: delphij Date: Fri Jul 5 03:50:11 2013 New Revision: 252759 URL: http://svnweb.freebsd.org/changeset/base/252759 Log: 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 <whatever> -m <whatever>". 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 <mahrens@delphix.com>, Christopher Siden <christopher.siden@delphix.com> Sponsored by: Spectra Logic Modified: stable/8/cddl/contrib/opensolaris/cmd/zpool/zpool_main.c stable/8/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_pool.c Directory Properties: stable/8/cddl/contrib/opensolaris/ (props changed) stable/8/cddl/contrib/opensolaris/lib/libzfs/ (props changed) Modified: stable/8/cddl/contrib/opensolaris/cmd/zpool/zpool_main.c ============================================================================== --- stable/8/cddl/contrib/opensolaris/cmd/zpool/zpool_main.c Fri Jul 5 03:49:52 2013 (r252758) +++ stable/8/cddl/contrib/opensolaris/cmd/zpool/zpool_main.c Fri Jul 5 03:50:11 2013 (r252759) @@ -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 " @@ -970,6 +981,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 @@ -1004,21 +1027,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); Modified: stable/8/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_pool.c ============================================================================== --- stable/8/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_pool.c Fri Jul 5 03:49:52 2013 (r252758) +++ stable/8/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_pool.c Fri Jul 5 03:50:11 2013 (r252759) @@ -1112,7 +1112,6 @@ zpool_create(libzfs_handle_t *hdl, const 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 } } - /* - * 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);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201307050350.r653oB7P068321>