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

diff --git a/cmd/zpool/zpool_main.c b/cmd/zpool/zpool_main.c
index 3679b4e..21548c8 100644 (file)
--- a/cmd/zpool/zpool_main.c
+++ b/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("    id: %llu\n"), (u_longlong_t)guid);
-       (void) printf(gettext(" state: %s"), health);
+       (void) printf(gettext("   pool: %s\n"), name);
+       (void) printf(gettext("     id: %llu\n"), (u_longlong_t)guid);
+       (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 "
-                   "from the system.\n"));
+               (void) printf(gettext(" status: One or more devices are "
+                   "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)

diff --git a/include/sys/fs/zfs.h b/include/sys/fs/zfs.h
index 86d7ad5..13f9cdc 100644 (file)
--- a/include/sys/fs/zfs.h
+++ b/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 */

diff --git a/include/sys/spa_impl.h b/include/sys/spa_impl.h
index 1317a9f..d367486 100644 (file)
--- a/include/sys/spa_impl.h
+++ b/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 */

diff --git a/include/sys/zfs_context.h b/include/sys/zfs_context.h
index e4af6fc..7bcdc9e 100644 (file)
--- a/include/sys/zfs_context.h
+++ b/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>

diff --git a/lib/libzfs/libzfs_import.c b/lib/libzfs/libzfs_import.c
index 7048a52..d12dc86 100644 (file)
--- a/lib/libzfs/libzfs_import.c
+++ b/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) {

diff --git a/lib/libzfs/libzfs_pool.c b/lib/libzfs/libzfs_pool.c
index 8339305..68bfdee 100644 (file)
--- a/lib/libzfs/libzfs_pool.c
+++ b/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,

diff --git a/man/man8/zpool.8 b/man/man8/zpool.8
index 5015e30..ec931a2 100644 (file)
--- a/man/man8/zpool.8
+++ b/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 "10 July 2012" "ZFS pool 28, filesystem 5" "System Administration Commands"
+.TH zpool 8 "2 August 2012" "ZFS pool 28, filesystem 5" "System Administration Commands"
 .SH NAME
 zpool \- configures ZFS storage pools
 .SH SYNOPSIS
@@ -465,6 +465,17 @@ Percentage of pool space used. This property can also be referred to by its shor
 .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
+.na
 \fB\fBhealth\fR\fR
 .ad
 .RS 20n

diff --git a/module/zcommon/zpool_prop.c b/module/zcommon/zpool_prop.c
index 249dd64..6c69fca 100644 (file)
--- a/module/zcommon/zpool_prop.c
+++ b/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,

diff --git a/module/zfs/spa.c b/module/zfs/spa.c
index 40849bc..692664b 100644 (file)
--- a/module/zfs/spa.c
+++ b/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.

diff --git a/module/zfs/spa_config.c b/module/zfs/spa_config.c
index d84d6b0..d814ae2 100644 (file)
--- a/module/zfs/spa_config.c
+++ b/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 */