diff --git a/cmd/zfs/zfs_main.c b/cmd/zfs/zfs_main.c index cf31624f0326..fce228a20981 100644 --- a/cmd/zfs/zfs_main.c +++ b/cmd/zfs/zfs_main.c @@ -21,7 +21,7 @@ /* * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved. - * Copyright (c) 2011, 2016 by Delphix. All rights reserved. + * Copyright (c) 2011, 2017 by Delphix. All rights reserved. * Copyright 2012 Milan Jurik. All rights reserved. * Copyright (c) 2012, Joyent, Inc. All rights reserved. * Copyright (c) 2013 Steven Hartland. All rights reserved. @@ -340,7 +340,7 @@ get_usage(zfs_help_t idx) case HELP_BOOKMARK: return (gettext("\tbookmark \n")); case HELP_CHANNEL_PROGRAM: - return (gettext("\tprogram [-t ] " + return (gettext("\tprogram [-n] [-t ] " "[-m ] " "[lua args...]\n")); case HELP_LOAD_KEY: @@ -7098,10 +7098,11 @@ zfs_do_channel_program(int argc, char **argv) nvlist_t *outnvl; uint64_t instrlimit = ZCP_DEFAULT_INSTRLIMIT; uint64_t memlimit = ZCP_DEFAULT_MEMLIMIT; + boolean_t sync_flag = B_TRUE; zpool_handle_t *zhp; /* check options */ - while ((c = getopt(argc, argv, "t:m:")) != -1) { + while ((c = getopt(argc, argv, "nt:m:")) != -1) { switch (c) { case 't': case 'm': { @@ -7139,6 +7140,10 @@ zfs_do_channel_program(int argc, char **argv) } break; } + case 'n': { + sync_flag = B_FALSE; + break; + } case '?': (void) fprintf(stderr, gettext("invalid option '%c'\n"), optopt); @@ -7210,8 +7215,13 @@ zfs_do_channel_program(int argc, char **argv) nvlist_t *argnvl = fnvlist_alloc(); fnvlist_add_string_array(argnvl, ZCP_ARG_CLIARGV, argv + 2, argc - 2); - ret = lzc_channel_program(poolname, progbuf, instrlimit, memlimit, - argnvl, &outnvl); + if (sync_flag) { + ret = lzc_channel_program(poolname, progbuf, + instrlimit, memlimit, argnvl, &outnvl); + } else { + ret = lzc_channel_program_nosync(poolname, progbuf, + instrlimit, memlimit, argnvl, &outnvl); + } if (ret != 0) { /* diff --git a/include/libzfs_core.h b/include/libzfs_core.h index b9e4019310c8..721d90f81ca6 100644 --- a/include/libzfs_core.h +++ b/include/libzfs_core.h @@ -20,7 +20,7 @@ */ /* - * Copyright (c) 2012, 2016 by Delphix. All rights reserved. + * Copyright (c) 2012, 2017 by Delphix. All rights reserved. * Copyright (c) 2017 Datto Inc. * Copyright 2017 RackTop Systems. * Copyright (c) 2017 Open-E, Inc. All Rights Reserved. @@ -101,8 +101,10 @@ boolean_t lzc_exists(const char *); int lzc_rollback(const char *, char *, int); int lzc_rollback_to(const char *, const char *); -int lzc_channel_program(const char *, const char *, uint64_t, uint64_t, - nvlist_t *, nvlist_t **); +int lzc_channel_program(const char *, const char *, uint64_t, + uint64_t, nvlist_t *, nvlist_t **); +int lzc_channel_program_nosync(const char *, const char *, uint64_t, + uint64_t, nvlist_t *, nvlist_t **); int lzc_sync(const char *, nvlist_t *, nvlist_t **); int lzc_reopen(const char *, boolean_t); diff --git a/include/sys/fs/zfs.h b/include/sys/fs/zfs.h index 480cecac9e5c..7b86f6631097 100644 --- a/include/sys/fs/zfs.h +++ b/include/sys/fs/zfs.h @@ -21,7 +21,7 @@ /* * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved. - * Copyright (c) 2011, 2016 by Delphix. All rights reserved. + * Copyright (c) 2011, 2017 by Delphix. All rights reserved. * Copyright 2011 Nexenta Systems, Inc. All rights reserved. * Copyright (c) 2013, 2017 Joyent, Inc. All rights reserved. * Copyright (c) 2014 Integros [integros.com] @@ -1204,6 +1204,7 @@ typedef enum { */ #define ZCP_ARG_PROGRAM "program" #define ZCP_ARG_ARGLIST "arg" +#define ZCP_ARG_SYNC "sync" #define ZCP_ARG_INSTRLIMIT "instrlimit" #define ZCP_ARG_MEMLIMIT "memlimit" diff --git a/include/sys/zcp.h b/include/sys/zcp.h index df13bf2302af..b56eda552e17 100644 --- a/include/sys/zcp.h +++ b/include/sys/zcp.h @@ -38,20 +38,25 @@ extern uint64_t zfs_lua_max_memlimit; int zcp_argerror(lua_State *, int, const char *, ...); -int zcp_eval(const char *, const char *, uint64_t, uint64_t, nvpair_t *, - nvlist_t *); +int zcp_eval(const char *, const char *, boolean_t, uint64_t, uint64_t, + nvpair_t *, nvlist_t *); int zcp_load_list_lib(lua_State *); int zcp_load_synctask_lib(lua_State *, boolean_t); typedef void (zcp_cleanup_t)(void *); +typedef struct zcp_cleanup_handler { + zcp_cleanup_t *zch_cleanup_func; + void *zch_cleanup_arg; + list_node_t zch_node; +} zcp_cleanup_handler_t; typedef struct zcp_run_info { dsl_pool_t *zri_pool; /* - * An estimate of the total ammount of space consumed by all + * An estimate of the total amount of space consumed by all * synctasks we have successfully performed so far in this * channel program. Used to generate ENOSPC errors for syncfuncs. */ @@ -89,16 +94,21 @@ typedef struct zcp_run_info { boolean_t zri_timed_out; /* - * The currently registered cleanup function, which will be called - * with the stored argument if a fatal error occurs. + * Boolean indicating whether or not we are running in syncing + * context. */ - zcp_cleanup_t *zri_cleanup; - void *zri_cleanup_arg; + boolean_t zri_sync; + + /* + * List of currently registered cleanup handlers, which will be + * triggered in the event of a fatal error. + */ + list_t zri_cleanup_handlers; } zcp_run_info_t; zcp_run_info_t *zcp_run_info(lua_State *); -void zcp_register_cleanup(lua_State *, zcp_cleanup_t, void *); -void zcp_clear_cleanup(lua_State *); +zcp_cleanup_handler_t *zcp_register_cleanup(lua_State *, zcp_cleanup_t, void *); +void zcp_deregister_cleanup(lua_State *, zcp_cleanup_handler_t *); void zcp_cleanup(lua_State *); /* diff --git a/lib/libzfs/libzfs_dataset.c b/lib/libzfs/libzfs_dataset.c index b9ba719a0c7c..a0e8e7fb6bf1 100644 --- a/lib/libzfs/libzfs_dataset.c +++ b/lib/libzfs/libzfs_dataset.c @@ -22,7 +22,7 @@ /* * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2013, Joyent, Inc. All rights reserved. - * Copyright (c) 2011, 2016 by Delphix. All rights reserved. + * Copyright (c) 2011, 2017 by Delphix. All rights reserved. * Copyright (c) 2012 DEY Storage Systems, Inc. All rights reserved. * Copyright (c) 2012 Pawel Jakub Dawidek . * Copyright (c) 2013 Martin Matuska. All rights reserved. @@ -2506,7 +2506,7 @@ zcp_check(zfs_handle_t *zhp, zfs_prop_t prop, uint64_t intval, fnvlist_add_string(argnvl, "dataset", zhp->zfs_name); fnvlist_add_string(argnvl, "property", zfs_prop_to_name(prop)); - error = lzc_channel_program(poolname, program, + error = lzc_channel_program_nosync(poolname, program, 10 * 1000 * 1000, 10 * 1024 * 1024, argnvl, &outnvl); if (error == 0) { diff --git a/lib/libzfs_core/libzfs_core.c b/lib/libzfs_core/libzfs_core.c index ca0b3cfb5d88..93704223eb6d 100644 --- a/lib/libzfs_core/libzfs_core.c +++ b/lib/libzfs_core/libzfs_core.c @@ -1063,6 +1063,25 @@ lzc_destroy_bookmarks(nvlist_t *bmarks, nvlist_t **errlist) return (error); } +static int +lzc_channel_program_impl(const char *pool, const char *program, boolean_t sync, + uint64_t instrlimit, uint64_t memlimit, nvlist_t *argnvl, nvlist_t **outnvl) +{ + int error; + nvlist_t *args; + + args = fnvlist_alloc(); + fnvlist_add_string(args, ZCP_ARG_PROGRAM, program); + fnvlist_add_nvlist(args, ZCP_ARG_ARGLIST, argnvl); + fnvlist_add_boolean_value(args, ZCP_ARG_SYNC, sync); + fnvlist_add_uint64(args, ZCP_ARG_INSTRLIMIT, instrlimit); + fnvlist_add_uint64(args, ZCP_ARG_MEMLIMIT, memlimit); + error = lzc_ioctl(ZFS_IOC_CHANNEL_PROGRAM, pool, args, outnvl); + fnvlist_free(args); + + return (error); +} + /* * Executes a channel program. * @@ -1100,18 +1119,28 @@ int lzc_channel_program(const char *pool, const char *program, uint64_t instrlimit, uint64_t memlimit, nvlist_t *argnvl, nvlist_t **outnvl) { - int error; - nvlist_t *args; - - args = fnvlist_alloc(); - fnvlist_add_string(args, ZCP_ARG_PROGRAM, program); - fnvlist_add_nvlist(args, ZCP_ARG_ARGLIST, argnvl); - fnvlist_add_uint64(args, ZCP_ARG_INSTRLIMIT, instrlimit); - fnvlist_add_uint64(args, ZCP_ARG_MEMLIMIT, memlimit); - error = lzc_ioctl(ZFS_IOC_CHANNEL_PROGRAM, pool, args, outnvl); - fnvlist_free(args); + return (lzc_channel_program_impl(pool, program, B_TRUE, instrlimit, + memlimit, argnvl, outnvl)); +} - return (error); +/* + * Executes a read-only channel program. + * + * A read-only channel program works programmatically the same way as a + * normal channel program executed with lzc_channel_program(). The only + * difference is it runs exclusively in open-context and therefore can + * return faster. The downside to that, is that the program cannot change + * on-disk state by calling functions from the zfs.sync submodule. + * + * The return values of this function (and their meaning) are exactly the + * same as the ones described in lzc_channel_program(). + */ +int +lzc_channel_program_nosync(const char *pool, const char *program, + uint64_t timeout, uint64_t memlimit, nvlist_t *argnvl, nvlist_t **outnvl) +{ + return (lzc_channel_program_impl(pool, program, B_FALSE, timeout, + memlimit, argnvl, outnvl)); } /* diff --git a/man/man8/zfs-program.8 b/man/man8/zfs-program.8 index b1d937b6e600..8a478b70fa0c 100644 --- a/man/man8/zfs-program.8 +++ b/man/man8/zfs-program.8 @@ -17,7 +17,8 @@ .Nm zfs program .Nd executes ZFS channel programs .Sh SYNOPSIS -.Cm zfs program +.Cm "zfs program" +.Op Fl n .Op Fl t Ar instruction-limit .Op Fl m Ar memory-limit .Ar pool @@ -45,6 +46,14 @@ will be run on and any attempts to access or modify other pools will cause an error. .Sh OPTIONS .Bl -tag -width "-t" +.It Fl n +Executes a read-only channel program, which runs faster. +The program cannot change on-disk state by calling functions from the +zfs.sync submodule. +The program can be used to gather information such as properties and +determining if changes would succeed (zfs.check.*). +Without this flag, all pending changes must be synced to disk before a +channel program can complete. .It Fl t Ar instruction-limit Execution time limit, in number of Lua instructions to execute. If a channel program executes more than the specified number of instructions, diff --git a/man/man8/zfs.8 b/man/man8/zfs.8 index 2cb4a19bbd17..2e1ffc6ea728 100644 --- a/man/man8/zfs.8 +++ b/man/man8/zfs.8 @@ -21,7 +21,7 @@ .\" .\" Copyright (c) 2009 Sun Microsystems, Inc. All Rights Reserved. .\" Copyright 2011 Joshua M. Clulow -.\" Copyright (c) 2011, 2016 by Delphix. All rights reserved. +.\" Copyright (c) 2011, 2017 by Delphix. All rights reserved. .\" Copyright (c) 2013 by Saso Kiselkov. All rights reserved. .\" Copyright (c) 2014, Joyent, Inc. All rights reserved. .\" Copyright (c) 2014 by Adam Stevko. All rights reserved. @@ -272,6 +272,7 @@ .Ar snapshot Ar snapshot Ns | Ns Ar filesystem .Nm .Cm program +.Op Fl n .Op Fl t Ar timeout .Op Fl m Ar memory_limit .Ar pool script @@ -4042,6 +4043,7 @@ Display the path's inode change time as the first column of output. .It Xo .Nm .Cm program +.Op Fl n .Op Fl t Ar timeout .Op Fl m Ar memory_limit .Ar pool script @@ -4063,6 +4065,14 @@ For full documentation of the ZFS channel program interface, see the manual page for .Xr zfs-program 8 . .Bl -tag -width "" +.It Fl n +Executes a read-only channel program, which runs faster. +The program cannot change on-disk state by calling functions from +the zfs.sync submodule. +The program can be used to gather information such as properties and +determining if changes would succeed (zfs.check.*). +Without this flag, all pending changes must be synced to disk before +a channel program can complete. .It Fl t Ar timeout Execution time limit, in milliseconds. If a channel program executes for longer than the provided timeout, it will diff --git a/module/zfs/dsl_destroy.c b/module/zfs/dsl_destroy.c index 7b49de95f798..d0fcacaedc6b 100644 --- a/module/zfs/dsl_destroy.c +++ b/module/zfs/dsl_destroy.c @@ -20,7 +20,7 @@ */ /* * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved. - * Copyright (c) 2012, 2016 by Delphix. All rights reserved. + * Copyright (c) 2012, 2017 by Delphix. All rights reserved. * Copyright (c) 2013 Steven Hartland. All rights reserved. * Copyright (c) 2013 by Joyent, Inc. All rights reserved. * Copyright (c) 2016 Actifio, Inc. All rights reserved. @@ -550,6 +550,7 @@ dsl_destroy_snapshots_nvl(nvlist_t *snaps, boolean_t defer, nvlist_t *result = fnvlist_alloc(); int error = zcp_eval(nvpair_name(nvlist_next_nvpair(snaps, NULL)), program, + B_TRUE, 0, zfs_lua_max_memlimit, nvlist_next_nvpair(wrapper, NULL), result); diff --git a/module/zfs/zcp.c b/module/zfs/zcp.c index 1a39330d363a..31230dc52b0b 100644 --- a/module/zfs/zcp.c +++ b/module/zfs/zcp.c @@ -134,13 +134,6 @@ typedef struct zcp_eval_arg { uint64_t ea_instrlimit; } zcp_eval_arg_t; -/*ARGSUSED*/ -static int -zcp_eval_check(void *arg, dmu_tx_t *tx) -{ - return (0); -} - /* * The outer-most error callback handler for use with lua_pcall(). On * error Lua will call this callback with a single argument that @@ -184,41 +177,45 @@ zcp_argerror(lua_State *state, int narg, const char *msg, ...) * * If an error occurs, the cleanup function will be invoked exactly once and * then unreigstered. + * + * Returns the registered cleanup handler so the caller can deregister it + * if no error occurs. */ -void +zcp_cleanup_handler_t * zcp_register_cleanup(lua_State *state, zcp_cleanup_t cleanfunc, void *cleanarg) { zcp_run_info_t *ri = zcp_run_info(state); - /* - * A cleanup function should always be explicitly removed before - * installing a new one to avoid accidental clobbering. - */ - ASSERT3P(ri->zri_cleanup, ==, NULL); - ri->zri_cleanup = cleanfunc; - ri->zri_cleanup_arg = cleanarg; + zcp_cleanup_handler_t *zch = kmem_alloc(sizeof (*zch), KM_SLEEP); + zch->zch_cleanup_func = cleanfunc; + zch->zch_cleanup_arg = cleanarg; + list_insert_head(&ri->zri_cleanup_handlers, zch); + + return (zch); } void -zcp_clear_cleanup(lua_State *state) +zcp_deregister_cleanup(lua_State *state, zcp_cleanup_handler_t *zch) { zcp_run_info_t *ri = zcp_run_info(state); - - ri->zri_cleanup = NULL; - ri->zri_cleanup_arg = NULL; + list_remove(&ri->zri_cleanup_handlers, zch); + kmem_free(zch, sizeof (*zch)); } /* - * If it exists, execute the currently set cleanup function then unregister it. + * Execute the currently registered cleanup handlers then free them and + * destroy the handler list. */ void zcp_cleanup(lua_State *state) { zcp_run_info_t *ri = zcp_run_info(state); - if (ri->zri_cleanup != NULL) { - ri->zri_cleanup(ri->zri_cleanup_arg); - zcp_clear_cleanup(state); + for (zcp_cleanup_handler_t *zch = + list_remove_head(&ri->zri_cleanup_handlers); zch != NULL; + zch = list_remove_head(&ri->zri_cleanup_handlers)) { + zch->zch_cleanup_func(zch->zch_cleanup_arg); + kmem_free(zch, sizeof (*zch)); } } @@ -819,19 +816,12 @@ zcp_panic_cb(lua_State *state) } static void -zcp_eval_sync(void *arg, dmu_tx_t *tx) +zcp_eval_impl(dmu_tx_t *tx, boolean_t sync, zcp_eval_arg_t *evalargs) { int err; zcp_run_info_t ri; - zcp_eval_arg_t *evalargs = arg; lua_State *state = evalargs->ea_state; - /* - * Open context should have setup the stack to contain: - * 1: Error handler callback - * 2: Script to run (converted to a Lua function) - * 3: nvlist input to function (converted to Lua table or nil) - */ VERIFY3U(3, ==, lua_gettop(state)); /* @@ -844,8 +834,9 @@ zcp_eval_sync(void *arg, dmu_tx_t *tx) ri.zri_cred = evalargs->ea_cred; ri.zri_tx = tx; ri.zri_timed_out = B_FALSE; - ri.zri_cleanup = NULL; - ri.zri_cleanup_arg = NULL; + ri.zri_sync = sync; + list_create(&ri.zri_cleanup_handlers, sizeof (zcp_cleanup_handler_t), + offsetof(zcp_cleanup_handler_t, zch_node)); ri.zri_curinstrs = 0; ri.zri_maxinstrs = evalargs->ea_instrlimit; @@ -882,10 +873,10 @@ zcp_eval_sync(void *arg, dmu_tx_t *tx) /* * Remove the error handler callback from the stack. At this point, - * if there is a cleanup function registered, then it was registered - * but never run or removed, which should never occur. + * there shouldn't be any cleanup handler registered in the handler + * list (zri_cleanup_handlers), regardless of whether it ran or not. */ - ASSERT3P(ri.zri_cleanup, ==, NULL); + list_destroy(&ri.zri_cleanup_handlers); lua_remove(state, 1); switch (err) { @@ -973,9 +964,73 @@ zcp_eval_sync(void *arg, dmu_tx_t *tx) } } +static void +zcp_pool_error(zcp_eval_arg_t *evalargs, const char *poolname) +{ + evalargs->ea_result = SET_ERROR(ECHRNG); + (void) lua_pushfstring(evalargs->ea_state, "Could not open pool: %s", + poolname); + zcp_convert_return_values(evalargs->ea_state, evalargs->ea_outnvl, + ZCP_RET_ERROR, evalargs); + +} + +static void +zcp_eval_sync(void *arg, dmu_tx_t *tx) +{ + zcp_eval_arg_t *evalargs = arg; + + /* + * Open context should have setup the stack to contain: + * 1: Error handler callback + * 2: Script to run (converted to a Lua function) + * 3: nvlist input to function (converted to Lua table or nil) + */ + VERIFY3U(3, ==, lua_gettop(evalargs->ea_state)); + + zcp_eval_impl(tx, B_TRUE, evalargs); +} + +static void +zcp_eval_open(zcp_eval_arg_t *evalargs, const char *poolname) +{ + + int error; + dsl_pool_t *dp; + dmu_tx_t *tx; + + /* + * See comment from the same assertion in zcp_eval_sync(). + */ + VERIFY3U(3, ==, lua_gettop(evalargs->ea_state)); + + error = dsl_pool_hold(poolname, FTAG, &dp); + if (error != 0) { + zcp_pool_error(evalargs, poolname); + return; + } + + /* + * As we are running in open-context, we have no transaction associated + * with the channel program. At the same time, functions from the + * zfs.check submodule need to be associated with a transaction as + * they are basically dry-runs of their counterparts in the zfs.sync + * submodule. These functions should be able to run in open-context. + * Therefore we create a new transaction that we later abort once + * the channel program has been evaluated. + */ + tx = dmu_tx_create_dd(dp->dp_mos_dir); + + zcp_eval_impl(tx, B_FALSE, evalargs); + + dmu_tx_abort(tx); + + dsl_pool_rele(dp, FTAG); +} + int -zcp_eval(const char *poolname, const char *program, uint64_t instrlimit, - uint64_t memlimit, nvpair_t *nvarg, nvlist_t *outnvl) +zcp_eval(const char *poolname, const char *program, boolean_t sync, + uint64_t instrlimit, uint64_t memlimit, nvpair_t *nvarg, nvlist_t *outnvl) { int err; lua_State *state; @@ -1086,9 +1141,14 @@ zcp_eval(const char *poolname, const char *program, uint64_t instrlimit, evalargs.ea_outnvl = outnvl; evalargs.ea_result = 0; - VERIFY0(dsl_sync_task(poolname, zcp_eval_check, - zcp_eval_sync, &evalargs, 0, ZFS_SPACE_CHECK_NONE)); - + if (sync) { + err = dsl_sync_task(poolname, NULL, + zcp_eval_sync, &evalargs, 0, ZFS_SPACE_CHECK_NONE); + if (err != 0) + zcp_pool_error(&evalargs, poolname); + } else { + zcp_eval_open(&evalargs, poolname); + } lua_close(state); return (evalargs.ea_result); diff --git a/module/zfs/zcp_synctask.c b/module/zfs/zcp_synctask.c index ad9bffacb46c..5a7066407394 100644 --- a/module/zfs/zcp_synctask.c +++ b/module/zfs/zcp_synctask.c @@ -55,6 +55,10 @@ typedef struct zcp_synctask_info { * * If 'sync' is false, executes a dry run and returns the error code. * + * If we are not running in syncing context and we are not doing a dry run + * (meaning we are running a zfs.sync function in open-context) then we + * return a Lua error. + * * This function also handles common fatal error cases for channel program * library functions. If a fatal error occurs, err_dsname will be the dataset * name reported in error messages, if supplied. @@ -70,6 +74,13 @@ zcp_sync_task(lua_State *state, dsl_checkfunc_t *checkfunc, if (!sync) return (err); + if (!ri->zri_sync) { + return (luaL_error(state, "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)")); + } + if (err == 0) { syncfunc(arg, ri->zri_tx); } else if (err == EIO) { @@ -233,6 +244,15 @@ zcp_synctask_snapshot(lua_State *state, boolean_t sync, nvlist_t *err_details) const char *dsname = lua_tostring(state, 1); zcp_run_info_t *ri = zcp_run_info(state); + /* + * On old pools, the ZIL must not be active when a snapshot is created, + * but we can't suspend the ZIL because we're already in syncing + * context. + */ + if (spa_version(ri->zri_pool->dp_spa) < SPA_VERSION_FAST_SNAP) { + return (ENOTSUP); + } + /* * We only allow for a single snapshot rather than a list, so the * error list output is unnecessary. @@ -243,33 +263,23 @@ zcp_synctask_snapshot(lua_State *state, boolean_t sync, nvlist_t *err_details) ddsa.ddsa_snaps = fnvlist_alloc(); fnvlist_add_boolean(ddsa.ddsa_snaps, dsname); - /* - * On old pools, the ZIL must not be active when a snapshot is created, - * but we can't suspend the ZIL because we're already in syncing - * context. - */ - if (spa_version(ri->zri_pool->dp_spa) < SPA_VERSION_FAST_SNAP) { - return (ENOTSUP); - } + zcp_cleanup_handler_t *zch = zcp_register_cleanup(state, + (zcp_cleanup_t *)&fnvlist_free, ddsa.ddsa_snaps); err = zcp_sync_task(state, dsl_dataset_snapshot_check, dsl_dataset_snapshot_sync, &ddsa, sync, dsname); + zcp_deregister_cleanup(state, zch); fnvlist_free(ddsa.ddsa_snaps); return (err); } -void -zcp_synctask_wrapper_cleanup(void *arg) -{ - fnvlist_free(arg); -} - static int zcp_synctask_wrapper(lua_State *state) { int err; + zcp_cleanup_handler_t *zch; int num_ret = 1; nvlist_t *err_details = fnvlist_alloc(); @@ -277,7 +287,8 @@ zcp_synctask_wrapper(lua_State *state) * Make sure err_details is properly freed, even if a fatal error is * thrown during the synctask. */ - zcp_register_cleanup(state, &zcp_synctask_wrapper_cleanup, err_details); + zch = zcp_register_cleanup(state, + (zcp_cleanup_t *)&fnvlist_free, err_details); zcp_synctask_info_t *info = lua_touserdata(state, lua_upvalueindex(1)); boolean_t sync = lua_toboolean(state, lua_upvalueindex(2)); @@ -317,7 +328,7 @@ zcp_synctask_wrapper(lua_State *state) num_ret++; } - zcp_clear_cleanup(state); + zcp_deregister_cleanup(state, zch); fnvlist_free(err_details); return (num_ret); diff --git a/module/zfs/zfs_ioctl.c b/module/zfs/zfs_ioctl.c index 443005165ad5..cc7155a78ad1 100644 --- a/module/zfs/zfs_ioctl.c +++ b/module/zfs/zfs_ioctl.c @@ -3673,11 +3673,15 @@ zfs_ioc_channel_program(const char *poolname, nvlist_t *innvl, { char *program; uint64_t instrlimit, memlimit; + boolean_t sync_flag; nvpair_t *nvarg = NULL; if (0 != nvlist_lookup_string(innvl, ZCP_ARG_PROGRAM, &program)) { return (EINVAL); } + if (0 != nvlist_lookup_boolean_value(innvl, ZCP_ARG_SYNC, &sync_flag)) { + sync_flag = B_TRUE; + } if (0 != nvlist_lookup_uint64(innvl, ZCP_ARG_INSTRLIMIT, &instrlimit)) { instrlimit = ZCP_DEFAULT_INSTRLIMIT; } @@ -3693,7 +3697,7 @@ zfs_ioc_channel_program(const char *poolname, nvlist_t *innvl, if (memlimit == 0 || memlimit > zfs_lua_max_memlimit) return (EINVAL); - return (zcp_eval(poolname, program, instrlimit, memlimit, + return (zcp_eval(poolname, program, sync_flag, instrlimit, memlimit, nvarg, outnvl)); } 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 f3097940f98c..980d7e9d3671 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 -# -# 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 so the +# test suite doesn't leak temp files on failures. So it +# is expected that is a temp file and not +# an installed file. +# +# +# 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 369ad846a026..ba9c40739471 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 c9a2e64b7bc7..b2e0e600eac5 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 d74b47a1dc1a..0205e6c11ef9 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 8c57bef4b0c5..01bdb63eb678 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 d5d377b771c2..0739940da8e6 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 13ed119ece9d..a3c05fabd55a 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 4d1ac5974a27..83685c4dc709 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 778abc09dd72..e0b917a32b4d 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 2a8e83913b69..d6396f488bd0 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 98317db6e6d1..3691492bfa0c 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 c3585c938673..c20ed8a61dc1 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 c0180d90c4db..f112bf0d2813 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 e818ce9fcb4d..a26234278a91 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"