7386 zfs get does not work properly with bookmarks

Reviewed by: Simon Klinkert <simon.klinkert@gmail.com>
Reviewed by: Paul Dagnelie <pcd@delphix.com>
Approved by: Matthew Ahrens <mahrens@delphix.com>

13 files changed, 307 insertions, 70 deletions

diff --git a/usr/src/cmd/zfs/zfs_main.c b/usr/src/cmd/zfs/zfs_main.c
index fc55e465b1..769d758ef5 100644
--- a/usr/src/cmd/zfs/zfs_main.c
+++ b/usr/src/cmd/zfs/zfs_main.c
@@ -232,7 +232,7 @@ get_usage(zfs_help_t idx)
 		    "[-o \"all\" | field[,...]]\n"
 		    "\t    [-t type[,...]] [-s source[,...]]\n"
 		    "\t    <\"all\" | property[,...]> "
-		    "[filesystem|volume|snapshot] ...\n"));
+		    "[filesystem|volume|snapshot|bookmark] ...\n"));
 	case HELP_INHERIT:
 		return (gettext("\tinherit [-rS] <property> "
 		    "<filesystem|volume|snapshot> ...\n"));
@@ -1594,7 +1594,7 @@ zfs_do_get(int argc, char **argv)
 {
 	zprop_get_cbdata_t cb = { 0 };
 	int i, c, flags = ZFS_ITER_ARGS_CAN_BE_PATHS;
-	int types = ZFS_TYPE_DATASET;
+	int types = ZFS_TYPE_DATASET | ZFS_TYPE_BOOKMARK;
 	char *value, *fields;
 	int ret = 0;
 	int limit = 0;
diff --git a/usr/src/common/zfs/zfs_namecheck.c b/usr/src/common/zfs/zfs_namecheck.c
index 439d4491a0..950d41f353 100644
--- a/usr/src/common/zfs/zfs_namecheck.c
+++ b/usr/src/common/zfs/zfs_namecheck.c
@@ -120,9 +120,9 @@ permset_namecheck(const char *path, namecheck_err_t *why, char *what)
 }
 
 /*
- * Dataset names must be of the following form:
+ * Entity names must be of the following form:
  *
- * 	[component][/]*[component][@component]
+ * 	[component/]*[component][(@|#)component]?
  *
  * Where each component is made up of alphanumeric characters plus the following
  * characters:
@@ -133,10 +133,10 @@ permset_namecheck(const char *path, namecheck_err_t *why, char *what)
  * names for temporary clones (for online recv).
  */
 int
-dataset_namecheck(const char *path, namecheck_err_t *why, char *what)
+entity_namecheck(const char *path, namecheck_err_t *why, char *what)
 {
-	const char *loc, *end;
-	int found_snapshot;
+	const char *start, *end;
+	int found_delim;
 
 	/*
 	 * Make sure the name is not too long.
@@ -161,12 +161,13 @@ dataset_namecheck(const char *path, namecheck_err_t *why, char *what)
 		return (-1);
 	}
 
-	loc = path;
-	found_snapshot = 0;
+	start = path;
+	found_delim = 0;
 	for (;;) {
 		/* Find the end of this component */
-		end = loc;
-		while (*end != '/' && *end != '@' && *end != '\0')
+		end = start;
+		while (*end != '/' && *end != '@' && *end != '#' &&
+		    *end != '\0')
 			end++;
 
 		if (*end == '\0' && end[-1] == '/') {
@@ -176,25 +177,8 @@ dataset_namecheck(const char *path, namecheck_err_t *why, char *what)
 			return (-1);
 		}
 
-		/* Zero-length components are not allowed */
-		if (loc == end) {
-			if (why) {
-				/*
-				 * Make sure this is really a zero-length
-				 * component and not a '@@'.
-				 */
-				if (*end == '@' && found_snapshot) {
-					*why = NAME_ERR_MULTIPLE_AT;
-				} else {
-					*why = NAME_ERR_EMPTY_COMPONENT;
-				}
-			}
-
-			return (-1);
-		}
-
 		/* Validate the contents of this component */
-		while (loc != end) {
+		for (const char *loc = start; loc != end; loc++) {
 			if (!valid_char(*loc) && *loc != '%') {
 				if (why) {
 					*why = NAME_ERR_INVALCHAR;
@@ -202,43 +186,64 @@ dataset_namecheck(const char *path, namecheck_err_t *why, char *what)
 				}
 				return (-1);
 			}
-			loc++;
 		}
 
-		/* If we've reached the end of the string, we're OK */
-		if (*end == '\0')
-			return (0);
-
-		if (*end == '@') {
-			/*
-			 * If we've found an @ symbol, indicate that we're in
-			 * the snapshot component, and report a second '@'
-			 * character as an error.
-			 */
-			if (found_snapshot) {
+		/* Snapshot or bookmark delimiter found */
+		if (*end == '@' || *end == '#') {
+			/* Multiple delimiters are not allowed */
+			if (found_delim != 0) {
 				if (why)
-					*why = NAME_ERR_MULTIPLE_AT;
+					*why = NAME_ERR_MULTIPLE_DELIMITERS;
 				return (-1);
 			}
 
-			found_snapshot = 1;
+			found_delim = 1;
+		}
+
+		/* Zero-length components are not allowed */
+		if (start == end) {
+			if (why)
+				*why = NAME_ERR_EMPTY_COMPONENT;
+			return (-1);
 		}
 
+		/* If we've reached the end of the string, we're OK */
+		if (*end == '\0')
+			return (0);
+
 		/*
-		 * If there is a '/' in a snapshot name
+		 * If there is a '/' in a snapshot or bookmark name
 		 * then report an error
 		 */
-		if (*end == '/' && found_snapshot) {
+		if (*end == '/' && found_delim != 0) {
 			if (why)
 				*why = NAME_ERR_TRAILING_SLASH;
 			return (-1);
 		}
 
 		/* Update to the next component */
-		loc = end + 1;
+		start = end + 1;
 	}
 }
 
+/*
+ * Dataset is any entity, except bookmark
+ */
+int
+dataset_namecheck(const char *path, namecheck_err_t *why, char *what)
+{
+	int ret = entity_namecheck(path, why, what);
+
+	if (ret == 0 && strchr(path, '#') != NULL) {
+		if (why != NULL) {
+			*why = NAME_ERR_INVALCHAR;
+			*what = '#';
+		}
+		return (-1);
+	}
+
+	return (ret);
+}
 
 /*
  * mountpoint names must be of the following form:
diff --git a/usr/src/common/zfs/zfs_namecheck.h b/usr/src/common/zfs/zfs_namecheck.h
index cbefbaa0d5..db70641dba 100644
--- a/usr/src/common/zfs/zfs_namecheck.h
+++ b/usr/src/common/zfs/zfs_namecheck.h
@@ -38,7 +38,7 @@ typedef enum {
 	NAME_ERR_EMPTY_COMPONENT,	/* name contains an empty component */
 	NAME_ERR_TRAILING_SLASH,	/* name ends with a slash */
 	NAME_ERR_INVALCHAR,		/* invalid character found */
-	NAME_ERR_MULTIPLE_AT,		/* multiple '@' characters found */
+	NAME_ERR_MULTIPLE_DELIMITERS,	/* multiple '@'/'#' delimiters found */
 	NAME_ERR_NOLETTER,		/* pool doesn't begin with a letter */
 	NAME_ERR_RESERVED,		/* entire name is reserved */
 	NAME_ERR_DISKLIKE,		/* reserved disk name (c[0-9].*) */
@@ -49,6 +49,7 @@ typedef enum {
 #define	ZFS_PERMSET_MAXLEN	64
 
 int pool_namecheck(const char *, namecheck_err_t *, char *);
+int entity_namecheck(const char *, namecheck_err_t *, char *);
 int dataset_namecheck(const char *, namecheck_err_t *, char *);
 int mountpoint_namecheck(const char *, namecheck_err_t *);
 int zfs_component_namecheck(const char *, namecheck_err_t *, char *);
diff --git a/usr/src/lib/libzfs/common/libzfs_dataset.c b/usr/src/lib/libzfs/common/libzfs_dataset.c
index cd9b61f0d2..4200f49cca 100644
--- a/usr/src/lib/libzfs/common/libzfs_dataset.c
+++ b/usr/src/lib/libzfs/common/libzfs_dataset.c
@@ -105,7 +105,7 @@ zfs_validate_name(libzfs_handle_t *hdl, const char *path, int type,
 	char what;
 
 	(void) zfs_prop_get_table();
-	if (dataset_namecheck(path, &why, &what) != 0) {
+	if (entity_namecheck(path, &why, &what) != 0) {
 		if (hdl != NULL) {
 			switch (why) {
 			case NAME_ERR_TOOLONG:
@@ -134,9 +134,10 @@ zfs_validate_name(libzfs_handle_t *hdl, const char *path, int type,
 				    "'%c' in name"), what);
 				break;
 
-			case NAME_ERR_MULTIPLE_AT:
+			case NAME_ERR_MULTIPLE_DELIMITERS:
 				zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
-				    "multiple '@' delimiters in name"));
+				    "multiple '@' and/or '#' delimiters in "
+				    "name"));
 				break;
 
 			case NAME_ERR_NOLETTER:
@@ -167,7 +168,7 @@ zfs_validate_name(libzfs_handle_t *hdl, const char *path, int type,
 	if (!(type & ZFS_TYPE_SNAPSHOT) && strchr(path, '@') != NULL) {
 		if (hdl != NULL)
 			zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
-			    "snapshot delimiter '@' in filesystem name"));
+			    "snapshot delimiter '@' is not expected here"));
 		return (0);
 	}
 
@@ -178,6 +179,20 @@ zfs_validate_name(libzfs_handle_t *hdl, const char *path, int type,
 		return (0);
 	}
 
+	if (!(type & ZFS_TYPE_BOOKMARK) && strchr(path, '#') != NULL) {
+		if (hdl != NULL)
+			zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
+			    "bookmark delimiter '#' is not expected here"));
+		return (0);
+	}
+
+	if (type == ZFS_TYPE_BOOKMARK && strchr(path, '#') == NULL) {
+		if (hdl != NULL)
+			zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
+			    "missing '#' delimiter in bookmark name"));
+		return (0);
+	}
+
 	if (modifying && strchr(path, '%') != NULL) {
 		if (hdl != NULL)
 			zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
@@ -615,8 +630,36 @@ make_bookmark_handle(zfs_handle_t *parent, const char *path,
 	return (zhp);
 }
 
+struct zfs_open_bookmarks_cb_data {
+	const char *path;
+	zfs_handle_t *zhp;
+};
+
+static int
+zfs_open_bookmarks_cb(zfs_handle_t *zhp, void *data)
+{
+	struct zfs_open_bookmarks_cb_data *dp = data;
+
+	/*
+	 * Is it the one we are looking for?
+	 */
+	if (strcmp(dp->path, zfs_get_name(zhp)) == 0) {
+		/*
+		 * We found it.  Save it and let the caller know we are done.
+		 */
+		dp->zhp = zhp;
+		return (EEXIST);
+	}
+
+	/*
+	 * Not found.  Close the handle and ask for another one.
+	 */
+	zfs_close(zhp);
+	return (0);
+}
+
 /*
- * Opens the given snapshot, filesystem, or volume.   The 'types'
+ * Opens the given snapshot, bookmark, filesystem, or volume.   The 'types'
  * argument is a mask of acceptable types.  The function will print an
  * appropriate error message and return NULL if it can't be opened.
  */
@@ -625,6 +668,7 @@ zfs_open(libzfs_handle_t *hdl, const char *path, int types)
 {
 	zfs_handle_t *zhp;
 	char errbuf[1024];
+	char *bookp;
 
 	(void) snprintf(errbuf, sizeof (errbuf),
 	    dgettext(TEXT_DOMAIN, "cannot open '%s'"), path);
@@ -632,20 +676,68 @@ zfs_open(libzfs_handle_t *hdl, const char *path, int types)
 	/*
 	 * Validate the name before we even try to open it.
 	 */
-	if (!zfs_validate_name(hdl, path, ZFS_TYPE_DATASET, B_FALSE)) {
-		zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
-		    "invalid dataset name"));
+	if (!zfs_validate_name(hdl, path, types, B_FALSE)) {
 		(void) zfs_error(hdl, EZFS_INVALIDNAME, errbuf);
 		return (NULL);
 	}
 
 	/*
-	 * Try to get stats for the dataset, which will tell us if it exists.
+	 * Bookmarks needs to be handled separately.
 	 */
-	errno = 0;
-	if ((zhp = make_dataset_handle(hdl, path)) == NULL) {
-		(void) zfs_standard_error(hdl, errno, errbuf);
-		return (NULL);
+	bookp = strchr(path, '#');
+	if (bookp == NULL) {
+		/*
+		 * Try to get stats for the dataset, which will tell us if it
+		 * exists.
+		 */
+		errno = 0;
+		if ((zhp = make_dataset_handle(hdl, path)) == NULL) {
+			(void) zfs_standard_error(hdl, errno, errbuf);
+			return (NULL);
+		}
+	} else {
+		char dsname[ZFS_MAX_DATASET_NAME_LEN];
+		zfs_handle_t *pzhp;
+		struct zfs_open_bookmarks_cb_data cb_data = {path, NULL};
+
+		/*
+		 * We need to cut out '#' and everything after '#'
+		 * to get the parent dataset name only.
+		 */
+		assert(bookp - path < sizeof (dsname));
+		(void) strncpy(dsname, path, bookp - path);
+		dsname[bookp - path] = '\0';
+
+		/*
+		 * Create handle for the parent dataset.
+		 */
+		errno = 0;
+		if ((pzhp = make_dataset_handle(hdl, dsname)) == NULL) {
+			(void) zfs_standard_error(hdl, errno, errbuf);
+			return (NULL);
+		}
+
+		/*
+		 * Iterate bookmarks to find the right one.
+		 */
+		errno = 0;
+		if ((zfs_iter_bookmarks(pzhp, zfs_open_bookmarks_cb,
+		    &cb_data) == 0) && (cb_data.zhp == NULL)) {
+			(void) zfs_error(hdl, EZFS_NOENT, errbuf);
+			zfs_close(pzhp);
+			return (NULL);
+		}
+		if (cb_data.zhp == NULL) {
+			(void) zfs_standard_error(hdl, errno, errbuf);
+			zfs_close(pzhp);
+			return (NULL);
+		}
+		zhp = cb_data.zhp;
+
+		/*
+		 * Cleanup.
+		 */
+		zfs_close(pzhp);
 	}
 
 	if (!(types & zhp->zfs_type)) {
diff --git a/usr/src/lib/libzfs/common/libzfs_pool.c b/usr/src/lib/libzfs/common/libzfs_pool.c
index e312a03e43..2c20ba8ffd 100644
--- a/usr/src/lib/libzfs/common/libzfs_pool.c
+++ b/usr/src/lib/libzfs/common/libzfs_pool.c
@@ -946,9 +946,10 @@ zpool_name_valid(libzfs_handle_t *hdl, boolean_t isopen, const char *pool)
 				    "trailing slash in name"));
 				break;
 
-			case NAME_ERR_MULTIPLE_AT:
+			case NAME_ERR_MULTIPLE_DELIMITERS:
 				zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
-				    "multiple '@' delimiters in name"));
+				    "multiple '@' and/or '#' delimiters in "
+				    "name"));
 				break;
 
 			default:
diff --git a/usr/src/lib/libzfs/common/libzfs_util.c b/usr/src/lib/libzfs/common/libzfs_util.c
index 7dd587a537..b8a16a08d4 100644
--- a/usr/src/lib/libzfs/common/libzfs_util.c
+++ b/usr/src/lib/libzfs/common/libzfs_util.c
@@ -689,7 +689,7 @@ zfs_get_pool_handle(const zfs_handle_t *zhp)
  * Given a name, determine whether or not it's a valid path
  * (starts with '/' or "./").  If so, walk the mnttab trying
  * to match the device number.  If not, treat the path as an
- * fs/vol/snap name.
+ * fs/vol/snap/bkmark name.
  */
 zfs_handle_t *
 zfs_path_to_zhandle(libzfs_handle_t *hdl, char *path, zfs_type_t argtype)
diff --git a/usr/src/man/man1m/zfs.1m b/usr/src/man/man1m/zfs.1m
index d2edcdb0fa..622de53a7f 100644
--- a/usr/src/man/man1m/zfs.1m
+++ b/usr/src/man/man1m/zfs.1m
@@ -28,7 +28,7 @@
 .\" Copyright (c) 2014 Integros [integros.com]
 .\" Copyright 2016 Nexenta Systems, Inc.
 .\"
-.Dd September 3, 2016
+.Dd September 16, 2016
 .Dt ZFS 1M
 .Os
 .Sh NAME
@@ -112,7 +112,7 @@
 .Oo Fl s Ar source Ns Oo , Ns Ar source Oc Ns ... Oc
 .Oo Fl t Ar type Ns Oo , Ns Ar type Oc Ns ... Oc
 .Cm all | Ar property Ns Oo , Ns Ar property Oc Ns ...
-.Ar filesystem Ns | Ns Ar volume Ns | Ns Ar snapshot Ns ...
+.Ar filesystem Ns | Ns Ar volume Ns | Ns Ar snapshot Ns | Ns Ar bookmark Ns ...
 .Nm
 .Cm inherit
 .Op Fl rS
@@ -2123,7 +2123,7 @@ section.
 .Oo Fl s Ar source Ns Oo , Ns Ar source Oc Ns ... Oc
 .Oo Fl t Ar type Ns Oo , Ns Ar type Oc Ns ... Oc
 .Cm all | Ar property Ns Oo , Ns Ar property Oc Ns ...
-.Ar filesystem Ns | Ns Ar volume Ns | Ns Ar snapshot Ns ...
+.Ar filesystem Ns | Ns Ar volume Ns | Ns Ar snapshot Ns | Ns Ar bookmark Ns ...
 .Xc
 Displays properties for the given datasets. If no datasets are specified, then
 the command displays properties for all datasets on the system. For each
diff --git a/usr/src/test/zfs-tests/include/default.cfg b/usr/src/test/zfs-tests/include/default.cfg
index 8747963bfa..d36b0f3b84 100644
--- a/usr/src/test/zfs-tests/include/default.cfg
+++ b/usr/src/test/zfs-tests/include/default.cfg
@@ -106,6 +106,7 @@ export TESTVOL2=testvol2
 export TESTFILE0=testfile0
 export TESTFILE1=testfile1
 export TESTFILE2=testfile2
+export TESTBKMARK=testbkmark
 
 export LONGPNAME="poolname50charslong_012345678901234567890123456789"
 export LONGFSNAME="fsysname50charslong_012345678901234567890123456789"
diff --git a/usr/src/test/zfs-tests/include/libtest.shlib b/usr/src/test/zfs-tests/include/libtest.shlib
index dc4e3ca3d2..2957cf0808 100644
--- a/usr/src/test/zfs-tests/include/libtest.shlib
+++ b/usr/src/test/zfs-tests/include/libtest.shlib
@@ -237,6 +237,35 @@ function create_clone   # snapshot clone
 	log_must zfs clone $snap $clone
 }
 
+#
+# Create a bookmark of the given snapshot.  Defaultly create a bookmark on
+# filesystem.
+#
+# $1 Existing filesystem or volume name. Default, $TESTFS
+# $2 Existing snapshot name. Default, $TESTSNAP
+# $3 bookmark name. Default, $TESTBKMARK
+#
+function create_bookmark
+{
+	typeset fs_vol=${1:-$TESTFS}
+	typeset snap=${2:-$TESTSNAP}
+	typeset bkmark=${3:-$TESTBKMARK}
+
+	[[ -z $fs_vol ]] && log_fail "Filesystem or volume's name is undefined."
+	[[ -z $snap ]] && log_fail "Snapshot's name is undefined."
+	[[ -z $bkmark ]] && log_fail "Bookmark's name is undefined."
+
+	if bkmarkexists $fs_vol#$bkmark; then
+		log_fail "$fs_vol#$bkmark already exists."
+	fi
+	datasetexists $fs_vol || \
+		log_fail "$fs_vol must exist."
+	snapexists $fs_vol@$snap || \
+		log_fail "$fs_vol@$snap must exist."
+
+	log_must $ZFS bookmark $fs_vol@$snap $fs_vol#$bkmark
+}
+
 function default_mirror_setup
 {
 	default_mirror_setup_noexit $1 $2 $3
@@ -515,6 +544,23 @@ function destroy_clone
 		log_must rm -rf $mtpt
 }
 
+#
+# Common function used to cleanup bookmark of file system or volume.  Default
+# to delete the file system's bookmark.
+#
+# $1 bookmark name
+#
+function destroy_bookmark
+{
+	typeset bkmark=${1:-$TESTPOOL/$TESTFS#$TESTBKMARK}
+
+	if ! bkmarkexists $bkmark; then
+		log_fail "'$bkmarkp' does not existed."
+	fi
+
+	log_must $ZFS destroy $bkmark
+}
+
 # Return 0 if a snapshot exists; $? otherwise
 #
 # $1 - snapshot name
@@ -526,6 +572,17 @@ function snapexists
 }
 
 #
+# Return 0 if a bookmark exists; $? otherwise
+#
+# $1 - bookmark name
+#
+function bkmarkexists
+{
+	$ZFS list -H -t bookmark "$1" > /dev/null 2>&1
+	return $?
+}
+
+#
 # Set a property to a certain value on a dataset.
 # Sets a property of the dataset to the value as passed in.
 # @param:
diff --git a/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_001_pos.ksh b/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_001_pos.ksh
index c7eefbc5d8..d12b1dc9a5 100644
--- a/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_001_pos.ksh
+++ b/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_001_pos.ksh
@@ -38,7 +38,7 @@
 # correct property value.
 #
 # STRATEGY:
-# 1. Create pool, filesystem, volume and snapshot.
+# 1. Create pool, filesystem, volume, snapshot, and bookmark.
 # 2. Setting valid parameter, 'zfs get' should succeed.
 # 3. Compare the output property name with the original input property.
 #
@@ -69,6 +69,9 @@ typeset all_props=("${zfs_props[@]}" "${userquota_props[@]}")
 typeset dataset=($TESTPOOL/$TESTCTR $TESTPOOL/$TESTFS $TESTPOOL/$TESTVOL \
 	$TESTPOOL/$TESTFS@$TESTSNAP $TESTPOOL/$TESTVOL@$TESTSNAP)
 
+typeset bookmark_props=(creation)
+typeset bookmark=($TESTPOOL/$TESTFS#$TESTBKMARK $TESTPOOL/$TESTVOL#$TESTBKMARK)
+
 #
 # According to dataset and option, checking if 'zfs get' return correct
 # property information.
@@ -115,6 +118,10 @@ log_onexit cleanup
 create_snapshot $TESTPOOL/$TESTFS $TESTSNAP
 create_snapshot $TESTPOOL/$TESTVOL $TESTSNAP
 
+# Create filesystem and volume's bookmark
+create_bookmark $TESTPOOL/$TESTFS $TESTSNAP $TESTBKMARK
+create_bookmark $TESTPOOL/$TESTVOL $TESTSNAP $TESTBKMARK
+
 typeset -i i=0
 while ((i < ${#dataset[@]})); do
 	for opt in "${options[@]}"; do
@@ -131,5 +138,21 @@ while ((i < ${#dataset[@]})); do
 	((i += 1))
 done
 
+i=0
+while ((i < ${#bookmark[@]})); do
+	for opt in "${options[@]}"; do
+		for prop in ${bookmark_props[@]}; do
+			eval "$ZFS get $opt $prop ${bookmark[i]} > \
+			    $TESTDIR/$TESTFILE0"
+			ret=$?
+			if [[ $ret != 0 ]]; then
+				log_fail "$ZFS get returned: $ret"
+			fi
+			check_return_value ${bookmark[i]} "$prop" "$opt"
+		done
+	done
+	((i += 1))
+done
+
 log_pass "Setting the valid options to dataset, it should succeed and return " \
     "valid value. 'zfs get' pass."
diff --git a/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_005_neg.ksh b/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_005_neg.ksh
index 701989695c..0d70fba54e 100644
--- a/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_005_neg.ksh
+++ b/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_005_neg.ksh
@@ -70,6 +70,9 @@ val_props_str=$(gen_option_str "${val_props[*]}" "" "," $prop_numb)
 inval_opts_str=$(gen_option_str "${inval_opts[*]}" "-" "" $opt_numb)
 inval_props_str=$(gen_option_str "${inval_props[*]}" "" "," $prop_numb)
 
+typeset val_bookmark_props=(creation)
+typeset bookmark=($TESTPOOL/$TESTFS#$TESTBKMARK $TESTPOOL/$TESTVOL#$TESTBKMARK)
+
 #
 # Test different options and properties combination.
 #
@@ -95,6 +98,31 @@ function test_options
 	done
 }
 
+#
+# Test different options and properties combination for bookmarks.
+#
+# $1 options
+# $2 properties
+#
+function test_options_bookmarks
+{
+	typeset opts=$1
+	typeset props=$2
+
+	for dst in ${bookmark[@]}; do
+		for opt in $opts; do
+			for prop in $props; do
+				$ZFS get $opt -- $prop $dst > /dev/null 2>&1
+				ret=$?
+				if [[ $ret == 0 ]]; then
+					log_fail "$ZFS get $opt -- $prop " \
+					    "$dst unexpectedly succeeded."
+				fi
+			done
+		done
+	done
+}
+
 log_assert "Setting the invalid option and properties, 'zfs get' should be \
     failed."
 log_onexit cleanup
@@ -103,13 +131,20 @@ log_onexit cleanup
 create_snapshot $TESTPOOL/$TESTFS $TESTSNAP
 create_snapshot $TESTPOOL/$TESTVOL $TESTSNAP
 
+# Create filesystem and volume's bookmark
+create_bookmark $TESTPOOL/$TESTFS $TESTSNAP $TESTBKMARK
+create_bookmark $TESTPOOL/$TESTVOL $TESTSNAP $TESTBKMARK
+
 log_note "Valid options + invalid properties, 'zfs get' should fail."
 test_options "$val_opts_str" "$inval_props_str"
+test_options_bookmark "$val_opts_str" "$inval_props_str"
 
 log_note "Invalid options + valid properties, 'zfs get' should fail."
 test_options "$inval_opts_str" "$val_props_str"
+test_options_bookmark "$inval_opts_str" "$val_bookmark_props"
 
 log_note "Invalid options + invalid properties, 'zfs get' should fail."
 test_options "$inval_opts_str" "$inval_props_str"
+test_options_bookmarks "$inval_opts_str" "$inval_props_str"
 
 log_pass "Setting the invalid options to dataset, 'zfs get' pass."
diff --git a/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_008_pos.ksh b/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_008_pos.ksh
index 976beb7ab9..b81a974370 100644
--- a/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_008_pos.ksh
+++ b/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_008_pos.ksh
@@ -37,7 +37,7 @@
 # Verify "-d <n>" can work with other options
 #
 # STRATEGY:
-# 1. Create pool, filesystem, dataset, volume and snapshot.
+# 1. Create pool, filesystem, dataset, volume, snapshot, and bookmark.
 # 2. Getting an -d option, other options and properties random combination.
 # 3. Using the combination as the parameters of 'zfs get' to check the
 # command line return value.
@@ -65,6 +65,9 @@ fi
 set -A dataset $TESTPOOL/$TESTCTR $TESTPOOL/$TESTFS $TESTPOOL/$TESTVOL \
 	$TESTPOOL/$TESTFS@$TESTSNAP $TESTPOOL/$TESTVOL@$TESTSNAP
 
+set -A bookmark_props creation
+set -A bookmark $TESTPOOL/$TESTFS#$TESTBKMARK $TESTPOOL/$TESTVOL#$TESTBKMARK
+
 log_assert "Verify '-d <n>' can work with other options"
 log_onexit cleanup
 
@@ -72,6 +75,10 @@ log_onexit cleanup
 create_snapshot $TESTPOOL/$TESTFS $TESTSNAP
 create_snapshot $TESTPOOL/$TESTVOL $TESTSNAP
 
+# Create filesystem and volume's bookmark
+create_bookmark $TESTPOOL/$TESTFS $TESTSNAP $TESTBKMARK
+create_bookmark $TESTPOOL/$TESTVOL $TESTSNAP $TESTBKMARK
+
 typeset -i opt_numb=16
 typeset -i prop_numb=16
 typeset -i i=0
@@ -91,5 +98,15 @@ for dst in ${dataset[@]}; do
 	done
 done
 
+for dst in ${bookmark[@]}; do
+	(( i=0 ))
+	while (( i < opt_numb )); do
+		(( item = $RANDOM % ${#options[@]} ))
+		(( depth_item = $RANDOM % ${#depth_options[@]} ))
+		log_must eval "$ZFS get -${depth_options[depth_item]} ${options[item]} $bookmark_props $dst > /dev/null 2>&1"
+		(( i += 1 ))
+	done
+done
+
 log_pass "Verify '-d <n>' can work with other options"
 
diff --git a/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_common.kshlib b/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_common.kshlib
index 6e05175aee..d8cb9af028 100644
--- a/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_common.kshlib
+++ b/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_common.kshlib
@@ -87,8 +87,8 @@ function gen_option_str # $elements $prefix $separator $counter
 }
 
 #
-# Cleanup the volume snapshot and filesystem snapshot were created for
-# this test case.
+# Cleanup the volume snapshot, filesystem snapshot, volume bookmark, and
+# filesystem bookmark that were created for this test case.
 #
 function cleanup
 {
@@ -97,5 +97,10 @@ function cleanup
 	datasetexists $TESTPOOL/$TESTFS@$TESTSNAP && \
 		destroy_snapshot $TESTPOOL/$TESTFS@$TESTSNAP
 
+	bkmarkexists $TESTPOOL/$TESTVOL#$TESTBKMARK && \
+		destroy_bookmark $TESTPOOL/$TESTVOL#$TESTBKMARK
+	bkmarkexists $TESTPOOL/$TESTFS#$TESTBKMARK && \
+		destroy_bookmark $TESTPOOL/$TESTFS#$TESTBKMARK
+
 	[[ -e $TESTFILE0 ]] && log_must rm $TESTFILE0
 }