From 5137c132a5e82b2e799ad3ee5d82fb32b500e5a4 Mon Sep 17 00:00:00 2001 From: Pawel Jakub Dawidek Date: Wed, 29 May 2024 13:34:59 -0700 Subject: [PATCH] zpool import output is not formated properly. The 'zpool status' output assumes that the longest prefix is six character long plus colon plus space, eg. 'status: ', 'action: ' or 'config: ' (so eight in total). This works well even when we have messages that requires more than one line, as '\t' is exactly eight characters, just like the longest prefix. The 'zpool import' output is a bit different, as it may display the comment pool property, then the longest prefix is 'comment: ', which is nine characters long, not eight. All the prefixes were given an extra space in front, but: - 'status: ' did not get an extra space. - Messages that require more than one line should use nine spaces of indentation, not eight. - The extra space in front looks redundant if there is no comment property set on the given pool. Fix it by adding an extra space to all prefixes, but only if the comment property is defined. Also, when we need to continue the message in a new line, use '\t ' for indentation. While here, apply small corrections to a couple messages. Before: pool: tank id: 7412636063178848859 state: ONLINE status: Some supported features are not enabled on the pool. (Note that they may be intentionally disabled if the 'compatibility' property is set.) action: The pool can be imported using its name or numeric identif[...] some features will not be available without an explicit 'zp[...] comment: Example comment. config: bclone ONLINE ada0 ONLINE After: pool: tank id: 10180960571062436759 state: ONLINE status: Some supported features are not enabled on the pool. (Note that they may be intentionally disabled if the 'compatibility' property is set.) action: The pool can be imported using its name or numeric identifi[...] some features will not be available without an explicit 'zp[...] config: tank ONLINE ada3 ONLINE pool: dozer id: 11028319538368222579 state: ONLINE status: Some supported features are not enabled on the pool. (Note that they may be intentionally disabled if the 'compatibility' property is set.) action: The pool can be imported using its name or numeric identif[...] some features will not be available without an explicit 'z[...] comment: Example comment. config: dozer ONLINE ada1 ONLINE Reviewed-by: Tony Hutter Reviewed-by: Brian Behlendorf Signed-off-by: Pawel Dawidek Closes #16128 --- cmd/zpool/zpool_main.c | 269 +++++++++++++++++++++-------------------- 1 file changed, 138 insertions(+), 131 deletions(-) diff --git a/cmd/zpool/zpool_main.c b/cmd/zpool/zpool_main.c index d47e1cda9c39..57170c8ae717 100644 --- a/cmd/zpool/zpool_main.c +++ b/cmd/zpool/zpool_main.c @@ -3016,6 +3016,7 @@ show_import(nvlist_t *config, boolean_t report_error) const char *health; uint_t vsc; const char *comment; + const char *indent; status_cbdata_t cb = { 0 }; verify(nvlist_lookup_string(config, ZPOOL_CONFIG_POOL_NAME, @@ -3040,82 +3041,84 @@ show_import(nvlist_t *config, boolean_t report_error) if (reason != ZPOOL_STATUS_OK && !report_error) return (reason); - (void) printf(gettext(" pool: %s\n"), name); - (void) printf(gettext(" id: %llu\n"), (u_longlong_t)guid); - (void) printf(gettext(" state: %s"), health); + if (nvlist_lookup_string(config, ZPOOL_CONFIG_COMMENT, &comment) == 0) { + indent = " "; + } else { + comment = NULL; + indent = ""; + } + + (void) printf(gettext("%s pool: %s\n"), indent, name); + (void) printf(gettext("%s id: %llu\n"), indent, (u_longlong_t)guid); + (void) printf(gettext("%s state: %s"), indent, health); if (pool_state == POOL_STATE_DESTROYED) (void) printf(gettext(" (DESTROYED)")); (void) printf("\n"); + if (reason != ZPOOL_STATUS_OK) { + (void) printf("%s", indent); + printf_color(ANSI_BOLD, gettext("status: ")); + } switch (reason) { case ZPOOL_STATUS_MISSING_DEV_R: case ZPOOL_STATUS_MISSING_DEV_NR: case ZPOOL_STATUS_BAD_GUID_SUM: - printf_color(ANSI_BOLD, gettext("status: ")); printf_color(ANSI_YELLOW, gettext("One or more devices are " "missing from the system.\n")); break; case ZPOOL_STATUS_CORRUPT_LABEL_R: case ZPOOL_STATUS_CORRUPT_LABEL_NR: - printf_color(ANSI_BOLD, gettext("status: ")); - printf_color(ANSI_YELLOW, gettext("One or more devices contains" - " corrupted data.\n")); + printf_color(ANSI_YELLOW, gettext("One or more devices " + "contains corrupted data.\n")); break; case ZPOOL_STATUS_CORRUPT_DATA: - (void) printf( - gettext(" status: The pool data is corrupted.\n")); + printf_color(ANSI_YELLOW, gettext("The pool data is " + "corrupted.\n")); break; case ZPOOL_STATUS_OFFLINE_DEV: - printf_color(ANSI_BOLD, gettext("status: ")); printf_color(ANSI_YELLOW, gettext("One or more devices " "are offlined.\n")); break; case ZPOOL_STATUS_CORRUPT_POOL: - printf_color(ANSI_BOLD, gettext("status: ")); printf_color(ANSI_YELLOW, gettext("The pool metadata is " "corrupted.\n")); break; case ZPOOL_STATUS_VERSION_OLDER: - printf_color(ANSI_BOLD, gettext("status: ")); printf_color(ANSI_YELLOW, gettext("The pool is formatted using " "a legacy on-disk version.\n")); break; case ZPOOL_STATUS_VERSION_NEWER: - printf_color(ANSI_BOLD, gettext("status: ")); printf_color(ANSI_YELLOW, gettext("The pool is formatted using " "an incompatible version.\n")); break; case ZPOOL_STATUS_FEAT_DISABLED: - printf_color(ANSI_BOLD, gettext("status: ")); 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")); + "features are not enabled on the pool.\n" + "\t%s(Note that they may be intentionally disabled if the\n" + "\t%s'compatibility' property is set.)\n"), indent, indent); break; case ZPOOL_STATUS_COMPATIBILITY_ERR: - printf_color(ANSI_BOLD, gettext("status: ")); printf_color(ANSI_YELLOW, gettext("Error reading or parsing " "the file(s) indicated by the 'compatibility'\n" - "property.\n")); + "\t%sproperty.\n"), indent); 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")); + "\t%srequested by the 'compatibility' property.\n"), + indent); break; case ZPOOL_STATUS_UNSUP_FEAT_READ: - printf_color(ANSI_BOLD, gettext("status: ")); printf_color(ANSI_YELLOW, gettext("The pool uses the following " "feature(s) not supported on this system:\n")); color_start(ANSI_YELLOW); @@ -3124,66 +3127,60 @@ show_import(nvlist_t *config, boolean_t report_error) break; case ZPOOL_STATUS_UNSUP_FEAT_WRITE: - printf_color(ANSI_BOLD, gettext("status: ")); printf_color(ANSI_YELLOW, gettext("The pool can only be " - "accessed in read-only mode on this system. It\n\tcannot be" - " accessed in read-write mode because it uses the " - "following\n\tfeature(s) not supported on this system:\n")); + "accessed in read-only mode on this system. It\n" + "\t%scannot be accessed in read-write mode because it uses " + "the following\n" + "\t%sfeature(s) not supported on this system:\n"), + indent, indent); color_start(ANSI_YELLOW); zpool_print_unsup_feat(config); color_end(); break; case ZPOOL_STATUS_HOSTID_ACTIVE: - printf_color(ANSI_BOLD, gettext("status: ")); printf_color(ANSI_YELLOW, gettext("The pool is currently " "imported by another system.\n")); break; case ZPOOL_STATUS_HOSTID_REQUIRED: - printf_color(ANSI_BOLD, gettext("status: ")); printf_color(ANSI_YELLOW, gettext("The pool has the " - "multihost property on. It cannot\n\tbe safely imported " - "when the system hostid is not set.\n")); + "multihost property on. It cannot\n" + "\t%sbe safely imported when the system hostid is not " + "set.\n"), indent); break; case ZPOOL_STATUS_HOSTID_MISMATCH: - printf_color(ANSI_BOLD, gettext("status: ")); printf_color(ANSI_YELLOW, gettext("The pool was last accessed " "by another system.\n")); break; case ZPOOL_STATUS_FAULTED_DEV_R: case ZPOOL_STATUS_FAULTED_DEV_NR: - printf_color(ANSI_BOLD, gettext("status: ")); printf_color(ANSI_YELLOW, gettext("One or more devices are " "faulted.\n")); break; case ZPOOL_STATUS_BAD_LOG: - printf_color(ANSI_BOLD, gettext("status: ")); printf_color(ANSI_YELLOW, gettext("An intent log record cannot " "be read.\n")); break; case ZPOOL_STATUS_RESILVERING: case ZPOOL_STATUS_REBUILDING: - printf_color(ANSI_BOLD, gettext("status: ")); printf_color(ANSI_YELLOW, gettext("One or more devices were " "being resilvered.\n")); break; case ZPOOL_STATUS_ERRATA: - printf_color(ANSI_BOLD, gettext("status: ")); printf_color(ANSI_YELLOW, gettext("Errata #%d detected.\n"), errata); break; case ZPOOL_STATUS_NON_NATIVE_ASHIFT: - printf_color(ANSI_BOLD, gettext("status: ")); printf_color(ANSI_YELLOW, gettext("One or more devices are " "configured to use a non-native block size.\n" - "\tExpect reduced performance.\n")); + "\t%sExpect reduced performance.\n"), indent); break; default: @@ -3196,114 +3193,121 @@ show_import(nvlist_t *config, boolean_t report_error) /* * Print out an action according to the overall state of the pool. */ + if (vs->vs_state != VDEV_STATE_HEALTHY || + reason != ZPOOL_STATUS_ERRATA || errata != ZPOOL_ERRATA_NONE) { + (void) printf("%s", indent); + (void) printf(gettext("action: ")); + } if (vs->vs_state == VDEV_STATE_HEALTHY) { if (reason == ZPOOL_STATUS_VERSION_OLDER || reason == ZPOOL_STATUS_FEAT_DISABLED) { - (void) printf(gettext(" action: The pool can be " - "imported using its name or numeric identifier, " - "though\n\tsome features will not be available " - "without an explicit 'zpool upgrade'.\n")); + (void) printf(gettext("The pool can be imported using " + "its name or numeric identifier, though\n" + "\t%ssome features will not be available without " + "an explicit 'zpool upgrade'.\n"), indent); } else if (reason == ZPOOL_STATUS_COMPATIBILITY_ERR) { - (void) printf(gettext(" action: The pool can be " - "imported using its name or numeric\n\tidentifier, " - "though the file(s) indicated by its " - "'compatibility'\n\tproperty cannot be parsed at " - "this time.\n")); + (void) printf(gettext("The pool can be imported using " + "its name or numeric\n" + "\t%sidentifier, though the file(s) indicated by " + "its 'compatibility'\n" + "\t%sproperty cannot be parsed at this time.\n"), + indent, indent); } else if (reason == ZPOOL_STATUS_HOSTID_MISMATCH) { - (void) printf(gettext(" action: The pool can be " - "imported using its name or numeric " - "identifier and\n\tthe '-f' flag.\n")); + (void) printf(gettext("The pool can be imported using " + "its name or numeric identifier and\n" + "\t%sthe '-f' flag.\n"), indent); } else if (reason == ZPOOL_STATUS_ERRATA) { switch (errata) { - case ZPOOL_ERRATA_NONE: - break; - case ZPOOL_ERRATA_ZOL_2094_SCRUB: - (void) printf(gettext(" action: The pool can " - "be imported using its name or numeric " - "identifier,\n\thowever there is a compat" - "ibility issue which should be corrected" - "\n\tby running 'zpool scrub'\n")); + (void) printf(gettext("The pool can be " + "imported using its name or numeric " + "identifier,\n" + "\t%showever there is a compatibility " + "issue which should be corrected\n" + "\t%sby running 'zpool scrub'\n"), + indent, indent); break; case ZPOOL_ERRATA_ZOL_2094_ASYNC_DESTROY: - (void) printf(gettext(" action: The pool can" - "not be imported with this version of ZFS " - "due to\n\tan active asynchronous destroy. " - "Revert to an earlier version\n\tand " - "allow the destroy to complete before " - "updating.\n")); + (void) printf(gettext("The pool cannot be " + "imported with this version of ZFS due to\n" + "\t%san active asynchronous destroy. " + "Revert to an earlier version\n" + "\t%sand allow the destroy to complete " + "before updating.\n"), indent, indent); break; case ZPOOL_ERRATA_ZOL_6845_ENCRYPTION: - (void) printf(gettext(" action: Existing " - "encrypted datasets contain an on-disk " - "incompatibility, which\n\tneeds to be " - "corrected. Backup these datasets to new " - "encrypted datasets\n\tand destroy the " - "old ones.\n")); + (void) printf(gettext("Existing encrypted " + "datasets contain an on-disk " + "incompatibility, which\n" + "\t%sneeds to be corrected. Backup these " + "datasets to new encrypted datasets\n" + "\t%sand destroy the old ones.\n"), + indent, indent); break; case ZPOOL_ERRATA_ZOL_8308_ENCRYPTION: - (void) printf(gettext(" action: Existing " - "encrypted snapshots and bookmarks contain " - "an on-disk\n\tincompatibility. This may " - "cause on-disk corruption if they are used" - "\n\twith 'zfs recv'. To correct the " - "issue, enable the bookmark_v2 feature.\n\t" - "No additional action is needed if there " - "are no encrypted snapshots or\n\t" - "bookmarks. If preserving the encrypted " - "snapshots and bookmarks is\n\trequired, " - "use a non-raw send to backup and restore " - "them. Alternately,\n\tthey may be removed" - " to resolve the incompatibility.\n")); + (void) printf(gettext("Existing encrypted " + "snapshots and bookmarks contain an " + "on-disk\n" + "\t%sincompatibility. This may cause " + "on-disk corruption if they are used\n" + "\t%swith 'zfs recv'. To correct the " + "issue, enable the bookmark_v2 feature.\n" + "\t%sNo additional action is needed if " + "there are no encrypted snapshots or\n" + "\t%sbookmarks. If preserving the " + "encrypted snapshots and bookmarks is\n" + "\t%srequired, use a non-raw send to " + "backup and restore them. Alternately,\n" + "\t%sthey may be removed to resolve the " + "incompatibility.\n"), indent, indent, + indent, indent, indent, indent); break; default: /* * All errata must contain an action message. */ - assert(0); + assert(errata == ZPOOL_ERRATA_NONE); } } else { - (void) printf(gettext(" action: The pool can be " - "imported using its name or numeric " - "identifier.\n")); + (void) printf(gettext("The pool can be imported using " + "its name or numeric identifier.\n")); } } else if (vs->vs_state == VDEV_STATE_DEGRADED) { - (void) printf(gettext(" action: The pool can be imported " - "despite missing or damaged devices. The\n\tfault " - "tolerance of the pool may be compromised if imported.\n")); + (void) printf(gettext("The pool can be imported despite " + "missing or damaged devices. The\n" + "\t%sfault tolerance of the pool may be compromised if " + "imported.\n"), indent); } else { switch (reason) { case ZPOOL_STATUS_VERSION_NEWER: - (void) printf(gettext(" action: The pool cannot be " - "imported. Access the pool on a system running " - "newer\n\tsoftware, or recreate the pool from " - "backup.\n")); + (void) printf(gettext("The pool cannot be imported. " + "Access the pool on a system running newer\n" + "\t%ssoftware, or recreate the pool from " + "backup.\n"), indent); break; case ZPOOL_STATUS_UNSUP_FEAT_READ: - printf_color(ANSI_BOLD, gettext("action: ")); - printf_color(ANSI_YELLOW, gettext("The pool cannot be " - "imported. Access the pool on a system that " - "supports\n\tthe required feature(s), or recreate " - "the pool from backup.\n")); + (void) printf(gettext("The pool cannot be imported. " + "Access the pool on a system that supports\n" + "\t%sthe required feature(s), or recreate the pool " + "from backup.\n"), indent); break; case ZPOOL_STATUS_UNSUP_FEAT_WRITE: - printf_color(ANSI_BOLD, gettext("action: ")); - printf_color(ANSI_YELLOW, gettext("The pool cannot be " - "imported in read-write mode. Import the pool " - "with\n" - "\t\"-o readonly=on\", access the pool on a system " - "that supports the\n\trequired feature(s), or " - "recreate the pool from backup.\n")); + (void) printf(gettext("The pool cannot be imported in " + "read-write mode. Import the pool with\n" + "\t%s'-o readonly=on', access the pool on a system " + "that supports the\n" + "\t%srequired feature(s), or recreate the pool " + "from backup.\n"), indent, indent); break; case ZPOOL_STATUS_MISSING_DEV_R: case ZPOOL_STATUS_MISSING_DEV_NR: case ZPOOL_STATUS_BAD_GUID_SUM: - (void) printf(gettext(" action: The pool cannot be " - "imported. Attach the missing\n\tdevices and try " - "again.\n")); + (void) printf(gettext("The pool cannot be imported. " + "Attach the missing\n" + "\t%sdevices and try again.\n"), indent); break; case ZPOOL_STATUS_HOSTID_ACTIVE: VERIFY0(nvlist_lookup_nvlist(config, @@ -3317,47 +3321,49 @@ show_import(nvlist_t *config, boolean_t report_error) hostid = fnvlist_lookup_uint64(nvinfo, ZPOOL_CONFIG_MMP_HOSTID); - (void) printf(gettext(" action: The pool must be " - "exported from %s (hostid=%"PRIx64")\n\tbefore it " - "can be safely imported.\n"), hostname, hostid); + (void) printf(gettext("The pool must be exported from " + "%s (hostid=%"PRIx64")\n" + "\t%sbefore it can be safely imported.\n"), + hostname, hostid, indent); break; case ZPOOL_STATUS_HOSTID_REQUIRED: - (void) printf(gettext(" action: Set a unique system " - "hostid with the zgenhostid(8) command.\n")); + (void) printf(gettext("Set a unique system hostid with " + "the zgenhostid(8) command.\n")); break; default: - (void) printf(gettext(" action: The pool cannot be " - "imported due to damaged devices or data.\n")); + (void) printf(gettext("The pool cannot be imported due " + "to damaged devices or data.\n")); } } /* Print the comment attached to the pool. */ - if (nvlist_lookup_string(config, ZPOOL_CONFIG_COMMENT, &comment) == 0) + if (comment != NULL) (void) printf(gettext("comment: %s\n"), comment); /* * If the state is "closed" or "can't open", and the aux state * is "corrupt data": */ - if (((vs->vs_state == VDEV_STATE_CLOSED) || - (vs->vs_state == VDEV_STATE_CANT_OPEN)) && - (vs->vs_aux == VDEV_AUX_CORRUPT_DATA)) { + if ((vs->vs_state == VDEV_STATE_CLOSED || + vs->vs_state == VDEV_STATE_CANT_OPEN) && + vs->vs_aux == VDEV_AUX_CORRUPT_DATA) { if (pool_state == POOL_STATE_DESTROYED) - (void) printf(gettext("\tThe pool was destroyed, " - "but can be imported using the '-Df' flags.\n")); + (void) printf(gettext("\t%sThe pool was destroyed, " + "but can be imported using the '-Df' flags.\n"), + indent); else if (pool_state != POOL_STATE_EXPORTED) - (void) printf(gettext("\tThe pool may be active on " - "another system, but can be imported using\n\t" - "the '-f' flag.\n")); + (void) printf(gettext("\t%sThe pool may be active on " + "another system, but can be imported using\n" + "\t%sthe '-f' flag.\n"), indent, indent); } if (msgid != NULL) { - (void) printf(gettext( - " see: https://openzfs.github.io/openzfs-docs/msg/%s\n"), - msgid); + (void) printf(gettext("%s see: " + "https://openzfs.github.io/openzfs-docs/msg/%s\n"), + indent, msgid); } - (void) printf(gettext(" config:\n\n")); + (void) printf(gettext("%sconfig:\n\n"), indent); cb.cb_namewidth = max_width(NULL, nvroot, 0, strlen(name), VDEV_NAME_TYPE_ID); @@ -3371,9 +3377,10 @@ show_import(nvlist_t *config, boolean_t report_error) print_class_vdevs(NULL, &cb, nvroot, VDEV_ALLOC_CLASS_LOGS); if (reason == ZPOOL_STATUS_BAD_GUID_SUM) { - (void) printf(gettext("\n\tAdditional devices are known to " - "be part of this pool, though their\n\texact " - "configuration cannot be determined.\n")); + (void) printf(gettext("\n\t%sAdditional devices are known to " + "be part of this pool, though their\n" + "\t%sexact configuration cannot be determined.\n"), + indent, indent); } return (0); }