diff options
26 files changed, 422 insertions, 133 deletions
diff --git a/cmd/zfs/zfs_main.c b/cmd/zfs/zfs_main.c index cf31624f0..fce228a20 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 <snapshot> <bookmark>\n")); case HELP_CHANNEL_PROGRAM: - return (gettext("\tprogram [-t <instruction limit>] " + return (gettext("\tprogram [-n] [-t <instruction limit>] " "[-m <memory limit (b)>] <pool> <program file> " "[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 b9e401931..721d90f81 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 480cecac9..7b86f6631 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 df13bf230..b56eda552 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 b9ba719a0..a0e8e7fb6 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 <[email protected]>. * 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 ca0b3cfb5..93704223e 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 b1d937b6e..8a478b70f 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 2cb4a19bb..2e1ffc6ea 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 <[email protected]> -.\" 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 7b49de95f..d0fcacaed 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 1a39330d3..31230dc52 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 ad9bffacb..5a7066407 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) { @@ -234,6 +245,15 @@ zcp_synctask_snapshot(lua_State *state, boolean_t sync, nvlist_t *err_details) 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 443005165..cc7155a78 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 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" |