[PATCH FOR REVIEW] (For stable/8 only) change default zpool creation version to 28

[Xin Li]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Hi, Matthew,

Thanks for your comments!  See my replies inline.

On 04/03/13 16:37, Matthew Ahrens wrote:
> What if they specify "-d -o feature at ...=enabled"?  It seems like
> your code will not find the "version" property set, and then try to
> set "version=28", which is not what the user intended.

This would be caught later (feature flags can not be specified with
version) but actually that's a good question for this case:

> Also, I'm not sure why you'd need to remove the "version" property
> if you are enabling all features.  They shouldn't have specified a
> version, and I believe that this error is checked later on.

I think you are right that the case -d -o feature at ...=enabled can not
be used with version=5000 case.

I haven't come to a better idea yet, the attached patch would fix that
issue but is ugly.

In the other thread you have mentioned an alternative idea that just
have the user to explicitly do an "zpool upgrade", I'll talk with the
re@ to figure out if that's acceptable.

Again, thanks for your time and your comments!

> On Tue, Apr 2, 2013 at 3:10 PM, Xin Li <delphij at delphij.net>
> wrote:
> 
> Hi,
> 
> The release engineer team have some concerns of having zpool
> default creation version leave at 5000 (feature flags) because they
> are not supported by "higher" versions of FreeBSD, specifically,
> 9.0-RELEASE (EoL'ed last month) and 9.1-RELEASE.  A user may be
> surprised if he or she "upgrades" a FreeBSD 8.4-RELEASE system to a
> 9.1-RELEASE system.
> 
> In order to solve this, there is a proposal that changes the
> default creation version to 28 instead, as it's supported
> universally.  The attached patch implements this idea.
> 
> If there would be no objections in the next 24 hours, I will
> commit this change directly to stable/8 and take care for the
> releng/8.4 merge should I get approval for that.  If you have
> better proposal, please do speak up and just go ahead with your
> version.
> 
> Thanks in advance!
> 
> Cheers,
>> 
>> _______________________________________________ 
>> zfs-devel at freebsd.org mailing list 
>> http://lists.freebsd.org/mailman/listinfo/zfs-devel To
>> unsubscribe, send any mail to
>> "zfs-devel-unsubscribe at freebsd.org"
>> 
> _______________________________________________ 
> zfs-devel at freebsd.org mailing list 
> http://lists.freebsd.org/mailman/listinfo/zfs-devel To unsubscribe,
> send any mail to "zfs-devel-unsubscribe at freebsd.org"
> 

- -- 
Xin LI <delphij at delphij.net>    https://www.delphij.net/
FreeBSD - The Power to Serve!           Live free or die
-----BEGIN PGP SIGNATURE-----

iQEcBAEBCgAGBQJRXMvcAAoJEG80Jeu8UPuzKFEIAIst/amXnTAyCoz5PFp8Y7mO
mnGQF0EicY/WYrxSmPQkVM8LheqFg7efT8As7Weq6B1PQ6Fqcw7WHxdDYad21ywQ
rTeB/R4l0f459jOKrgjuWiIg/t27riK/xPwwLOxpcFZ8eh86kLN6n2xxW348hZ13
ma4g1wbU4dcvhPtHS4BT6/lWSEkrfpbPDKLPjbuwPuU6NhiOW1jJIr1hc8W1z+Ep
roSnN+RNKfya/Ur2MI7eN15py1BLFqzPbDrLukx2wf6i+Z9rmwd0lvMx8lZKDcEe
/9dEjWueEYYCh3i7rRM6/r4PoTbVBKimpeSEbtDtDyKpHOlu7C2ZmnE+Ej/Q+58=
=KOnh
-----END PGP SIGNATURE-----
-------------- next part --------------
Index: cddl/contrib/opensolaris/cmd/zpool/zpool_main.c
===================================================================
--- cddl/contrib/opensolaris/cmd/zpool/zpool_main.c	(revision 249071)
+++ cddl/contrib/opensolaris/cmd/zpool/zpool_main.c	(working copy)
@@ -768,6 +768,7 @@
 	boolean_t force = B_FALSE;
 	boolean_t dryrun = B_FALSE;
 	boolean_t enable_all_pool_feat = B_TRUE;
+	boolean_t spa_want_features = B_FALSE;
 	int c;
 	nvlist_t *nvroot = NULL;
 	char *poolname;
@@ -815,9 +816,6 @@
 			*propval = '\0';
 			propval++;
 
-			if (add_prop_list(optarg, propval, &props, B_TRUE))
-				goto errout;
-
 			/*
 			 * If the user is creating a pool that doesn't support
 			 * feature flags, don't enable any features.
@@ -830,8 +828,16 @@
 				if (*end == '\0' &&
 				    ver < SPA_VERSION_FEATURES) {
 					enable_all_pool_feat = B_FALSE;
+				} else if (*end == '\0' &&
+				    ver == SPA_VERSION_FEATURES) {
+					spa_want_features = B_TRUE;
+					break;
 				}
 			}
+
+			if (add_prop_list(optarg, propval, &props, B_TRUE))
+				goto errout;
+
 			break;
 		case 'O':
 			if ((propval = strchr(optarg, '=')) == NULL) {
@@ -856,6 +862,19 @@
 		}
 	}
 
+#ifdef __FreeBSD__
+	/* Compatiblity with FreeBSD 9.0 and 9.1: Use version 28 if unspecified */
+	if (nvlist_lookup_string(props,
+	    zpool_prop_to_name(ZPOOL_PROP_VERSION),
+	    &propval) != 0 && !spa_want_features &&
+	    !prop_list_contains_feature(props)) {
+		if (add_prop_list(zpool_prop_to_name(
+		    ZPOOL_PROP_VERSION), "28", &props, B_TRUE))
+			goto errout;
+		enable_all_pool_feat = B_FALSE;
+	}
+#endif /* __FreeBSD__ */
+
 	argc -= optind;
 	argv += optind;