svn commit: r252758 - in stable/9/cddl/contrib/opensolaris: cmd/zpool lib/libzfs/common

[Xin LI]

Author: delphij
Date: Fri Jul  5 03:49:52 2013
New Revision: 252758
URL: http://svnweb.freebsd.org/changeset/base/252758

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 at delphix.com>,
  		Christopher Siden <christopher.siden at delphix.com>
  Sponsored by:	Spectra Logic

Modified:
  stable/9/cddl/contrib/opensolaris/cmd/zpool/zpool_main.c
  stable/9/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_pool.c
Directory Properties:
  stable/9/cddl/contrib/opensolaris/   (props changed)
  stable/9/cddl/contrib/opensolaris/lib/libzfs/   (props changed)

Modified: stable/9/cddl/contrib/opensolaris/cmd/zpool/zpool_main.c
==============================================================================
--- stable/9/cddl/contrib/opensolaris/cmd/zpool/zpool_main.c	Fri Jul  5 03:44:49 2013	(r252757)
+++ stable/9/cddl/contrib/opensolaris/cmd/zpool/zpool_main.c	Fri Jul  5 03:49:52 2013	(r252758)
@@ -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);

Modified: stable/9/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_pool.c
==============================================================================
--- stable/9/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_pool.c	Fri Jul  5 03:44:49 2013	(r252757)
+++ stable/9/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_pool.c	Fri Jul  5 03:49:52 2013	(r252758)
@@ -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);