Support setting user properties in a channel program

This adds support for setting user properties in a
zfs channel program by adding 'zfs.sync.set_prop'
and 'zfs.check.set_prop' to the ZFS LUA API.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Matt Ahrens <matt@delphix.com>
Co-authored-by: Sara Hartse <sara.hartse@delphix.com>
Contributions-by: Jason King <jason.king@joyent.com>
Signed-off-by: Sara Hartse <sara.hartse@delphix.com>
Signed-off-by: Jason King <jason.king@joyent.com>
Closes #9950

include/sys/Makefile.am

@@ -99,6 +99,7 @@ COMMON_H = \
 	$(top_srcdir)/include/sys/zcp_global.h \
 	$(top_srcdir)/include/sys/zcp_iter.h \
 	$(top_srcdir)/include/sys/zcp_prop.h \
+	$(top_srcdir)/include/sys/zcp_set.h \
 	$(top_srcdir)/include/sys/zfeature.h \
 	$(top_srcdir)/include/sys/zfs_acl.h \
 	$(top_srcdir)/include/sys/zfs_context.h \

include/sys/zcp_set.h

@@ -0,0 +1,44 @@
+/*
+ * CDDL HEADER START
+ *
+ * This file and its contents are supplied under the terms of the
+ * Common Development and Distribution License ("CDDL"), version 1.0.
+ * You may only use this file in accordance with the terms of version
+ * 1.0 of the CDDL.
+ *
+ * A full copy of the text of the CDDL should have accompanied this
+ * source.  A copy of the CDDL is also available via the Internet at
+ * http://www.illumos.org/license/CDDL.
+ *
+ * CDDL HEADER END
+ */
+
+/*
+ * Copyright 2019 Joyent, Inc.
+ */
+
+#ifndef _SYS_ZCP_SET_H
+#define	_SYS_ZCP_SET_H
+
+#include <sys/dmu_tx.h>
+#include <sys/dsl_pool.h>
+
+#ifdef  __cplusplus
+extern "C" {
+#endif
+
+typedef struct zcp_set_prop_arg {
+	lua_State	*state;
+	const char	*dsname;
+	const char	*prop;
+	const char	*val;
+} zcp_set_prop_arg_t;
+
+int zcp_set_prop_check(void *arg, dmu_tx_t *tx);
+void zcp_set_prop_sync(void *arg, dmu_tx_t *tx);
+
+#ifdef  __cplusplus
+}
+#endif
+
+#endif /* _SYS_ZCP_SET_H */

lib/libzpool/Makefile.am

@@ -141,6 +141,7 @@ KERNEL_C = \
 	zcp_get.c \
 	zcp_global.c \
 	zcp_iter.c \
+	zcp_set.c \
 	zcp_synctask.c \
 	zfeature.c \
 	zfs_byteswap.c \

man/man8/zfs-program.8

@@ -12,7 +12,7 @@
 .\" Copyright (c) 2019, 2020 by Christian Schwarz. All Rights Reserved.
 .\" Copyright 2020 Joyent, Inc.
 .\"
-.Dd January 15, 2020
+.Dd February 3, 2020
 .Dt ZFS-PROGRAM 8
 .Os
 .Sh NAME
@@ -155,7 +155,7 @@ can guarantee that it will finish successfully against a similar size system.
 If a channel program attempts to return too large a value, the program will
 fully execute but exit with a nonzero status code and no return value.
 .Pp
-.Em Note:
+.Em Note :
 ZFS API functions do not generate Fatal Errors when correctly invoked, they
 return an error code and the channel program continues executing.
 See the
@@ -408,6 +408,26 @@ filesystem (string)
 .Bd -ragged -compact -offset "xxxx"
 Filesystem to rollback.
 .Ed
+.It Em zfs.sync.set_prop(dataset, property, value)
+Sets the given property on a dataset.
+Currently only user properties are supported.
+Returns 0 if the property was set, or a nonzero error code otherwise.
+.Pp
+dataset (string)
+.Bd -ragged -compact -offset "xxxx"
+The dataset where the property will be set.
+.Ed
+.Pp
+property (string)
+.Bd -ragged -compact -offset "xxxx"
+The property to set.
+Only user properties are supported.
+.Ed
+.Pp
+value (string)
+.Bd -ragged -compact -offset "xxxx"
+The value of the property to be set.
+.Ed
 .It Em zfs.sync.snapshot(dataset)
 Create a snapshot of a filesystem.
 Returns 0 if the snapshot was successfully created,
@@ -455,6 +475,7 @@ The available zfs.check functions are:
 .It Em zfs.check.destroy(dataset, [defer=true|false])
 .It Em zfs.check.promote(dataset)
 .It Em zfs.check.rollback(filesystem)
+.It Em zfs.check.set_property(dataset, property, value)
 .It Em zfs.check.snapshot(dataset)
 .El
 .It Sy zfs.list submodule

module/zfs/Makefile.in

@@ -105,6 +105,7 @@ $(MODULE)-objs += zcp.o
 $(MODULE)-objs += zcp_get.o
 $(MODULE)-objs += zcp_global.o
 $(MODULE)-objs += zcp_iter.o
+$(MODULE)-objs += zcp_set.o
 $(MODULE)-objs += zcp_synctask.o
 $(MODULE)-objs += zfeature.o
 $(MODULE)-objs += zfs_byteswap.o

module/zfs/zcp_set.c

@@ -0,0 +1,100 @@
+/*
+ * CDDL HEADER START
+ *
+ * This file and its contents are supplied under the terms of the
+ * Common Development and Distribution License ("CDDL"), version 1.0.
+ * You may only use this file in accordance with the terms of version
+ * 1.0 of the CDDL.
+ *
+ * A full copy of the text of the CDDL should have accompanied this
+ * source.  A copy of the CDDL is also available via the Internet at
+ * http://www.illumos.org/license/CDDL.
+ *
+ * CDDL HEADER END
+ */
+
+/*
+ * Copyright (c) 2016 by Delphix. All rights reserved.
+ * Copyrigh 2020 Joyent, Inc.
+ */
+
+#include <sys/lua/lua.h>
+#include <sys/lua/lualib.h>
+#include <sys/lua/lauxlib.h>
+
+#include <sys/dsl_prop.h>
+#include <sys/dsl_dir.h>
+#include <sys/dsl_synctask.h>
+#include <sys/dsl_dataset.h>
+#include <sys/zcp.h>
+#include <sys/zcp_set.h>
+#include <sys/zcp_iter.h>
+#include <sys/zcp_global.h>
+#include <sys/zvol.h>
+
+#include <zfs_prop.h>
+
+static void
+zcp_set_user_prop(lua_State *state, dsl_pool_t *dp, const char *dsname,
+    const char *prop_name, const char *prop_val, dmu_tx_t *tx)
+{
+	dsl_dataset_t *ds = zcp_dataset_hold(state, dp, dsname, FTAG);
+	if (ds == NULL)
+		return; /* not reached; zcp_dataset_hold() longjmp'd */
+
+	nvlist_t *nvl = fnvlist_alloc();
+	fnvlist_add_string(nvl, prop_name, prop_val);
+
+	dsl_props_set_sync_impl(ds, ZPROP_SRC_LOCAL, nvl, tx);
+
+	fnvlist_free(nvl);
+	dsl_dataset_rele(ds, FTAG);
+}
+
+int
+zcp_set_prop_check(void *arg, dmu_tx_t *tx)
+{
+	zcp_set_prop_arg_t *args = arg;
+	const char *prop_name = args->prop;
+	dsl_props_set_arg_t dpsa = {
+		.dpsa_dsname = args->dsname,
+		.dpsa_source = ZPROP_SRC_LOCAL,
+	};
+	nvlist_t *nvl = NULL;
+	int ret = 0;
+
+	/*
+	 * Only user properties are currently supported. When non-user
+	 * properties are supported, we will want to use
+	 * zfs_valid_proplist() to verify the properties.
+	 */
+	if (!zfs_prop_user(prop_name)) {
+		return (EINVAL);
+	}
+
+	nvl = fnvlist_alloc();
+	fnvlist_add_string(nvl, args->prop, args->val);
+	dpsa.dpsa_props = nvl;
+
+	ret = dsl_props_set_check(&dpsa, tx);
+	nvlist_free(nvl);
+
+	return (ret);
+}
+
+void
+zcp_set_prop_sync(void *arg, dmu_tx_t *tx)
+{
+	zcp_set_prop_arg_t *args = arg;
+	zcp_run_info_t *ri = zcp_run_info(args->state);
+	dsl_pool_t *dp = ri->zri_pool;
+
+	const char *dsname = args->dsname;
+	const char *prop_name = args->prop;
+	const char *prop_val = args->val;
+
+	if (zfs_prop_user(prop_name)) {
+		zcp_set_user_prop(args->state, dp, dsname, prop_name,
+		    prop_val, tx);
+	}
+}

module/zfs/zcp_synctask.c

@@ -23,6 +23,7 @@
 #include <sys/lua/lauxlib.h>
 
 #include <sys/zcp.h>
+#include <sys/zcp_set.h>
 #include <sys/dsl_dir.h>
 #include <sys/dsl_pool.h>
 #include <sys/dsl_prop.h>
@@ -414,6 +415,44 @@ zcp_synctask_bookmark(lua_State *state, boolean_t sync, nvlist_t *err_details)
 	return (err);
 }
 
+static int zcp_synctask_set_prop(lua_State *, boolean_t, nvlist_t *err_details);
+static zcp_synctask_info_t zcp_synctask_set_prop_info = {
+	.name = "set_prop",
+	.func = zcp_synctask_set_prop,
+	.space_check = ZFS_SPACE_CHECK_RESERVED,
+	.blocks_modified = 2,
+	.pargs = {
+		{ .za_name = "dataset", .za_lua_type = LUA_TSTRING},
+		{ .za_name = "property", .za_lua_type =  LUA_TSTRING},
+		{ .za_name = "value", .za_lua_type =  LUA_TSTRING},
+		{ NULL, 0 }
+	},
+	.kwargs = {
+		{ NULL, 0 }
+	}
+};
+
+static int
+zcp_synctask_set_prop(lua_State *state, boolean_t sync, nvlist_t *err_details)
+{
+	int err;
+	zcp_set_prop_arg_t args = { 0 };
+
+	const char *dsname = lua_tostring(state, 1);
+	const char *prop = lua_tostring(state, 2);
+	const char *val = lua_tostring(state, 3);
+
+	args.state = state;
+	args.dsname = dsname;
+	args.prop = prop;
+	args.val = val;
+
+	err = zcp_sync_task(state, zcp_set_prop_check, zcp_set_prop_sync,
+	    &args, sync, dsname);
+
+	return (err);
+}
+
 static int
 zcp_synctask_wrapper(lua_State *state)
 {
@@ -484,6 +523,7 @@ zcp_load_synctask_lib(lua_State *state, boolean_t sync)
 		&zcp_synctask_snapshot_info,
 		&zcp_synctask_inherit_prop_info,
 		&zcp_synctask_bookmark_info,
+		&zcp_synctask_set_prop_info,
 		NULL
 	};
 

tests/runfiles/common.run

@@ -85,7 +85,7 @@ tests = ['tst.destroy_fs', 'tst.destroy_snap', 'tst.get_count_and_limit',
     'tst.list_snapshots', 'tst.list_system_props',
     'tst.list_user_props', 'tst.parse_args_neg','tst.promote_conflict',
     'tst.promote_multiple', 'tst.promote_simple', 'tst.rollback_mult',
-    'tst.rollback_one', 'tst.snapshot_destroy', 'tst.snapshot_neg',
+    'tst.rollback_one', 'tst.set_props', 'tst.snapshot_destroy', 'tst.snapshot_neg',
     'tst.snapshot_recursive', 'tst.snapshot_simple',
     'tst.bookmark.create', 'tst.bookmark.copy',
     'tst.terminate_by_signal'

tests/zfs-tests/tests/functional/channel_program/synctask_core/Makefile.am

@@ -27,6 +27,7 @@ dist_pkgdata_SCRIPTS = \
 	tst.promote_simple.ksh \
 	tst.rollback_mult.ksh \
 	tst.rollback_one.ksh \
+	tst.set_props.ksh \
 	tst.snapshot_destroy.ksh \
 	tst.snapshot_neg.ksh \
 	tst.snapshot_recursive.ksh \
@@ -43,6 +44,7 @@ dist_pkgdata_DATA = \
 	tst.get_string_props.out \
 	tst.get_string_props.zcp \
 	tst.promote_conflict.zcp \
+	tst.set_props.zcp \
 	tst.snapshot_destroy.zcp \
 	tst.snapshot_neg.zcp \
 	tst.snapshot_recursive.zcp \

tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.set_props.ksh

@@ -0,0 +1,39 @@
+#!/bin/ksh -p
+#
+# This file and its contents are supplied under the terms of the
+# Common Development and Distribution License ("CDDL"), version 1.0.
+# You may only use this file in accordance with the terms of version
+# 1.0 of the CDDL.
+#
+# A full copy of the text of the CDDL should have accompanied this
+# source.  A copy of the CDDL is also available via the Internet at
+# http://www.illumos.org/license/CDDL.
+#
+
+#
+# Copyright (c) 2016 by Delphix. All rights reserved.
+#
+
+. $STF_SUITE/tests/functional/channel_program/channel_common.kshlib
+
+#
+# DESCRIPTION:
+#	Setting user props should work correctly on datasets.
+#
+
+verify_runnable "global"
+
+fs=$TESTPOOL/$TESTFS/testchild
+
+function cleanup
+{
+	destroy_dataset $fs "-R"
+}
+
+log_onexit cleanup
+
+log_must zfs create $fs
+
+log_must_program_sync $TESTPOOL $ZCP_ROOT/synctask_core/tst.set_props.zcp $fs
+
+log_pass "Setting props from channel program works correctly."

tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.set_props.zcp

@@ -0,0 +1,109 @@
+--
+-- This file and its contents are supplied under the terms of the
+-- Common Development and Distribution License ("CDDL"), version 1.0.
+-- You may only use this file in accordance with the terms of version
+-- 1.0 of the CDDL.
+--
+-- A full copy of the text of the CDDL should have accompanied this
+-- source.  A copy of the CDDL is also available via the Internet at
+-- http://www.illumos.org/license/CDDL.
+--
+
+--
+-- Copyright (c) 2017 by Delphix. All rights reserved.
+-- Copyright 2020 Joyent, Inc.
+--
+
+arg = ...
+fs = arg["argv"][1]
+
+-- values from zfs.h
+maxname = 256       -- ZAP_MAXNAMELEN
+maxvalue = 8192     -- ZAP_MAXVALUELEN
+
+pos_props = {}
+neg_props = {}
+
+-- In lua, strings are immutable, so to avoid a bunch of copies, we
+-- build the value in a table and use concat (which appears to be the
+-- recommend method for such things).
+largeprop = {}
+for i = 0,maxvalue,8
+do
+    table.insert(largeprop, "aaaaaaaa")
+end
+-- add an extra character so we spill over the limit
+table.insert(largeprop, "b")
+
+largepropv = table.concat(largeprop)
+
+largepropname = { "b:" }
+for i = 0,maxname,8
+do
+    table.insert(largepropname, "aaaaaaaa")
+end
+largepropnamev = table.concat(largepropname)
+
+pos_props["a:prop"] = {"hello"}
+
+-- For neg_props, an optional expected error value can be added after the
+-- property value as seen below.
+neg_props["notaproperty"] = {"hello", EINVAL}
+neg_props["a:very.long.property.value"] = { largepropv, E2BIG }
+neg_props[largepropnamev] = {"greetings", ENAMETOOLONG }
+
+-- non-user properties aren't currently supported
+-- Even if they were, the argument must be a string due to requirements of
+-- the ZCP api.
+neg_props["mountpoint"] = {"/foo/bar"}
+neg_props["copies"] = { "2" }
+
+-- read-only properties should never succeed
+neg_props["guid"] = { "12345" }
+
+set_fail = {}
+val_fail = {}
+
+-- Test properies that should work
+for prop, values in pairs(pos_props) do
+    for i, val in ipairs(values) do
+        old_val, src = zfs.get_prop(fs, prop)
+
+        -- Attempt to set the property to the specified value
+        err = zfs.sync.set_prop(fs, prop, val)
+
+        if (err ~= 0) then
+            set_fail[prop] = err -- tuple of prop, val that resulted in error
+        else
+            -- use get_prop to check that the set took affect
+            new_val, src = zfs.get_prop(fs, prop)
+            if (tostring(new_val) ~= tostring(val)) then
+                val_fail[prop] = new_val
+            end
+
+            -- We modified the prop, restore old value (if one existed)
+            if (old_val ~= nil) then
+                err = zfs.sync.set_prop(fs, prop, old_val)
+                if (err ~= 0) then return err end
+            else
+                -- Didn't have an old value, delete (inherit) instead
+                err = zfs.sync.inherit(fs, prop)
+                if (err ~= 0) then return err end
+            end
+        end
+    end
+end
+
+-- Test properies that should fail
+for prop, expected in pairs(neg_props) do
+    exp_val = expected[1]
+    exp_err = expected[2]
+
+    -- Attempt to set the property to the specified value
+    err = zfs.sync.set_prop(fs, prop, exp_val)
+    if (err == 0 or (exp_err ~= nil and err ~= exp_err)) then
+        set_fail[prop] = err -- tuple of prop, val that resulted in error
+    end
+end
+
+return {set_fail, val_fail}