OpenZFS 7386 - zfs get does not work properly with bookmarks

Authored by: Marcel Telka <[email protected]>
Reviewed by: Simon Klinkert <[email protected]>
Reviewed by: Paul Dagnelie <[email protected]>
Approved by: Matthew Ahrens <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Ported-by: George Melikov <[email protected]>

OpenZFS-issue: https://www.illumos.org/issues/7386
OpenZFS-commit: https://github.com/openzfs/openzfs/commit/edb901a
Closes #5666 

6 files changed, 142 insertions, 4 deletions

diff --git a/tests/zfs-tests/include/default.cfg.in b/tests/zfs-tests/include/default.cfg.in
index f93bfa98c..d6913f1f3 100644
--- a/tests/zfs-tests/include/default.cfg.in
+++ b/tests/zfs-tests/include/default.cfg.in
@@ -147,6 +147,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/tests/zfs-tests/include/libtest.shlib b/tests/zfs-tests/include/libtest.shlib
index 4e68ffc3e..199a93503 100644
--- a/tests/zfs-tests/include/libtest.shlib
+++ b/tests/zfs-tests/include/libtest.shlib
@@ -291,6 +291,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
@@ -575,6 +604,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
@@ -586,6 +632,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/tests/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_001_pos.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_001_pos.ksh
index dbac85402..e62482d3f 100755
--- a/tests/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_001_pos.ksh
+++ b/tests/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_001_pos.ksh
@@ -34,7 +34,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.
 #
@@ -65,6 +65,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.
@@ -111,6 +114,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
@@ -127,5 +134,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/tests/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_005_neg.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_005_neg.ksh
index c77f769de..bc4ca2a43 100755
--- a/tests/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_005_neg.ksh
+++ b/tests/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_005_neg.ksh
@@ -67,6 +67,9 @@ val_props_str="$val_props_str -a -d"
 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.
 #
@@ -92,6 +95,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
@@ -100,13 +128,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/tests/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_008_pos.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_008_pos.ksh
index af1b5681a..9d5462562 100755
--- a/tests/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_008_pos.ksh
+++ b/tests/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_008_pos.ksh
@@ -33,7 +33,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.
@@ -61,6 +61,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
 
@@ -68,6 +71,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
@@ -87,5 +94,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/tests/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_common.kshlib b/tests/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_common.kshlib
index b1978cb6c..965686b93 100644
--- a/tests/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_common.kshlib
+++ b/tests/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_common.kshlib
@@ -83,8 +83,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
 {
@@ -93,5 +93,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
 }