OpenZFS 8677 - Open-Context Channel Programs

Authored by: Serapheim Dimitropoulos <[email protected]>
Reviewed by: Matt Ahrens <[email protected]>
Reviewed by: Chris Williamson <[email protected]>
Reviewed by: Pavel Zakharov <[email protected]>
Approved by: Robert Mustacchi <[email protected]>
Ported-by: Don Brady <[email protected]>

We want to be able to run channel programs outside of synching
context. This would greatly improve performance for channel programs
that just gather information, as they won't have to wait for synching
context anymore.

=== What is implemented?

This feature introduces the following:
- A new command line flag in "zfs program" to specify our intention
  to run in open context. (The -n option)
- A new flag/option within the channel program ioctl which selects
  the context.
- Appropriate error handling whenever we try a channel program in
  open-context that contains zfs.sync* expressions.
- Documentation for the new feature in the manual pages.

=== How do we handle zfs.sync functions in open context?

When such a function is found by the interpreter and we are running
in open context we abort the script and we spit out a descriptive
runtime error. For example, given the script below ...

arg = ...
fs = arg["argv"][1]
err = zfs.sync.destroy(fs)
msg = "destroying " .. fs .. " err=" .. err
return msg

if we run it in open context, we will get back the following error:

Channel program execution failed:
[string "channel program"]:3: running functions from the zfs.sync
submodule requires passing sync=TRUE to lzc_channel_program()
(i.e. do not specify the "-n" command line argument)
stack traceback:
            [C]: in function 'destroy'
            [string "channel program"]:3: in main chunk

=== What about testing?

We've introduced new wrappers for all channel program tests that
run each channel program as both (startard & open-context) and
expect the appropriate behavior depending on the program using
the zfs.sync module.

OpenZFS-issue: https://www.illumos.org/issues/8677
OpenZFS-commit: https://github.com/openzfs/openzfs/commit/17a49e15
Closes #6558

14 files changed, 183 insertions, 41 deletions

diff --git a/tests/zfs-tests/tests/functional/channel_program/channel_common.kshlib b/tests/zfs-tests/tests/functional/channel_program/channel_common.kshlib
index f3097940f..980d7e9d3 100644
--- a/tests/zfs-tests/tests/functional/channel_program/channel_common.kshlib
+++ b/tests/zfs-tests/tests/functional/channel_program/channel_common.kshlib
@@ -11,22 +11,31 @@
 #
 
 #
-# Copyright (c) 2016 by Delphix. All rights reserved.
+# Copyright (c) 2016, 2017 by Delphix. All rights reserved.
 #
 
 . $STF_SUITE/include/libtest.shlib
 
 ZCP_ROOT=$STF_SUITE/tests/functional/channel_program
 
-# <exitcode> <expected error string> <zfs program args>
-# e.g. log_program 0 $POOL foo.zcp arg1 arg2
+#
+# Note: In case of failure (log_fail) in this function
+# we delete the file passed as <input file> so the
+# test suite doesn't leak temp files on failures. So it
+# is expected that <input file> is a temp file and not
+# an installed file.
+#
+# <exitcode> <expected error string> <input file> <zfs program args>
+# e.g. log_program 0 "" tmp.7a12V $POOL foo.zcp arg1 arg2
 function log_program
 {
 	typeset expectexit=$1
 	shift
 	typeset expecterror=$1
 	shift
-	typeset cmdargs=$@ tmpout=$(mktemp) tmperr=$(mktemp) tmpin=$(mktemp)
+	typeset tmpin=$1
+	shift
+	typeset cmdargs=$@ tmpout=$(mktemp) tmperr=$(mktemp)
 
 	# Expected output/error filename is the same as the .zcp name
 	typeset basename
@@ -36,65 +45,195 @@ function log_program
 
 	log_note "running: zfs program $cmdargs:"
 
-	tee $tmpin | zfs program $cmdargs >$tmpout 2>$tmperr
+	zfs program $cmdargs >$tmpout 2>$tmperr
 	typeset ret=$?
 
 	log_note "input:\n$(cat $tmpin)"
 	log_note "output:\n$(cat $tmpout)"
 	log_note "error:\n$(cat $tmperr)"
-	# verify correct return value
+
+	#
+	# Verify correct return value
+	#
 	if [[ $ret -ne $expectexit ]]; then
+		rm $tmpout $tmperr $tmpin
 		log_fail "return mismatch: expected $expectexit, got $ret"
 	fi
 
 	#
 	# Check the output or reported error for successful or error returns,
 	# respectively.
+	#
 	if [[ -f "$basename.out" ]] && [[ $expectexit -eq 0 ]]; then
 
 		outdiff=$(diff "$basename.out" "$tmpout")
-		[[ $? -ne 0 ]] && log_fail "Output mismatch. Expected:\n" \
-		    "$(cat $basename.out)\nBut got:$(cat $tmpout)\n" \
-		    "Diff:\n$outdiff"
+		if [[ $? -ne 0 ]]; then
+			output=$(cat $tmpout)
+			rm $tmpout $tmperr $tmpin
+			log_fail "Output mismatch. Expected:\n" \
+				"$(cat $basename.out)\nBut got:\n$output\n" \
+				"Diff:\n$outdiff"
+		fi
 
 	elif [[ -f "$basename.err" ]] && [[ $expectexit -ne 0 ]]; then
 
 		outdiff=$(diff "$basename.err" "$tmperr")
-		[[ $? -ne 0 ]] && log_fail "Error mismatch. Expected:\n" \
-		    "$(cat $basename.err)\nBut got:$(cat $tmpout)\n" \
-		    "Diff:\n$outdiff"
+		if [[ $? -ne 0 ]]; then
+			outputerror=$(cat $tmperr)
+			rm $tmpout $tmperr $tmpin
+			log_fail "Error mismatch. Expected:\n" \
+				"$(cat $basename.err)\nBut got:\n$outputerror\n" \
+				"Diff:\n$outdiff"
+		fi
 
 	elif [[ -n $expecterror ]] && [[ $expectexit -ne 0 ]]; then
 
-		grep -q "$expecterror" $tmperr || \
-		    log_fail "Error mismatch. Expected to contain:\n" \
-		    "$expecterror\nBut got:$(cat $tmpout)\n"
+		grep -q "$expecterror" $tmperr
+		if [[ $? -ne 0 ]]; then
+			outputerror=$(cat $tmperr)
+			rm $tmpout $tmperr $tmpin
+			log_fail "Error mismatch. Expected to contain:\n" \
+				"$expecterror\nBut got:\n$outputerror\n"
+		fi
 
 	elif [[ $expectexit -ne 0 ]]; then
 		#
 		# If there's no expected output, error reporting is allowed to
 		# vary, but ensure that we didn't fail silently.
 		#
-		[[ -z "$(cat $tmperr)" ]] && \
-		    log_fail "error with no stderr output"
+		if [[ -z "$(cat $tmperr)" ]]; then
+			rm $tmpout $tmperr $tmpin
+			log_fail "error with no stderr output"
+		fi
 	fi
 
+	#
+	# Clean up all temp files except $tmpin which is
+	# reused for the second invocation of log_program.
+	#
 	rm $tmpout $tmperr
 }
 
+#
+# Even though the command's arguments are passed correctly
+# to the log_must_program family of wrappers the majority
+# of the time, zcp scripts passed as HERE documents can
+# make things trickier (see comment within the function
+# below) in the ordering of the commands arguments and how
+# they are passed. Thus, with this function we reconstruct
+# them to ensure that they are passed properly.
+#
+function log_program_construct_args
+{
+	typeset tmpin=$1
+	shift
+
+	args=""
+	i=0
+	while getopts "nt:m:" opt; do
+		case $opt in
+			t) args="$args -t $OPTARG"; i=$(($i + 2)) ;;
+			m) args="$args -m $OPTARG"; i=$(($i + 2)) ;;
+			n) args="$args -n"; i=$(($i + 1)) ;;
+		esac
+	done
+	shift $i
+
+	pool=$1
+	shift
+
+	#
+	# Catch HERE document if it exists and save it within our
+	# temp file. The reason we do this is that since the
+	# log_must_program wrapper calls zfs-program twice (once
+	# for open context and once for syncing) the HERE doc
+	# is consumed in the first invocation and the second one
+	# does not have a program to run.
+	#
+	test -s /dev/stdin && cat > $tmpin
+
+	#
+	# If $tmpin has contents it means that we consumed a HERE
+	# doc and $1 currently holds "-" (a dash). If there is no
+	# HERE doc and $tmpin is empty, then we copy the contents
+	# of the original channel program to $tmpin.
+	#
+	[[ -s $tmpin ]] || cp $1 $tmpin
+	shift
+
+	lua_args=$@
+
+	echo "$args $pool $tmpin $lua_args"
+}
+
+#
+# Program should complete successfully
+# when run in either context.
+#
 function log_must_program
 {
-	log_program 0 "" "$@"
+	typeset tmpin=$(mktemp)
+
+	program_args=$(log_program_construct_args $tmpin $@)
+
+	log_program 0 "" $tmpin "-n $program_args"
+	log_program 0 "" $tmpin "$program_args"
+
+	rm $tmpin
+}
+#
+# Program should error as expected in
+# the same way in both contexts.
+#
+function log_mustnot_checkerror_program
+{
+	typeset expecterror=$1
+	shift
+	typeset tmpin=$(mktemp)
+
+	program_args=$(log_program_construct_args $tmpin $@)
+
+	log_program 1 "$expecterror" $tmpin "-n $program_args"
+	log_program 1 "$expecterror" $tmpin "$program_args"
+
+	rm $tmpin
 }
 
+#
+# Program should fail when run in either
+# context.
+#
 function log_mustnot_program
 {
-	log_program 1 "" "$@"
+	log_mustnot_checkerror_program "" $@
 }
 
-function log_mustnot_checkerror_program
+
+#
+# Program should error as expected in
+# open context but complete successfully
+# in syncing context.
+#
+function log_mustnot_checkerror_program_open
 {
 	typeset expecterror=$1
 	shift
-	log_program 1 "$expecterror" "$@"
+	typeset tmpin=$(mktemp)
+
+	program_args=$(log_program_construct_args $tmpin $@)
+
+	log_program 1 "$expecterror" $tmpin "-n $program_args"
+	log_program 0 "" $tmpin "$program_args"
+
+	rm $tmpin
+}
+
+#
+# Program should complete successfully
+# when run in syncing context but fail
+# when attempted to run in open context.
+#
+function log_must_program_sync
+{
+	log_mustnot_checkerror_program_open "requires passing sync=TRUE" $@
 }
diff --git a/tests/zfs-tests/tests/functional/channel_program/lua_core/tst.return_large.ksh b/tests/zfs-tests/tests/functional/channel_program/lua_core/tst.return_large.ksh
index 369ad846a..ba9c40739 100755
--- a/tests/zfs-tests/tests/functional/channel_program/lua_core/tst.return_large.ksh
+++ b/tests/zfs-tests/tests/functional/channel_program/lua_core/tst.return_large.ksh
@@ -47,7 +47,7 @@ output_lines=$(log_must zfs program $TESTPOOL \
 #
 # Make sure we fail if the return is over the memory limit
 #
-log_mustnot_program $TESTPOOL -m 10000 \
+log_mustnot_program -m 10000 $TESTPOOL \
     $ZCP_ROOT/lua_core/tst.return_large.zcp
 
 log_pass "Large return values work properly"
diff --git a/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.destroy_fs.ksh b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.destroy_fs.ksh
index c9a2e64b7..b2e0e600e 100755
--- a/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.destroy_fs.ksh
+++ b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.destroy_fs.ksh
@@ -11,7 +11,7 @@
 #
 
 #
-# Copyright (c) 2016 by Delphix. All rights reserved.
+# Copyright (c) 2016, 2017 by Delphix. All rights reserved.
 #
 
 verify_runnable "global"
@@ -32,7 +32,7 @@ log_must zfs unmount $fs
 
 log_must datasetexists $fs
 
-log_must_program $TESTPOOL - $fs <<-EOF
+log_must_program_sync $TESTPOOL - $fs <<-EOF
 	arg = ...
 	fs = arg["argv"][1]
 	err = zfs.sync.destroy(fs)
diff --git a/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.destroy_snap.ksh b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.destroy_snap.ksh
index d74b47a1d..0205e6c11 100755
--- a/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.destroy_snap.ksh
+++ b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.destroy_snap.ksh
@@ -11,7 +11,7 @@
 #
 
 #
-# Copyright (c) 2016 by Delphix. All rights reserved.
+# Copyright (c) 2016, 2017 by Delphix. All rights reserved.
 #
 
 verify_runnable "global"
@@ -31,7 +31,7 @@ create_snapshot $TESTPOOL/$TESTFS $TESTSNAP
 
 log_must snapexists $snap
 
-log_must_program $TESTPOOL - $snap <<-EOF
+log_must_program_sync $TESTPOOL - $snap <<-EOF
 	arg = ...
 	snap = arg["argv"][1]
 	err = zfs.sync.destroy(snap)
diff --git a/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.parse_args_neg.ksh b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.parse_args_neg.ksh
index 8c57bef4b..01bdb63eb 100755
--- a/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.parse_args_neg.ksh
+++ b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.parse_args_neg.ksh
@@ -11,7 +11,7 @@
 #
 
 #
-# Copyright (c) 2016 by Delphix. All rights reserved.
+# Copyright (c) 2016, 2017 by Delphix. All rights reserved.
 #
 
 . $STF_SUITE/tests/functional/channel_program/channel_common.kshlib
@@ -40,8 +40,10 @@ set -A progs "zfs.sync.destroy(\"foo\", \"bar\")" \
 typeset -i i=0
 while (( i < ${#progs[*]} )); do
 	log_note "running program: ${progs[i]}"
-	# output should contain the usage message, which starts with "destroy{"
-	echo ${progs[i]} | log_mustnot_checkerror_program "destroy{" $TESTPOOL -
+	# output should contain the usage message, which contains "destroy{"
+	log_mustnot_checkerror_program "destroy{" $TESTPOOL - <<-EOF
+		${progs[i]}
+	EOF
 	((i = i + 1))
 done
 
diff --git a/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.promote_conflict.ksh b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.promote_conflict.ksh
index d5d377b77..0739940da 100755
--- a/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.promote_conflict.ksh
+++ b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.promote_conflict.ksh
@@ -11,7 +11,7 @@
 #
 
 #
-# Copyright (c) 2016 by Delphix. All rights reserved.
+# Copyright (c) 2016, 2017 by Delphix. All rights reserved.
 #
 
 . $STF_SUITE/tests/functional/channel_program/channel_common.kshlib
@@ -49,7 +49,7 @@ log_must zfs snapshot $clone@$snap
 # code and description, which should be EEXIST (17) and the name of the
 # conflicting snapshot.
 #
-log_must_program $TESTPOOL \
+log_must_program_sync $TESTPOOL \
     $ZCP_ROOT/synctask_core/tst.promote_conflict.zcp $clone
 
 log_pass "Promoting a clone with a conflicting snapshot fails."
diff --git a/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.promote_multiple.ksh b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.promote_multiple.ksh
index 13ed119ec..a3c05fabd 100755
--- a/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.promote_multiple.ksh
+++ b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.promote_multiple.ksh
@@ -11,7 +11,7 @@
 #
 
 #
-# Copyright (c) 2016 by Delphix. All rights reserved.
+# Copyright (c) 2016, 2017 by Delphix. All rights reserved.
 #
 
 . $STF_SUITE/tests/functional/channel_program/channel_common.kshlib
@@ -62,7 +62,7 @@ log_must zfs clone $snap2 $clone2
 
 log_must zfs unmount -f $clone1
 
-log_must_program $TESTPOOL - <<-EOF
+log_must_program_sync $TESTPOOL - <<-EOF
     assert(zfs.sync.promote("$clone2") == 0)
     assert(zfs.sync.promote("$clone2") == 0)
     assert(zfs.sync.destroy("$clone1") == 0)
diff --git a/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.promote_simple.ksh b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.promote_simple.ksh
index 4d1ac5974..83685c4dc 100755
--- a/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.promote_simple.ksh
+++ b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.promote_simple.ksh
@@ -11,7 +11,7 @@
 #
 
 #
-# Copyright (c) 2016 by Delphix. All rights reserved.
+# Copyright (c) 2016, 2017 by Delphix. All rights reserved.
 #
 
 . $STF_SUITE/tests/functional/channel_program/channel_common.kshlib
@@ -40,7 +40,7 @@ log_must zfs create $fs
 log_must zfs snapshot $snap
 log_must zfs clone $snap $clone
 
-log_must_program $TESTPOOL - <<-EOF
+log_must_program_sync $TESTPOOL - <<-EOF
     assert(zfs.sync.promote("$clone") == 0)
 EOF
 
diff --git a/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.rollback_mult.ksh b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.rollback_mult.ksh
index 778abc09d..e0b917a32 100755
--- a/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.rollback_mult.ksh
+++ b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.rollback_mult.ksh
@@ -39,7 +39,7 @@ log_must snapexists $snap1
 log_must snapexists $snap2
 log_must zfs unmount $fs
 
-log_must_program $TESTPOOL - $fs $snap2 <<-EOF
+log_must_program_sync $TESTPOOL - $fs $snap2 <<-EOF
 	arg = ...
 	fs = arg["argv"][1]
 	snap = arg["argv"][2]
diff --git a/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.rollback_one.ksh b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.rollback_one.ksh
index 2a8e83913..d6396f488 100755
--- a/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.rollback_one.ksh
+++ b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.rollback_one.ksh
@@ -37,7 +37,7 @@ log_must rm $file
 log_must snapexists $snap
 log_must zfs unmount $fs
 
-log_must_program $TESTPOOL - $fs <<-EOF
+log_must_program_sync $TESTPOOL - $fs <<-EOF
 	arg = ...
 	fs = arg["argv"][1]
 	err = zfs.sync.rollback(fs)
diff --git a/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.snapshot_destroy.ksh b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.snapshot_destroy.ksh
index 98317db6e..3691492bf 100755
--- a/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.snapshot_destroy.ksh
+++ b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.snapshot_destroy.ksh
@@ -33,7 +33,7 @@ log_onexit cleanup
 
 log_must zfs create $fs
 
-log_must_program $TESTPOOL \
+log_must_program_sync $TESTPOOL \
     $ZCP_ROOT/synctask_core/tst.snapshot_destroy.zcp $fs
 
 log_pass "Creating/destroying snapshots in one channel program works"
diff --git a/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.snapshot_neg.ksh b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.snapshot_neg.ksh
index c3585c938..c20ed8a61 100755
--- a/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.snapshot_neg.ksh
+++ b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.snapshot_neg.ksh
@@ -38,7 +38,8 @@ log_must zfs create $fs1
 log_must zfs create $fs2
 log_must zfs snapshot $fs1@snap1
 
-log_must_program $TESTPOOL $ZCP_ROOT/synctask_core/tst.snapshot_neg.zcp $fs1 $fs2
+log_must_program_sync $TESTPOOL \
+    $ZCP_ROOT/synctask_core/tst.snapshot_neg.zcp $fs1 $fs2
 
 log_pass "zfs.sync.snapshot returns correct errors on invalid input"
 
diff --git a/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.snapshot_recursive.ksh b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.snapshot_recursive.ksh
index c0180d90c..f112bf0d2 100755
--- a/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.snapshot_recursive.ksh
+++ b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.snapshot_recursive.ksh
@@ -46,7 +46,7 @@ for fs in $filesystems; do
     log_must zfs create $fs
 done
 
-log_must_program $TESTPOOL \
+log_must_program_sync $TESTPOOL \
     $ZCP_ROOT/synctask_core/tst.snapshot_recursive.zcp $rootfs $snapname
 
 #
diff --git a/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.snapshot_simple.ksh b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.snapshot_simple.ksh
index e818ce9fc..a26234278 100755
--- a/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.snapshot_simple.ksh
+++ b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.snapshot_simple.ksh
@@ -34,7 +34,7 @@ log_onexit cleanup
 
 log_must zfs create $fs
 
-log_must_program $TESTPOOL \
+log_must_program_sync $TESTPOOL \
     $ZCP_ROOT/synctask_core/tst.snapshot_simple.zcp $fs $snapname
 
 log_pass "Simple snapshotting works"