Illumos #1693: persistent 'comment' field for a zpool

Reviewed by: George Wilson <gwilson@zfsmail.com>
Reviewed by: Eric Schrock <eric.schrock@delphix.com>
Approved by: Richard Lowe <richlowe@richlowe.net>

References:
  https://www.illumos.org/issues/1693

Ported by: Martin Matuska <martin@matuska.org>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #678

cmd/zpool/zpool_main.c

@@ -1343,6 +1343,7 @@ show_import(nvlist_t *config)
 	const char *health;
 	uint_t vsc;
 	int namewidth;
+	char *comment;
 
 	verify(nvlist_lookup_string(config, ZPOOL_CONFIG_POOL_NAME,
 	    &name) == 0);
@@ -1359,9 +1360,9 @@ show_import(nvlist_t *config)
 
 	reason = zpool_import_status(config, &msgid);
 
-	(void) printf(gettext("  pool: %s\n"), name);
+	(void) printf(gettext("   pool: %s\n"), name);
-	(void) printf(gettext("    id: %llu\n"), (u_longlong_t)guid);
+	(void) printf(gettext("     id: %llu\n"), (u_longlong_t)guid);
-	(void) printf(gettext(" state: %s"), health);
+	(void) printf(gettext("  state: %s"), health);
 	if (pool_state == POOL_STATE_DESTROYED)
 		(void) printf(gettext(" (DESTROYED)"));
 	(void) printf("\n");
@@ -1370,58 +1371,59 @@ show_import(nvlist_t *config)
 	case ZPOOL_STATUS_MISSING_DEV_R:
 	case ZPOOL_STATUS_MISSING_DEV_NR:
 	case ZPOOL_STATUS_BAD_GUID_SUM:
-		(void) printf(gettext("status: One or more devices are missing "
+		(void) printf(gettext(" status: One or more devices are "
-		    "from the system.\n"));
+		    "missing from the system.\n"));
 		break;
 
 	case ZPOOL_STATUS_CORRUPT_LABEL_R:
 	case ZPOOL_STATUS_CORRUPT_LABEL_NR:
-		(void) printf(gettext("status: One or more devices contains "
+		(void) printf(gettext(" status: One or more devices contains "
 		    "corrupted data.\n"));
 		break;
 
 	case ZPOOL_STATUS_CORRUPT_DATA:
-		(void) printf(gettext("status: The pool data is corrupted.\n"));
+		(void) printf(
+		    gettext(" status: The pool data is corrupted.\n"));
 		break;
 
 	case ZPOOL_STATUS_OFFLINE_DEV:
-		(void) printf(gettext("status: One or more devices "
+		(void) printf(gettext(" status: One or more devices "
 		    "are offlined.\n"));
 		break;
 
 	case ZPOOL_STATUS_CORRUPT_POOL:
-		(void) printf(gettext("status: The pool metadata is "
+		(void) printf(gettext(" status: The pool metadata is "
 		    "corrupted.\n"));
 		break;
 
 	case ZPOOL_STATUS_VERSION_OLDER:
-		(void) printf(gettext("status: The pool is formatted using an "
+		(void) printf(gettext(" status: The pool is formatted using an "
 		    "older on-disk version.\n"));
 		break;
 
 	case ZPOOL_STATUS_VERSION_NEWER:
-		(void) printf(gettext("status: The pool is formatted using an "
+		(void) printf(gettext(" status: The pool is formatted using an "
 		    "incompatible version.\n"));
 		break;
 
 	case ZPOOL_STATUS_HOSTID_MISMATCH:
-		(void) printf(gettext("status: The pool was last accessed by "
+		(void) printf(gettext(" status: The pool was last accessed by "
 		    "another system.\n"));
 		break;
 
 	case ZPOOL_STATUS_FAULTED_DEV_R:
 	case ZPOOL_STATUS_FAULTED_DEV_NR:
-		(void) printf(gettext("status: One or more devices are "
+		(void) printf(gettext(" status: One or more devices are "
 		    "faulted.\n"));
 		break;
 
 	case ZPOOL_STATUS_BAD_LOG:
-		(void) printf(gettext("status: An intent log record cannot be "
+		(void) printf(gettext(" status: An intent log record cannot be "
 		    "read.\n"));
 		break;
 
 	case ZPOOL_STATUS_RESILVERING:
-		(void) printf(gettext("status: One or more devices were being "
+		(void) printf(gettext(" status: One or more devices were being "
 		    "resilvered.\n"));
 		break;
 
@@ -1437,26 +1439,26 @@ show_import(nvlist_t *config)
 	 */
 	if (vs->vs_state == VDEV_STATE_HEALTHY) {
 		if (reason == ZPOOL_STATUS_VERSION_OLDER)
-			(void) printf(gettext("action: The pool can be "
+			(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"));
 		else if (reason == ZPOOL_STATUS_HOSTID_MISMATCH)
-			(void) printf(gettext("action: The pool can be "
+			(void) printf(gettext(" action: The pool can be "
 			    "imported using its name or numeric "
 			    "identifier and\n\tthe '-f' flag.\n"));
 		else
-			(void) printf(gettext("action: The pool can be "
+			(void) printf(gettext(" action: 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 "
+		(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"));
 	} else {
 		switch (reason) {
 		case ZPOOL_STATUS_VERSION_NEWER:
-			(void) printf(gettext("action: The pool cannot be "
+			(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"));
@@ -1464,16 +1466,20 @@ show_import(nvlist_t *config)
 		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 "
+			(void) printf(gettext(" action: The pool cannot be "
 			    "imported. Attach the missing\n\tdevices and try "
 			    "again.\n"));
 			break;
 		default:
-			(void) printf(gettext("action: The pool cannot be "
+			(void) printf(gettext(" action: 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)
+		(void) printf(gettext("comment: %s\n"), comment);
+
 	/*
 	 * If the state is "closed" or "can't open", and the aux state
 	 * is "corrupt data":
@@ -1494,7 +1500,7 @@ show_import(nvlist_t *config)
 		(void) printf(gettext("   see: http://zfsonlinux.org/msg/%s\n"),
 		    msgid);
 
-	(void) printf(gettext("config:\n\n"));
+	(void) printf(gettext(" config:\n\n"));
 
 	namewidth = max_width(NULL, nvroot, 0, 0);
 	if (namewidth < 10)

include/sys/fs/zfs.h

@@ -167,9 +167,13 @@ typedef enum {
 	ZPOOL_PROP_ALLOCATED,
 	ZPOOL_PROP_READONLY,
 	ZPOOL_PROP_ASHIFT,
+	ZPOOL_PROP_COMMENT,
 	ZPOOL_NUM_PROPS
 } zpool_prop_t;
 
+/* Small enough to not hog a whole line of printout in zpool(1M). */
+#define	ZPROP_MAX_COMMENT	32
+
 #define	ZPROP_CONT		-2
 #define	ZPROP_INVAL		-1
 
@@ -500,6 +504,7 @@ typedef struct zpool_rewind_policy {
 #define	ZPOOL_CONFIG_SPLIT_LIST		"guid_list"
 #define	ZPOOL_CONFIG_REMOVING		"removing"
 #define	ZPOOL_CONFIG_RESILVERING	"resilvering"
+#define	ZPOOL_CONFIG_COMMENT		"comment"
 #define	ZPOOL_CONFIG_SUSPENDED		"suspended"	/* not stored on disk */
 #define	ZPOOL_CONFIG_TIMESTAMP		"timestamp"	/* not stored on disk */
 #define	ZPOOL_CONFIG_BOOTFS		"bootfs"	/* not stored on disk */

include/sys/spa_impl.h

@@ -112,6 +112,7 @@ struct spa {
 	 * Fields protected by spa_namespace_lock.
 	 */
 	char		spa_name[MAXNAMELEN];	/* pool name */
+	char		*spa_comment;		/* comment */
 	avl_node_t	spa_avl;		/* node in spa_namespace_avl */
 	nvlist_t	*spa_config;		/* last synced config */
 	nvlist_t	*spa_config_syncing;	/* currently syncing config */

include/sys/zfs_context.h

@@ -60,6 +60,7 @@
 #include <sys/zfs_debug.h>
 #include <sys/fm/fs/zfs.h>
 #include <sys/sunddi.h>
+#include <sys/ctype.h>
 #include <linux/dcache_compat.h>
 
 #else /* _KERNEL */
@@ -92,6 +93,7 @@
 #include <atomic.h>
 #include <dirent.h>
 #include <time.h>
+#include <ctype.h>
 #include <sys/note.h>
 #include <sys/types.h>
 #include <sys/cred.h>

lib/libzfs/libzfs_import.c

@@ -20,6 +20,8 @@
  */
 /*
  * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright 2011 Nexenta Systems, Inc. All rights reserved.
+ * Copyright (c) 2011 by Delphix. All rights reserved.
  */
 
 /*
@@ -441,7 +443,7 @@ get_configs(libzfs_handle_t *hdl, pool_list_t *pl, boolean_t active_ok)
 	uint_t i, nspares, nl2cache;
 	boolean_t config_seen;
 	uint64_t best_txg;
-	char *name, *hostname;
+	char *name, *hostname, *comment;
 	uint64_t version, guid;
 	uint_t children = 0;
 	nvlist_t **child = NULL;
@@ -530,6 +532,7 @@ get_configs(libzfs_handle_t *hdl, pool_list_t *pl, boolean_t active_ok)
 				 *	version
 				 * 	pool guid
 				 * 	name
+				 *	comment (if available)
 				 * 	pool state
 				 *	hostid (if available)
 				 *	hostname (if available)
@@ -551,11 +554,24 @@ get_configs(libzfs_handle_t *hdl, pool_list_t *pl, boolean_t active_ok)
 				if (nvlist_add_string(config,
 				    ZPOOL_CONFIG_POOL_NAME, name) != 0)
 					goto nomem;
+
+				/*
+				 * COMMENT is optional, don't bail if it's not
+				 * there, instead, set it to NULL.
+				 */
+				if (nvlist_lookup_string(tmp,
+				    ZPOOL_CONFIG_COMMENT, &comment) != 0)
+					comment = NULL;
+				else if (nvlist_add_string(config,
+				    ZPOOL_CONFIG_COMMENT, comment) != 0)
+					goto nomem;
+
 				verify(nvlist_lookup_uint64(tmp,
 				    ZPOOL_CONFIG_POOL_STATE, &state) == 0);
 				if (nvlist_add_uint64(config,
 				    ZPOOL_CONFIG_POOL_STATE, state) != 0)
 					goto nomem;
+
 				hostid = 0;
 				if (nvlist_lookup_uint64(tmp,
 				    ZPOOL_CONFIG_HOSTID, &hostid) == 0) {

lib/libzfs/libzfs_pool.c

@@ -235,6 +235,7 @@ zpool_get_prop(zpool_handle_t *zhp, zpool_prop_t prop, char *buf, size_t len,
 
 		case ZPOOL_PROP_ALTROOT:
 		case ZPOOL_PROP_CACHEFILE:
+		case ZPOOL_PROP_COMMENT:
 			if (zhp->zpool_props != NULL ||
 			    zpool_get_all_props(zhp) == 0) {
 				(void) strlcpy(buf,
@@ -385,7 +386,7 @@ zpool_valid_proplist(libzfs_handle_t *hdl, const char *poolname,
 	zpool_prop_t prop;
 	char *strval;
 	uint64_t intval;
-	char *slash;
+	char *slash, *check;
 	struct stat64 statbuf;
 	zpool_handle_t *zhp;
 	nvlist_t *nvroot;
@@ -566,6 +567,26 @@ zpool_valid_proplist(libzfs_handle_t *hdl, const char *poolname,
 			*slash = '/';
 			break;
 
+		case ZPOOL_PROP_COMMENT:
+			for (check = strval; *check != '\0'; check++) {
+				if (!isprint(*check)) {
+					zfs_error_aux(hdl,
+					    dgettext(TEXT_DOMAIN,
+					    "comment may only have printable "
+					    "characters"));
+					(void) zfs_error(hdl, EZFS_BADPROP,
+					    errbuf);
+					goto error;
+				}
+			}
+			if (strlen(strval) > ZPROP_MAX_COMMENT) {
+				zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
+				    "comment must not exceed %d characters"),
+				    ZPROP_MAX_COMMENT);
+				(void) zfs_error(hdl, EZFS_BADPROP, errbuf);
+				goto error;
+			}
+			break;
 		case ZPOOL_PROP_READONLY:
 			if (!flags.import) {
 				zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,

man/man8/zpool.8

@@ -1,10 +1,10 @@
 '\" te
 .\" Copyright (c) 2007, Sun Microsystems, Inc. All Rights Reserved.
+.\" Copyright 2011 Nexenta Systems, Inc.  All rights reserved.
 .\" The contents of this file are subject to the terms of the Common Development and Distribution License (the "License"). You may not use this file except in compliance with the License. You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE or http://www.opensolaris.org/os/licensing.
 .\" See the License for the specific language governing permissions and limitations under the License. When distributing Covered Code, include this CDDL HEADER in each file and include the License file at usr/src/OPENSOLARIS.LICENSE. If applicable, add the following below this CDDL HEADER, with the
 .\" fields enclosed by brackets "[]" replaced with your own identifying information: Portions Copyright [yyyy] [name of copyright owner]
-.\" Copyright 2011 Nexenta Systems, Inc.  All rights reserved.
+.TH zpool 8 "2 August 2012" "ZFS pool 28, filesystem 5" "System Administration Commands"
-.TH zpool 8 "10 July 2012" "ZFS pool 28, filesystem 5" "System Administration Commands"
 .SH NAME
 zpool \- configures ZFS storage pools
 .SH SYNOPSIS
@@ -461,6 +461,17 @@ Amount of storage available within the pool. This property can also be referred
 Percentage of pool space used. This property can also be referred to by its shortened column name, "cap".
 .RE
 
+.sp
+.ne 2
+.mk
+.na
+\fB\fBcomment\fR\fR
+.ad
+.RS 20n
+.rt
+A text string consisting of printable ASCII characters that will be stored such that it is available even if the pool becomes faulted.  An administrator can provide additional information about a pool using this property.
+.RE
+
 .sp
 .ne 2
 .mk

module/zcommon/zpool_prop.c

@@ -20,6 +20,8 @@
  */
 /*
  * Copyright (c) 2007, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright 2011 Nexenta Systems, Inc. All rights reserved.
+ * Copyright (c) 2011 by Delphix. All rights reserved.
  */
 
 #include <sys/zio.h>
@@ -69,6 +71,8 @@ zpool_prop_init(void)
 	    ZFS_TYPE_POOL, "<filesystem>", "BOOTFS");
 	zprop_register_string(ZPOOL_PROP_CACHEFILE, "cachefile", NULL,
 	    PROP_DEFAULT, ZFS_TYPE_POOL, "<file> | none", "CACHEFILE");
+	zprop_register_string(ZPOOL_PROP_COMMENT, "comment", NULL,
+	    PROP_DEFAULT, ZFS_TYPE_POOL, "<comment-string>", "COMMENT");
 
 	/* readonly number properties */
 	zprop_register_number(ZPOOL_PROP_SIZE, "size", 0, PROP_READONLY,

module/zfs/spa.c

@@ -206,6 +206,11 @@ spa_prop_get_config(spa_t *spa, nvlist_t **nvp)
 
 	spa_prop_add_list(*nvp, ZPOOL_PROP_GUID, NULL, spa_guid(spa), src);
 
+	if (spa->spa_comment != NULL) {
+		spa_prop_add_list(*nvp, ZPOOL_PROP_COMMENT, spa->spa_comment,
+		    0, ZPROP_SRC_LOCAL);
+	}
+
 	if (spa->spa_root != NULL)
 		spa_prop_add_list(*nvp, ZPOOL_PROP_ALTROOT, spa->spa_root,
 		    0, ZPROP_SRC_LOCAL);
@@ -347,7 +352,7 @@ spa_prop_validate(spa_t *spa, nvlist_t *props)
 		char *propname, *strval;
 		uint64_t intval;
 		objset_t *os;
-		char *slash;
+		char *slash, *check;
 
 		propname = nvpair_name(elem);
 
@@ -467,6 +472,20 @@ spa_prop_validate(spa_t *spa, nvlist_t *props)
 				error = EINVAL;
 			break;
 
+		case ZPOOL_PROP_COMMENT:
+			if ((error = nvpair_value_string(elem, &strval)) != 0)
+				break;
+			for (check = strval; *check != '\0'; check++) {
+				if (!isprint(*check)) {
+					error = EINVAL;
+					break;
+				}
+				check++;
+			}
+			if (strlen(strval) > ZPROP_MAX_COMMENT)
+				error = E2BIG;
+			break;
+
 		case ZPOOL_PROP_DEDUPDITTO:
 			if (spa_version(spa) < SPA_VERSION_DEDUP)
 				error = ENOTSUP;
@@ -1060,6 +1079,11 @@ spa_unload(spa_t *spa)
 
 	spa->spa_async_suspended = 0;
 
+	if (spa->spa_comment != NULL) {
+		spa_strfree(spa->spa_comment);
+		spa->spa_comment = NULL;
+	}
+
 	spa_config_exit(spa, SCL_ALL, FTAG);
 }
 
@@ -1787,6 +1811,7 @@ spa_load(spa_t *spa, spa_load_state_t state, spa_import_type_t type,
 {
 	nvlist_t *config = spa->spa_config;
 	char *ereport = FM_EREPORT_ZFS_POOL;
+	char *comment;
 	int error;
 	uint64_t pool_guid;
 	nvlist_t *nvl;
@@ -1794,6 +1819,10 @@ spa_load(spa_t *spa, spa_load_state_t state, spa_import_type_t type,
 	if (nvlist_lookup_uint64(config, ZPOOL_CONFIG_POOL_GUID, &pool_guid))
 		return (EINVAL);
 
+	ASSERT(spa->spa_comment == NULL);
+	if (nvlist_lookup_string(config, ZPOOL_CONFIG_COMMENT, &comment) == 0)
+		spa->spa_comment = spa_strdup(comment);
+
 	/*
 	 * Versioning wasn't explicitly added to the label until later, so if
 	 * it's not present treat it as the initial version.
@@ -5401,6 +5430,20 @@ spa_sync_props(void *arg1, void *arg2, dmu_tx_t *tx)
 			 * properties.
 			 */
 			break;
+		case ZPOOL_PROP_COMMENT:
+			VERIFY(nvpair_value_string(elem, &strval) == 0);
+			if (spa->spa_comment != NULL)
+				spa_strfree(spa->spa_comment);
+			spa->spa_comment = spa_strdup(strval);
+			/*
+			 * We need to dirty the configuration on all the vdevs
+			 * so that their labels get updated.  It's unnecessary
+			 * to do this for pool creation since the vdev's
+			 * configuratoin has already been dirtied.
+			 */
+			if (tx->tx_txg != TXG_INITIAL)
+				vdev_config_dirty(spa->spa_root_vdev);
+			break;
 		default:
 			/*
 			 * Set pool property values in the poolprops mos object.

module/zfs/spa_config.c

@@ -21,6 +21,8 @@
 
 /*
  * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright 2011 Nexenta Systems, Inc. All rights reserved.
+ * Copyright (c) 2011 by Delphix. All rights reserved.
  */
 
 #include <sys/spa.h>
@@ -344,6 +346,10 @@ spa_config_generate(spa_t *spa, vdev_t *vd, uint64_t txg, int getstats)
 	    txg) == 0);
 	VERIFY(nvlist_add_uint64(config, ZPOOL_CONFIG_POOL_GUID,
 	    spa_guid(spa)) == 0);
+	VERIFY(spa->spa_comment == NULL || nvlist_add_string(config,
+	    ZPOOL_CONFIG_COMMENT, spa->spa_comment) == 0);
+
+
 #ifdef	_KERNEL
 	hostid = zone_get_hostid(NULL);
 #else	/* _KERNEL */