From 13b5a4d5c018f94d04efefcec6205aa73205e05f Mon Sep 17 00:00:00 2001 From: Jason King Date: Fri, 14 Feb 2020 15:41:42 -0600 Subject: [PATCH] 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 Reviewed-by: Matt Ahrens Co-authored-by: Sara Hartse Contributions-by: Jason King Signed-off-by: Sara Hartse Signed-off-by: Jason King Closes #9950 --- include/sys/Makefile.am | 1 + include/sys/zcp_set.h | 44 +++++++ lib/libzpool/Makefile.am | 1 + man/man8/zfs-program.8 | 25 +++- module/zfs/Makefile.in | 1 + module/zfs/zcp_set.c | 100 ++++++++++++++++ module/zfs/zcp_synctask.c | 40 +++++++ tests/runfiles/common.run | 2 +- .../channel_program/synctask_core/Makefile.am | 2 + .../synctask_core/tst.set_props.ksh | 39 +++++++ .../synctask_core/tst.set_props.zcp | 109 ++++++++++++++++++ 11 files changed, 361 insertions(+), 3 deletions(-) create mode 100644 include/sys/zcp_set.h create mode 100644 module/zfs/zcp_set.c create mode 100755 tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.set_props.ksh create mode 100644 tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.set_props.zcp diff --git a/include/sys/Makefile.am b/include/sys/Makefile.am index 99e38acb22..bcfa12fca4 100644 --- a/include/sys/Makefile.am +++ b/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 \ diff --git a/include/sys/zcp_set.h b/include/sys/zcp_set.h new file mode 100644 index 0000000000..b7428d6fc0 --- /dev/null +++ b/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 +#include + +#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 */ diff --git a/lib/libzpool/Makefile.am b/lib/libzpool/Makefile.am index f1f56b7049..4225246b96 100644 --- a/lib/libzpool/Makefile.am +++ b/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 \ diff --git a/man/man8/zfs-program.8 b/man/man8/zfs-program.8 index c754fcf99f..41d38587e5 100644 --- a/man/man8/zfs-program.8 +++ b/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 diff --git a/module/zfs/Makefile.in b/module/zfs/Makefile.in index 129c83eb73..1ba7db27b9 100644 --- a/module/zfs/Makefile.in +++ b/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 diff --git a/module/zfs/zcp_set.c b/module/zfs/zcp_set.c new file mode 100644 index 0000000000..cebb56a5f1 --- /dev/null +++ b/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 +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +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); + } +} diff --git a/module/zfs/zcp_synctask.c b/module/zfs/zcp_synctask.c index a6f7a04c71..350b1e8e26 100644 --- a/module/zfs/zcp_synctask.c +++ b/module/zfs/zcp_synctask.c @@ -23,6 +23,7 @@ #include #include +#include #include #include #include @@ -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 }; diff --git a/tests/runfiles/common.run b/tests/runfiles/common.run index dfdc5ee58a..3c71458ebc 100644 --- a/tests/runfiles/common.run +++ b/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' diff --git a/tests/zfs-tests/tests/functional/channel_program/synctask_core/Makefile.am b/tests/zfs-tests/tests/functional/channel_program/synctask_core/Makefile.am index e74c5223b5..4d9aa9cebb 100644 --- a/tests/zfs-tests/tests/functional/channel_program/synctask_core/Makefile.am +++ b/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 \ diff --git a/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.set_props.ksh b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.set_props.ksh new file mode 100755 index 0000000000..6ac1c2b205 --- /dev/null +++ b/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." diff --git a/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.set_props.zcp b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.set_props.zcp new file mode 100644 index 0000000000..eade17aa89 --- /dev/null +++ b/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}