From e086db16568e2cb8f484325481430aafd414d913 Mon Sep 17 00:00:00 2001 From: Colm Date: Mon, 12 Apr 2021 17:08:56 +0100 Subject: [PATCH] Improvements to the 'compatibility' property Several improvements to the operation of the 'compatibility' property: 1) Improved handling of unrecognized features: Change the way unrecognized features in compatibility files are handled. * invalid features in files under /usr/share/zfs/compatibility.d only get a warning (as these may refer to future features not yet in the library), * invalid features in files under /etc/zfs/compatibility.d get an error (as these are presumed to refer to the current system). 2) Improved error reporting from zpool_load_compat. Note: slight ABI change to zpool_load_compat for better error reporting. 3) compatibility=legacy inhibits all 'zpool upgrade' operations. 4) Detect when features are enabled outside current compatibility set * zpool set compatibility=foo <-- print a warning * zpool set feature@xxx=enabled <-- error * zpool status <-- indicate this state Reviewed-by: Brian Behlendorf Signed-off-by: Colm Buckley Closes #11861 --- cmd/zpool/zpool_main.c | 165 +- include/libzfs.h | 7 +- lib/libzfs/libzfs.abi | 5109 ++++++++++++++++++------------------ lib/libzfs/libzfs_pool.c | 242 +- lib/libzfs/libzfs_status.c | 28 +- man/man5/zpool-features.5 | 15 + man/man8/zpool-upgrade.8 | 16 +- 7 files changed, 2903 insertions(+), 2679 deletions(-) diff --git a/cmd/zpool/zpool_main.c b/cmd/zpool/zpool_main.c index 81bbf1375..34742eab5 100644 --- a/cmd/zpool/zpool_main.c +++ b/cmd/zpool/zpool_main.c @@ -786,7 +786,7 @@ add_prop_list(const char *propname, char *propval, nvlist_t **props, if (poolprop) { const char *vname = zpool_prop_to_name(ZPOOL_PROP_VERSION); - const char *fname = + const char *cname = zpool_prop_to_name(ZPOOL_PROP_COMPATIBILITY); if ((prop = zpool_name_to_prop(propname)) == ZPOOL_PROP_INVAL && @@ -811,16 +811,19 @@ add_prop_list(const char *propname, char *propval, nvlist_t **props, } /* - * compatibility property and version should not be specified - * at the same time. + * if version is specified, only "legacy" compatibility + * may be requested */ if ((prop == ZPOOL_PROP_COMPATIBILITY && + strcmp(propval, ZPOOL_COMPAT_LEGACY) != 0 && nvlist_exists(proplist, vname)) || (prop == ZPOOL_PROP_VERSION && - nvlist_exists(proplist, fname))) { - (void) fprintf(stderr, gettext("'compatibility' and " - "'version' properties cannot be specified " - "together\n")); + nvlist_exists(proplist, cname) && + strcmp(fnvlist_lookup_string(proplist, cname), + ZPOOL_COMPAT_LEGACY) != 0)) { + (void) fprintf(stderr, gettext("when 'version' is " + "specified, the 'compatibility' feature may only " + "be set to '" ZPOOL_COMPAT_LEGACY "'\n")); return (2); } @@ -1674,6 +1677,7 @@ zpool_do_create(int argc, char **argv) * - enable_pool_features (ie: no '-d' or '-o version') * - it's supported by the kernel module * - it's in the requested feature set + * - warn if it's enabled but not in compat */ for (spa_feature_t i = 0; i < SPA_FEATURES; i++) { char propname[MAXPATHLEN]; @@ -1687,6 +1691,14 @@ zpool_do_create(int argc, char **argv) if (strcmp(propval, ZFS_FEATURE_DISABLED) == 0) (void) nvlist_remove_all(props, propname); + if (strcmp(propval, + ZFS_FEATURE_ENABLED) == 0 && + !requested_features[i]) + (void) fprintf(stderr, gettext( + "Warning: feature \"%s\" enabled " + "but is not in specified " + "'compatibility' feature set.\n"), + feat->fi_uname); } else if ( enable_pool_features && feat->fi_zfs_mod_supported && @@ -2717,8 +2729,10 @@ show_import(nvlist_t *config, boolean_t report_error) case ZPOOL_STATUS_FEAT_DISABLED: printf_color(ANSI_BOLD, gettext("status: ")); - printf_color(ANSI_YELLOW, gettext("Some supported and " - "requested features are not enabled on the pool.\n")); + printf_color(ANSI_YELLOW, gettext("Some supported " + "features are not enabled on the pool.\n\t" + "(Note that they may be intentionally disabled " + "if the\n\t'compatibility' property is set.)\n")); break; case ZPOOL_STATUS_COMPATIBILITY_ERR: @@ -2728,6 +2742,13 @@ show_import(nvlist_t *config, boolean_t report_error) "property.\n")); break; + case ZPOOL_STATUS_INCOMPATIBLE_FEAT: + printf_color(ANSI_BOLD, gettext("status: ")); + printf_color(ANSI_YELLOW, gettext("One or more features " + "are enabled on the pool despite not being\n" + "requested by the 'compatibility' property.\n")); + break; + case ZPOOL_STATUS_UNSUP_FEAT_READ: printf_color(ANSI_BOLD, gettext("status: ")); printf_color(ANSI_YELLOW, gettext("The pool uses the following " @@ -8055,7 +8076,8 @@ status_callback(zpool_handle_t *zhp, void *data) (reason == ZPOOL_STATUS_OK || reason == ZPOOL_STATUS_VERSION_OLDER || reason == ZPOOL_STATUS_FEAT_DISABLED || - reason == ZPOOL_STATUS_COMPATIBILITY_ERR)) { + reason == ZPOOL_STATUS_COMPATIBILITY_ERR || + reason == ZPOOL_STATUS_INCOMPATIBLE_FEAT)) { if (!cbp->cb_allpools) { (void) printf(gettext("pool '%s' is healthy\n"), zpool_get_name(zhp)); @@ -8254,6 +8276,18 @@ status_callback(zpool_handle_t *zhp, void *data) ZPOOL_DATA_COMPAT_D ".\n")); break; + case ZPOOL_STATUS_INCOMPATIBLE_FEAT: + printf_color(ANSI_BOLD, gettext("status: ")); + printf_color(ANSI_YELLOW, gettext("One or more features " + "are enabled on the pool despite not being\n\t" + "requested by the 'compatibility' property.\n")); + printf_color(ANSI_BOLD, gettext("action: ")); + printf_color(ANSI_YELLOW, gettext("Consider setting " + "'compatibility' to an appropriate value, or\n\t" + "adding needed features to the relevant file in\n\t" + ZPOOL_SYSCONF_COMPAT_D " or " ZPOOL_DATA_COMPAT_D ".\n")); + break; + case ZPOOL_STATUS_UNSUP_FEAT_READ: printf_color(ANSI_BOLD, gettext("status: ")); printf_color(ANSI_YELLOW, gettext("The pool cannot be accessed " @@ -8713,6 +8747,11 @@ upgrade_version(zpool_handle_t *zhp, uint64_t version) verify(nvlist_lookup_uint64(config, ZPOOL_CONFIG_VERSION, &oldversion) == 0); + char compat[ZFS_MAXPROPLEN]; + if (zpool_get_prop(zhp, ZPOOL_PROP_COMPATIBILITY, compat, + ZFS_MAXPROPLEN, NULL, B_FALSE) != 0) + compat[0] = '\0'; + assert(SPA_VERSION_IS_SUPPORTED(oldversion)); assert(oldversion < version); @@ -8727,6 +8766,13 @@ upgrade_version(zpool_handle_t *zhp, uint64_t version) return (1); } + if (strcmp(compat, ZPOOL_COMPAT_LEGACY) == 0) { + (void) fprintf(stderr, gettext("Upgrade not performed because " + "'compatibility' property set to '" + ZPOOL_COMPAT_LEGACY "'.\n")); + return (1); + } + ret = zpool_upgrade(zhp, version); if (ret != 0) return (ret); @@ -8868,7 +8914,10 @@ upgrade_list_older_cb(zpool_handle_t *zhp, void *arg) "be upgraded to use feature flags. After " "being upgraded, these pools\nwill no " "longer be accessible by software that does not " - "support feature\nflags.\n\n")); + "support feature\nflags.\n\n" + "Note that setting a pool's 'compatibility' " + "feature to '" ZPOOL_COMPAT_LEGACY "' will\n" + "inhibit upgrades.\n\n")); (void) printf(gettext("VER POOL\n")); (void) printf(gettext("--- ------------\n")); cbp->cb_first = B_FALSE; @@ -8914,7 +8963,11 @@ upgrade_list_disabled_cb(zpool_handle_t *zhp, void *arg) "software\nthat does not support " "the feature. See " "zpool-features(5) for " - "details.\n\n")); + "details.\n\n" + "Note that the pool " + "'compatibility' feature can be " + "used to inhibit\nfeature " + "upgrades.\n\n")); (void) printf(gettext("POOL " "FEATURE\n")); (void) printf(gettext("------" @@ -9970,6 +10023,63 @@ set_callback(zpool_handle_t *zhp, void *data) int error; set_cbdata_t *cb = (set_cbdata_t *)data; + /* Check if we have out-of-bounds features */ + if (strcmp(cb->cb_propname, ZPOOL_CONFIG_COMPATIBILITY) == 0) { + boolean_t features[SPA_FEATURES]; + if (zpool_do_load_compat(cb->cb_value, features) != + ZPOOL_COMPATIBILITY_OK) + return (-1); + + nvlist_t *enabled = zpool_get_features(zhp); + spa_feature_t i; + for (i = 0; i < SPA_FEATURES; i++) { + const char *fguid = spa_feature_table[i].fi_guid; + if (nvlist_exists(enabled, fguid) && !features[i]) + break; + } + if (i < SPA_FEATURES) + (void) fprintf(stderr, gettext("Warning: one or " + "more features already enabled on pool '%s'\n" + "are not present in this compatibility set.\n"), + zpool_get_name(zhp)); + } + + /* if we're setting a feature, check it's in compatibility set */ + if (zpool_prop_feature(cb->cb_propname) && + strcmp(cb->cb_value, ZFS_FEATURE_ENABLED) == 0) { + char *fname = strchr(cb->cb_propname, '@') + 1; + spa_feature_t f; + + if (zfeature_lookup_name(fname, &f) == 0) { + char compat[ZFS_MAXPROPLEN]; + if (zpool_get_prop(zhp, ZPOOL_PROP_COMPATIBILITY, + compat, ZFS_MAXPROPLEN, NULL, B_FALSE) != 0) + compat[0] = '\0'; + + boolean_t features[SPA_FEATURES]; + if (zpool_do_load_compat(compat, features) != + ZPOOL_COMPATIBILITY_OK) { + (void) fprintf(stderr, gettext("Error: " + "cannot enable feature '%s' on pool '%s'\n" + "because the pool's 'compatibility' " + "property cannot be parsed.\n"), + fname, zpool_get_name(zhp)); + return (-1); + } + + if (!features[f]) { + (void) fprintf(stderr, gettext("Error: " + "cannot enable feature '%s' on pool '%s'\n" + "as it is not specified in this pool's " + "current compatibility set.\n" + "Consider setting 'compatibility' to a " + "less restrictive set, or to 'off'.\n"), + fname, zpool_get_name(zhp)); + return (-1); + } + } + } + error = zpool_set_prop(zhp, cb->cb_propname, cb->cb_value); if (!error) @@ -10492,28 +10602,25 @@ zpool_do_version(int argc, char **argv) static zpool_compat_status_t zpool_do_load_compat(const char *compat, boolean_t *list) { - char badword[ZFS_MAXPROPLEN]; - char badfile[MAXPATHLEN]; + char report[1024]; + zpool_compat_status_t ret; - switch (ret = zpool_load_compat(compat, list, badword, badfile)) { + ret = zpool_load_compat(compat, list, report, 1024); + switch (ret) { + case ZPOOL_COMPATIBILITY_OK: break; - case ZPOOL_COMPATIBILITY_READERR: - (void) fprintf(stderr, gettext("error reading compatibility " - "file '%s'\n"), badfile); - break; - case ZPOOL_COMPATIBILITY_BADFILE: - (void) fprintf(stderr, gettext("compatibility file '%s' " - "too large or not newline-terminated\n"), badfile); - break; - case ZPOOL_COMPATIBILITY_BADWORD: - (void) fprintf(stderr, gettext("unknown feature '%s' in " - "compatibility file '%s'\n"), badword, badfile); - break; + case ZPOOL_COMPATIBILITY_NOFILES: - (void) fprintf(stderr, gettext("no compatibility files " - "specified\n")); + case ZPOOL_COMPATIBILITY_BADFILE: + case ZPOOL_COMPATIBILITY_BADTOKEN: + (void) fprintf(stderr, "Error: %s\n", report); + break; + + case ZPOOL_COMPATIBILITY_WARNTOKEN: + (void) fprintf(stderr, "Warning: %s\n", report); + ret = ZPOOL_COMPATIBILITY_OK; break; } return (ret); diff --git a/include/libzfs.h b/include/libzfs.h index e8e771382..eeb4daae7 100644 --- a/include/libzfs.h +++ b/include/libzfs.h @@ -393,6 +393,7 @@ typedef enum { ZPOOL_STATUS_REBUILD_SCRUB, /* recommend scrubbing the pool */ ZPOOL_STATUS_NON_NATIVE_ASHIFT, /* (e.g. 512e dev with ashift of 9) */ ZPOOL_STATUS_COMPATIBILITY_ERR, /* bad 'compatibility' property */ + ZPOOL_STATUS_INCOMPATIBLE_FEAT, /* feature set outside compatibility */ /* * Finally, the following indicates a healthy pool. @@ -922,14 +923,14 @@ extern int zpool_disable_datasets(zpool_handle_t *, boolean_t); */ typedef enum { ZPOOL_COMPATIBILITY_OK, - ZPOOL_COMPATIBILITY_READERR, + ZPOOL_COMPATIBILITY_WARNTOKEN, + ZPOOL_COMPATIBILITY_BADTOKEN, ZPOOL_COMPATIBILITY_BADFILE, - ZPOOL_COMPATIBILITY_BADWORD, ZPOOL_COMPATIBILITY_NOFILES } zpool_compat_status_t; extern zpool_compat_status_t zpool_load_compat(const char *, - boolean_t *, char *, char *); + boolean_t *, char *, size_t); #ifdef __FreeBSD__ diff --git a/lib/libzfs/libzfs.abi b/lib/libzfs/libzfs.abi index aa69a4bf6..572297123 100644 --- a/lib/libzfs/libzfs.abi +++ b/lib/libzfs/libzfs.abi @@ -421,40 +421,40 @@ - - + + - + - + - + - + - + - + - + - + - + - + - + @@ -736,67 +736,62 @@ - - - - + - + - + - + - + - + - - - - + + + + - - + + - + - - + + - - - - - - - - - - + + + + + + + + - - + + - - - + + + @@ -810,7 +805,7 @@ - + @@ -822,193 +817,193 @@ - + - - - - + + + + - - + + - + - + - + - + - + - - - - - - - - - - - - + + + + + + + + + + + + - - + + - + - + - + - + - + - + - + - + - + - + - + - + - + - - + + - + - + - - + + - - + + - - + + - + - + - + - + - + - + - + - + - + - - + + - - - - - + + + + + - + - + - + - + - + - + - + - + - + - + - + - + - + - - - - - - - - + + + + + + + + @@ -1016,8 +1011,8 @@ - - + + @@ -1028,22 +1023,22 @@ - + - + - + - + - + @@ -1053,589 +1048,583 @@ - - - - - - - - - - - - + + + + + + + + + + + + - - - + + + - - - - - - + + + + + + - - + + - + - - - - - + + + + + - - - - - + + + + + - - - + + + - - + + - - + + - + - + - + - - + + - - + + - - + + - - + + - - + + - - + + - + - + - - + + - - + + - - + + - - + + - - + + - + - + - - - - - + + - + - + - + - + - + - + - + - + - + - + - + - + - - + + - + - - + + - - + + - - + + - - + + - - + + - - + + - - + + - + - - + + - - + + - - + + - - - + + + - - - - - - - + + + + + + + - - - - - - + + + + + + - - + + - - - - + + + + - - + + - - - - + + + + - - - + + + - + - + - + - + - - + + - - - - - + + - + + + + - + - + - + - - + + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - - - + + + - + - + - - - - - + + + + + - - + + - - - - + + + + - - - + + + - - - - - + + + + + - - - - - - - - - - - + + + + + + + + + + + - - - - + + + + - - - - + - + - + - - + + - - + + + + + - + - - + + - + - + - + - + - + - + - - + + - - + + - - + + + + + - - - - + - + - - + + - + - - - - - + + - + - + - - + + - + - + - + + + + - + - + - + - - + + - - - - + - + - - + + + + + - + - + - - + + - + - + - - + + - - + + - - - - + - - + + - - - - - - + + + + + + - - - - + + + + - - - + + + - - - - + + + + - - - + + + - - - - - + + + + + - - - - + + + + - - - - - - + + + + + + - + @@ -1651,229 +1640,229 @@ - - - - - - - - - - + + + + + + + + + + - - - - - - + + + + + + - - - - + + + + - - - - - + + + + + - - - - - + + + + + - - - - + + + + - + - + - + - + - + - + - + - + + + - - - - - - - - + + + + + + - - + + - - + + - - + + - + - + - + - + - - - - - + + + + + - - - - + + + + - - - - - + + + + + - - - - + + + + - - + + - - - - + + + + - - - - + + + + - - - - + + + + - - - + + + - - - - - + + + + + - - - + + + - - - - + + + + - - - - + + + + - - - - + + + + - - - + + + - - - + + + - - - - - - + + + + + + - - - - - + + + + + - - - - - - + + + + + + - - - - + + + + - + @@ -1882,120 +1871,131 @@ - - - - - - - - - + + + + + + + + + - - - + + + - - + + + + + + + + + + + + + - - - - - - + + + + + + - - - - + + + + - - - + + + - - - - + + + + - - - - - - - - - + + + + + + + + + - - - - + + + + - - - - + + + + - - - - - - + + + + + + - - - - + + + + - - - + + + - - - + + + - - - + + + - - - - - + + + + + - - - - - + + + + + - - + + - - - + + + - + - + @@ -2019,7 +2019,7 @@ - + @@ -2055,22 +2055,22 @@ - + - + - + - + - + @@ -2079,10 +2079,10 @@ - + - + @@ -2094,23 +2094,23 @@ - + - - + + - - + + - + @@ -2124,8 +2124,8 @@ - - + + @@ -2136,10 +2136,10 @@ - + - + @@ -2151,7 +2151,7 @@ - + @@ -2168,28 +2168,28 @@ - + - + - + - + - + - + @@ -2198,19 +2198,19 @@ - + - + - + - - + + - + @@ -2221,57 +2221,57 @@ - + - - + + - - - - - - - + + + + + + + - - - - + + + + - - - - + + + + - - - + + + - - - + + + - - - + + + - - - - - + + + + + - - - + + + - + @@ -2285,454 +2285,448 @@ - - - - - + + + + + - - - - - + + + + + - - - - + + + + - + - + - + - - + + - + - + - + - + - + - - + + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - - + + - - + + - + - + - - + + - + - + - + - + - - + + - + - + - + - + - - + + - + - - + + - + - - + + - - + + - - + + - - + + - - + + - + - + - + - + - + - + - - - - + - + - + - + - + - - + + - + - - + + - + - + - + - - + + - + - + - + - + - + - + - + - - - - + - + + + + - + - + - - + + - - + + - + - + - + - + - + - + - + - - + + - + - + - - + + - + - - - - - - - + - + - + - - + + - - + + - + - + - + - + - + - + - + - + - + - - + + - + - - + + - + - + + + + + + + - + - + - - - - + + + + - - - - - - - + + + + + + + - - - - + - + - + - + - - + + - - + + - - + + - + - + - - + + - - + + - + - + - - - - - - - - + + + + + + + + @@ -2743,137 +2737,137 @@ - - - - - - - - + + + + + + + + - - + + - + - + - - + + - + - + - + - - - + + + - + - - + + - - - - - + + + + + - - - - - + + + + + - - - - + + + + - - - - - + + + + + - - - - - - + + + + + + - - - - + + + + - - - - - - - + + + + + + + - - - - + + + + - - + + - - + + - + - + - + - + - - + + - + - + - - + + @@ -2887,335 +2881,335 @@ - + - - + + - + - - + + - - + + - - + + - - - - + + + + - - - - + + + + - - - - - - - - - + + + + + + + + + - + - + - + - + - - - - - - + + + + + + - - - + + + - - - - + + + + - - - + + + - - + + - - + + - - + + - - - + + + - - - + + + - - - - + + + + - - + + - - + + - - - + + + - - - + + + - - - + + + - - - + + + - - - + + + - + - - - - - - + + + + + + - - + + - - + + - - - + + + - - - - - + + + + + - - + + - - - + + + - - - - + + + + - - - - - + + + + + - - - - + + + + - - - + + + - - - - + + + + - - - - - - + + + + + + - - - - + + + + - + - - + + - + - + - + - + - + - + - + - + - + - + - + - + - + - - + + - - + + - + - - + + - + - - + + - + - + - + - + - + - + - + - + - - + + - - + + - + - - - - + + + + - - - - - + + + + + - + @@ -3227,131 +3221,131 @@ - - - - - - + + + + + + - - - + + + - - - - - - - + + + + + + + - - - - - - - + + + + + + + - - - - + + + + - - - + + + - - - - - - + + + + + + - - - - - + + + + + - - - + + + - - - - - - + + + + + + - - - + + + - - - + + + - - - - - + + + + + - - - + + + - - - + + + - - + + - - - + + + - - - - + + + + - - - - + + + + - - + + - - - + + + - + @@ -3362,30 +3356,30 @@ - - - - - - - + + + + + + + - - - + + + - - - - - - - + + + + + + + - + @@ -3409,26 +3403,26 @@ - - - - - + + + + + - - - - + + + + - - - - + + + + - + @@ -3439,72 +3433,72 @@ - - - - - - - + + + + + + + - - - + + + - - - - + + + + - - - - - - + + + + + + - - - - - - + + + + + + - + - - + + - - - - - + + + + + - + - - + + @@ -3518,166 +3512,157 @@ - - - - - - - + + + + + + + - + - - - - - + + + + + - - - - + + + + - - - - - - + + + + + + - - - + + + - - - - - + + + + + - - - - - - + + + + + + - - - + + + - - - - + + + + - - - + + + - - + + - - + + - - - + + + - - - - - - + + + + + + - - + + - - + + - - - + + + - - - + + + - - - + + + - - - - - + + + + + - - - + + + - - - - + + + + - - - - - + + + + + - - - - + + + + - - - - + + + + - - - - - - - - - - + @@ -3715,463 +3700,481 @@ - - - - - + + + + + + + + + + + + + + + + + + + + + + + - - + + - - - + + + - + - - - + + + - - - - - - - + + + + + + + - + - + - + - + - + - + - + - - + + - - + + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - - + + - + - + - + - - + + - + - + - + - + - + - + - - + + - - + + - - - - - - - + - + - - + + + + + + + + - + - - + + - + - + - + - + - + - + - + - + - + - + - + - + - + - - - - - - - - - - + + + + + + + + + + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - - - - - - - - + + + + + + + + - - - - - - - - - - - + + + + + + + + + + + - - - - - + + + + + - - - - - + + + + + - - - + + + - - - - - + + + + + - + - - + + - - + + - + - + - - + + - + - + - + - + - + - + - + - - + + - + - - - - + - + - - + + - - + + - + + + + - + - + - - + + - + - + - + - + - - + + - - + + - + - + - + - - + + - + - - - + + + - - + + @@ -4204,10 +4207,11 @@ - + + - - + + @@ -4215,99 +4219,171 @@ - - - - - - - + + + + + + + - - - - - + + + + + - - + + - - - - + + + + - + + + + + + + + - - - - + + + + - - - - - - - - + + + + + + + + - - - - + + + + - - - + + + - - - - - + + + + + - - - - - - - + - + - + - - + + - - + + - - - - - - - - - - - + + - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -4316,468 +4392,461 @@ - + - - + + - - + + - - - - - - - - - - - + + + + + + + + + + + - - - - + + + + - - - - + + + + - - - - + + + + - - - + + + - - - + + + - - - - + + + + - - - - - + + + + + - - + + - - + + - - + + - - - + + + - + - - + + - - - - + + + + - - - - - - - + + + + + + + - - - - - - + + + + + + - - - - + + + + - - - - + + + + - - - + + + - - - - - - + + + + + + - - - + + + - - - - + + + + - - + + - - - - + + + + - - - - + + + + - - - - - - + + + + + + - - - - + + + + - - - - + + + + - - - - + + + + - - - - + + + + - - - + + + - + - - - + + + - - + + - - - + + + - - - + + - - - - - - - - + + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - - + + - - + + - + - + - + - + - + - + - + - + - - + + - - + + - - + + - + - + - + - + - - + + - - + + - + - + - - + + - + - + - - + + - - + + - + - + - + - - + + - + - + - + - + - + - + - + - + - + - + - + - + - - + + - + - + - + - + @@ -4831,10 +4900,10 @@ - - + + - + @@ -4842,140 +4911,140 @@ - + - + - - + + - + - + - + - - + + - + - + - + - + - + - + - + - - + + - - + + - + - + - + - - + + - + - + - + - + - - + + - - + + - - + + - - + + - - - + + + - - - - + + + + - - + + - - + + - - + + - - + + - - + + - - + + - - + + - + - + @@ -4984,35 +5053,35 @@ - + - + - + - + - + - + - + - + - + - + @@ -5051,70 +5120,70 @@ - - + + - - + + - - - - + + + + - - + + - + - - + + - + - - + + - - + + - + - + - - + + - + - - + + - + - + @@ -5125,9 +5194,9 @@ - + - + @@ -5139,12 +5208,12 @@ - - + + - - + + @@ -5156,19 +5225,19 @@ - + - - + + - + - + @@ -5203,15 +5272,15 @@ - - + + - - + + - - + + @@ -5225,303 +5294,303 @@ - + - - - - + + + + - - + + - + - + - - + + - + - + - + - + - + - + - + - + - + - - + + - + - + - + - + - + - + - + - - + + - - + + - + - + - - + + - + - + - + - - + + - + - + - + - - + + - + - + - + - - + + - - - - - - - - - - - - - - + + + + + + + + + + + + + + - - - + + + - + - - + + - + - + - - - + + + - - - + + + - + - - - + + + - - - + + + - - + + - + - - - + + + - + - - - + + + - - - + + + - + - + - - - + + + - - - + + + - - + + - + - + - + - + - + - + - + - + - - - - - - - - - - - - + + + + + + + + + + + + - - - + + + - - - + + + - + - - - - + + + + - - + + - - - + + + - - + + - - + + - + - + @@ -5537,78 +5606,78 @@ - - + + - - + + - - + + - - + + - - + + - - + + - + - - + + - + - - + + - - + + - + - - + + - + - + @@ -5616,10 +5685,10 @@ - + - + @@ -5633,7 +5702,7 @@ - + @@ -5641,7 +5710,7 @@ - + @@ -5655,16 +5724,16 @@ - + - + - + @@ -5677,66 +5746,66 @@ - + - + - - + + - + - + - + - + - + - + - + - + - + - + - + @@ -5748,13 +5817,13 @@ - + - + - + @@ -5762,121 +5831,121 @@ - - + + - + - - - - - + + + + + - + - + - + - + - + - + - + - + - + - + - - + + - - + + - + - + - + - - - + + + - + - + - + - - + + - + - + - - + + - - + + - - + + - + - + - + - + @@ -5887,7 +5956,7 @@ - + @@ -5898,25 +5967,25 @@ - + - - + + - + - - + + @@ -5924,260 +5993,260 @@ - - - + + + - - + + - + - + - - - - + + + + - + - + - - - + + + - - - + + + - - - + + + - - + + - - + + - - + + - - + + - + - + - - - - + + + + - - + + - - - + + + - - - + + + - - - + + + - - - - - + + + + + - + - + - + - + - + - - + + - + - + - + - + - + - + - + - + - + - + - + - + - + - - - - - - - - - + + + + - - - - - - - - - + + + + + + + + + + + + + + - - + + - - + + + + + + + + + + - - + + + - - - - - - - - - - - + + - - - + + + - + - + - + - - + + - - + + - + - + - - + + - + - - + + - + - + - + - + - - + + - - - - - - - + + + + - + + + + diff --git a/lib/libzfs/libzfs_pool.c b/lib/libzfs/libzfs_pool.c index 12de6887d..b4c9d7df9 100644 --- a/lib/libzfs/libzfs_pool.c +++ b/lib/libzfs/libzfs_pool.c @@ -467,8 +467,7 @@ zpool_valid_proplist(libzfs_handle_t *hdl, const char *poolname, char *slash, *check; struct stat64 statbuf; zpool_handle_t *zhp; - char badword[ZFS_MAXPROPLEN]; - char badfile[MAXPATHLEN]; + char report[1024]; if (nvlist_alloc(&retprops, NV_UNIQUE_NAME, 0) != 0) { (void) no_memory(hdl); @@ -679,33 +678,14 @@ zpool_valid_proplist(libzfs_handle_t *hdl, const char *poolname, break; case ZPOOL_PROP_COMPATIBILITY: - switch (zpool_load_compat(strval, NULL, - badword, badfile)) { + switch (zpool_load_compat(strval, NULL, report, 1024)) { case ZPOOL_COMPATIBILITY_OK: + case ZPOOL_COMPATIBILITY_WARNTOKEN: break; - case ZPOOL_COMPATIBILITY_READERR: - zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, - "error reading feature file '%s'"), - badfile); - (void) zfs_error(hdl, EZFS_BADPROP, errbuf); - goto error; case ZPOOL_COMPATIBILITY_BADFILE: - zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, - "feature file '%s' too large or not " - "newline-terminated"), - badfile); - (void) zfs_error(hdl, EZFS_BADPROP, errbuf); - goto error; - case ZPOOL_COMPATIBILITY_BADWORD: - zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, - "unknown feature '%s' in feature " - "file '%s'"), - badword, badfile); - (void) zfs_error(hdl, EZFS_BADPROP, errbuf); - goto error; + case ZPOOL_COMPATIBILITY_BADTOKEN: case ZPOOL_COMPATIBILITY_NOFILES: - zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, - "no feature files specified")); + zfs_error_aux(hdl, report); (void) zfs_error(hdl, EZFS_BADPROP, errbuf); goto error; } @@ -4742,8 +4722,8 @@ zpool_get_bootenv(zpool_handle_t *zhp, nvlist_t **nvlp) * Arguments: * compatibility : string containing feature filenames * features : either NULL or pointer to array of boolean - * badtoken : either NULL or pointer to char[ZFS_MAXPROPLEN] - * badfile : either NULL or pointer to char[MAXPATHLEN] + * report : either NULL or pointer to string buffer + * rlen : length of "report" buffer * * compatibility is NULL (unset), "", "off", "legacy", or list of * comma-separated filenames. filenames should either be absolute, @@ -4752,48 +4732,56 @@ zpool_get_bootenv(zpool_handle_t *zhp, nvlist_t **nvlp) * 2) ZPOOL_DATA_COMPAT_D (eg: /usr/share/zfs/compatibility.d). * (Unset), "" or "off" => enable all features * "legacy" => disable all features + * * Any feature names read from files which match unames in spa_feature_table * will have the corresponding boolean set in the features array (if non-NULL). * If more than one feature set specified, only features present in *all* of * them will be set. * - * An unreadable filename will be strlcpy'd to badfile (if non-NULL). - * An unrecognized feature will be strlcpy'd to badtoken (if non-NULL). + * "report" if not NULL will be populated with a suitable status message. * * Return values: * ZPOOL_COMPATIBILITY_OK : files read and parsed ok - * ZPOOL_COMPATIBILITY_READERR : file could not be opened / mmap'd * ZPOOL_COMPATIBILITY_BADFILE : file too big or not a text file - * ZPOOL_COMPATIBILITY_BADWORD : file contains invalid feature name - * ZPOOL_COMPATIBILITY_NOFILES : no file names found + * ZPOOL_COMPATIBILITY_BADTOKEN : SYSCONF file contains invalid feature name + * ZPOOL_COMPATIBILITY_WARNTOKEN : DATA file contains invalid feature name + * ZPOOL_COMPATIBILITY_NOFILES : no feature files found */ zpool_compat_status_t -zpool_load_compat(const char *compatibility, - boolean_t *features, char *badtoken, char *badfile) +zpool_load_compat(const char *compat, boolean_t *features, char *report, + size_t rlen) { int sdirfd, ddirfd, featfd; - int i; struct stat fs; - char *fc; /* mmap of file */ - char *ps, *ls, *ws; /* strtok state */ + char *fc; + char *ps, *ls, *ws; char *file, *line, *word; - char filenames[ZFS_MAXPROPLEN]; - int filecount = 0; + + char l_compat[ZFS_MAXPROPLEN]; + + boolean_t ret_nofiles = B_TRUE; + boolean_t ret_badfile = B_FALSE; + boolean_t ret_badtoken = B_FALSE; + boolean_t ret_warntoken = B_FALSE; /* special cases (unset), "" and "off" => enable all features */ - if (compatibility == NULL || compatibility[0] == '\0' || - strcmp(compatibility, ZPOOL_COMPAT_OFF) == 0) { + if (compat == NULL || compat[0] == '\0' || + strcmp(compat, ZPOOL_COMPAT_OFF) == 0) { if (features != NULL) - for (i = 0; i < SPA_FEATURES; i++) + for (uint_t i = 0; i < SPA_FEATURES; i++) features[i] = B_TRUE; + if (report != NULL) + strlcpy(report, gettext("all features enabled"), rlen); return (ZPOOL_COMPATIBILITY_OK); } /* Final special case "legacy" => disable all features */ - if (strcmp(compatibility, ZPOOL_COMPAT_LEGACY) == 0) { + if (strcmp(compat, ZPOOL_COMPAT_LEGACY) == 0) { if (features != NULL) - for (i = 0; i < SPA_FEATURES; i++) + for (uint_t i = 0; i < SPA_FEATURES; i++) features[i] = B_FALSE; + if (report != NULL) + strlcpy(report, gettext("all features disabled"), rlen); return (ZPOOL_COMPATIBILITY_OK); } @@ -4801,9 +4789,12 @@ zpool_load_compat(const char *compatibility, * Start with all true; will be ANDed with results from each file */ if (features != NULL) - for (i = 0; i < SPA_FEATURES; i++) + for (uint_t i = 0; i < SPA_FEATURES; i++) features[i] = B_TRUE; + char err_badfile[1024] = ""; + char err_badtoken[1024] = ""; + /* * We ignore errors from the directory open() * as they're only needed if the filename is relative @@ -4815,32 +4806,33 @@ zpool_load_compat(const char *compatibility, sdirfd = open(ZPOOL_SYSCONF_COMPAT_D, O_DIRECTORY | O_PATH | O_CLOEXEC); ddirfd = open(ZPOOL_DATA_COMPAT_D, O_DIRECTORY | O_PATH | O_CLOEXEC); - (void) strlcpy(filenames, compatibility, ZFS_MAXPROPLEN); - file = strtok_r(filenames, ",", &ps); - while (file != NULL) { - boolean_t features_local[SPA_FEATURES]; + (void) strlcpy(l_compat, compat, ZFS_MAXPROPLEN); + + for (file = strtok_r(l_compat, ",", &ps); + file != NULL; + file = strtok_r(NULL, ",", &ps)) { + + boolean_t l_features[SPA_FEATURES]; + + enum { Z_SYSCONF, Z_DATA } source; /* try sysconfdir first, then datadir */ - if ((featfd = openat(sdirfd, file, 0, O_RDONLY)) < 0) + source = Z_SYSCONF; + if ((featfd = openat(sdirfd, file, 0, O_RDONLY)) < 0) { featfd = openat(ddirfd, file, 0, O_RDONLY); - - if (featfd < 0 || fstat(featfd, &fs) < 0) { - (void) close(featfd); - (void) close(sdirfd); - (void) close(ddirfd); - if (badfile != NULL) - (void) strlcpy(badfile, file, MAXPATHLEN); - return (ZPOOL_COMPATIBILITY_READERR); + source = Z_DATA; } - /* Too big or too small */ - if (fs.st_size < 1 || fs.st_size > ZPOOL_COMPAT_MAXSIZE) { + /* File readable and correct size? */ + if (featfd < 0 || + fstat(featfd, &fs) < 0 || + fs.st_size < 1 || + fs.st_size > ZPOOL_COMPAT_MAXSIZE) { (void) close(featfd); - (void) close(sdirfd); - (void) close(ddirfd); - if (badfile != NULL) - (void) strlcpy(badfile, file, MAXPATHLEN); - return (ZPOOL_COMPATIBILITY_BADFILE); + strlcat(err_badfile, file, ZFS_MAXPROPLEN); + strlcat(err_badfile, " ", ZFS_MAXPROPLEN); + ret_badfile = B_TRUE; + continue; } /* private mmap() so we can strtok safely */ @@ -4848,73 +4840,99 @@ zpool_load_compat(const char *compatibility, PROT_READ|PROT_WRITE, MAP_PRIVATE, featfd, 0); (void) close(featfd); - if (fc < 0) { - (void) close(sdirfd); - (void) close(ddirfd); - if (badfile != NULL) - (void) strlcpy(badfile, file, MAXPATHLEN); - return (ZPOOL_COMPATIBILITY_READERR); - } - - /* Text file sanity check - last char should be newline */ - if (fc[fs.st_size - 1] != '\n') { + /* map ok, and last character == newline? */ + if (fc < 0 || fc[fs.st_size - 1] != '\n') { (void) munmap((void *) fc, fs.st_size); - (void) close(sdirfd); - (void) close(ddirfd); - if (badfile != NULL) - (void) strlcpy(badfile, file, MAXPATHLEN); - return (ZPOOL_COMPATIBILITY_BADFILE); + strlcat(err_badfile, file, ZFS_MAXPROPLEN); + strlcat(err_badfile, " ", ZFS_MAXPROPLEN); + ret_badfile = B_TRUE; + continue; } - /* replace with NUL to ensure we have a delimiter */ + ret_nofiles = B_FALSE; + + for (uint_t i = 0; i < SPA_FEATURES; i++) + l_features[i] = B_FALSE; + + /* replace last char with NUL to ensure we have a delimiter */ fc[fs.st_size - 1] = '\0'; - for (i = 0; i < SPA_FEATURES; i++) - features_local[i] = B_FALSE; - - line = strtok_r(fc, "\n", &ls); - while (line != NULL) { + for (line = strtok_r(fc, "\n", &ls); + line != NULL; + line = strtok_r(NULL, "\n", &ls)) { /* discard comments */ *(strchrnul(line, '#')) = '\0'; - word = strtok_r(line, ", \t", &ws); - while (word != NULL) { + for (word = strtok_r(line, ", \t", &ws); + word != NULL; + word = strtok_r(NULL, ", \t", &ws)) { /* Find matching feature name */ - for (i = 0; i < SPA_FEATURES; i++) { + uint_t f; + for (f = 0; f < SPA_FEATURES; f++) { zfeature_info_t *fi = - &spa_feature_table[i]; + &spa_feature_table[f]; if (strcmp(word, fi->fi_uname) == 0) { - features_local[i] = B_TRUE; + l_features[f] = B_TRUE; break; } } - if (i == SPA_FEATURES) { - if (badtoken != NULL) - (void) strlcpy(badtoken, word, - ZFS_MAXPROPLEN); - if (badfile != NULL) - (void) strlcpy(badfile, file, - MAXPATHLEN); - (void) munmap((void *) fc, fs.st_size); - (void) close(sdirfd); - (void) close(ddirfd); - return (ZPOOL_COMPATIBILITY_BADWORD); - } - word = strtok_r(NULL, ", \t", &ws); + if (f < SPA_FEATURES) + continue; + + /* found an unrecognized word */ + /* lightly sanitize it */ + if (strlen(word) > 32) + word[32] = '\0'; + for (char *c = word; *c != '\0'; c++) + if (!isprint(*c)) + *c = '?'; + + strlcat(err_badtoken, word, ZFS_MAXPROPLEN); + strlcat(err_badtoken, " ", ZFS_MAXPROPLEN); + if (source == Z_SYSCONF) + ret_badtoken = B_TRUE; + else + ret_warntoken = B_TRUE; } - line = strtok_r(NULL, "\n", &ls); } (void) munmap((void *) fc, fs.st_size); - if (features != NULL) { - for (i = 0; i < SPA_FEATURES; i++) - features[i] &= features_local[i]; - } - filecount++; - file = strtok_r(NULL, ",", &ps); + + if (features != NULL) + for (uint_t i = 0; i < SPA_FEATURES; i++) + features[i] &= l_features[i]; } (void) close(sdirfd); (void) close(ddirfd); - if (filecount == 0) + + /* Return the most serious error */ + if (ret_badfile) { + if (report != NULL) + snprintf(report, rlen, gettext("could not read/" + "parse feature file(s): %s"), err_badfile); + return (ZPOOL_COMPATIBILITY_BADFILE); + } + if (ret_nofiles) { + if (report != NULL) + strlcpy(report, + gettext("no valid compatibility files specified"), + rlen); return (ZPOOL_COMPATIBILITY_NOFILES); + } + if (ret_badtoken) { + if (report != NULL) + snprintf(report, rlen, gettext("invalid feature " + "name(s) in local compatibility files: %s"), + err_badtoken); + return (ZPOOL_COMPATIBILITY_BADTOKEN); + } + if (ret_warntoken) { + if (report != NULL) + snprintf(report, rlen, gettext("unrecognized feature " + "name(s) in distribution compatibility files: %s"), + err_badtoken); + return (ZPOOL_COMPATIBILITY_WARNTOKEN); + } + if (report != NULL) + strlcpy(report, gettext("compatibility set ok"), rlen); return (ZPOOL_COMPATIBILITY_OK); } diff --git a/lib/libzfs/libzfs_status.c b/lib/libzfs/libzfs_status.c index 5e5cb5f5d..33d6e1bfd 100644 --- a/lib/libzfs/libzfs_status.c +++ b/lib/libzfs/libzfs_status.c @@ -89,6 +89,7 @@ static char *zfs_msgid_table[] = { * ZPOOL_STATUS_REBUILDING * ZPOOL_STATUS_REBUILD_SCRUB * ZPOOL_STATUS_COMPATIBILITY_ERR + * ZPOOL_STATUS_INCOMPATIBLE_FEAT * ZPOOL_STATUS_OK */ }; @@ -453,11 +454,17 @@ check_status(nvlist_t *config, boolean_t isimport, /* * Outdated, but usable, version */ - if (SPA_VERSION_IS_SUPPORTED(version) && version != SPA_VERSION) - return (ZPOOL_STATUS_VERSION_OLDER); + if (SPA_VERSION_IS_SUPPORTED(version) && version != SPA_VERSION) { + /* "legacy" compatibility disables old version reporting */ + if (compat != NULL && strcmp(compat, ZPOOL_COMPAT_LEGACY) == 0) + return (ZPOOL_STATUS_OK); + else + return (ZPOOL_STATUS_VERSION_OLDER); + } /* - * Usable pool with disabled features + * Usable pool with disabled or superfluous features + * (superfluous = beyond what's requested by 'compatibility') */ if (version >= SPA_VERSION_FEATURES) { int i; @@ -475,18 +482,23 @@ check_status(nvlist_t *config, boolean_t isimport, } /* check against all features, or limited set? */ - boolean_t pool_features[SPA_FEATURES]; + boolean_t c_features[SPA_FEATURES]; - if (zpool_load_compat(compat, pool_features, NULL, NULL) != - ZPOOL_COMPATIBILITY_OK) + switch (zpool_load_compat(compat, c_features, NULL, 0)) { + case ZPOOL_COMPATIBILITY_OK: + case ZPOOL_COMPATIBILITY_WARNTOKEN: + break; + default: return (ZPOOL_STATUS_COMPATIBILITY_ERR); + } for (i = 0; i < SPA_FEATURES; i++) { zfeature_info_t *fi = &spa_feature_table[i]; if (!fi->fi_zfs_mod_supported) continue; - if (pool_features[i] && - !nvlist_exists(feat, fi->fi_guid)) + if (c_features[i] && !nvlist_exists(feat, fi->fi_guid)) return (ZPOOL_STATUS_FEAT_DISABLED); + if (!c_features[i] && nvlist_exists(feat, fi->fi_guid)) + return (ZPOOL_STATUS_INCOMPATIBLE_FEAT); } } diff --git a/man/man5/zpool-features.5 b/man/man5/zpool-features.5 index 20e1dfde7..c97870dbb 100644 --- a/man/man5/zpool-features.5 +++ b/man/man5/zpool-features.5 @@ -166,6 +166,12 @@ enabled when using \fBzpool upgrade\fR. \fBzpool status\fR will not show a warning about disabled features which are not part of the requested feature set. .LP +The special value \fBlegacy\fR prevents any features from being enabled, +either via \fBzpool upgrade\fR or via \fBzpool set feature@XX=enabled\fR. +This setting also prevents pools from being upgraded to newer on-disk +versions. This is a safety measure to prevent new features from being +accidentally enabled, breaking compatibility. +.LP By convention, compatibility files in \fB/usr/share/zfs/compatibility.d\fR are provided by the distribution package, and include feature sets supported by important versions of popular distributions, and feature @@ -173,6 +179,15 @@ sets commonly supported at the start of each year. Compatibility files in \fB/etc/zfs/compatibility.d\fR, if present, will take precedence over files with the same name in \fB/usr/share/zfs/compatibility.d\fR. .LP +If an unrecognized feature is found in these files, an error message will +be shown. If the unrecognized feature is in a file in +\fB/etc/zfs/compatibility.d\fR, this is treated as an error and processing +will stop. If the unrecognized feature is under +\fB/usr/share/zfs/compatibility.d\fR, this is treated as a warning and +processing will continue. This difference is to allow distributions to +include features which might not be recognized by the currently-installed +binaries. +.LP Compatibility files may include comments; any text from \fB#\fR to the end of the line is ignored. .LP diff --git a/man/man8/zpool-upgrade.8 b/man/man8/zpool-upgrade.8 index face5b138..15baf8a52 100644 --- a/man/man8/zpool-upgrade.8 +++ b/man/man8/zpool-upgrade.8 @@ -55,11 +55,9 @@ formatted using a legacy ZFS version number. These pools can continue to be used, but some features may not be available. Use .Nm zpool Cm upgrade Fl a -to enable all features on all pools. (If a pool has specified compatibility -feature sets using the +to enable all features on all pools (subject to the .Fl o Ar compatibility -property, only the features present in all requested compatibility sets will -be enabled on that pool.) +property). .It Xo .Nm zpool .Cm upgrade @@ -75,11 +73,15 @@ for a description of feature flags features supported by the current software. .Op Fl V Ar version .Fl a Ns | Ns Ar pool Ns ... .Xc -Enables all supported features on the given pool. (If the pool has specified -compatibility feature sets using the +Enables all supported features on the given pool. +.Pp +If the pool has specified compatibility feature sets using the .Fl o Ar compatibility property, only the features present in all requested compatibility sets will be -enabled.) +enabled. If this property is set to +.Ar legacy +then no upgrade will take place. +.Pp Once this is done, the pool will no longer be accessible on systems that do not support feature flags. See