Add configure option to disable zfs channel programs#18658
Conversation
|
CI fails to build this on any Linux. Checkstyle is also unhappy. Also IIRC channel programs were used for some other operations, like snapshot deletion, I don't remember, that would need an alternative (restricted?) implementation. |
amotin
added
Status: Revision Needed
github-actions
Bot
removed
the
Status: Revision Needed
|
Sorry for the mess of commits.
There is an embedded channel program in This is the only place I know of that has an embedded channel program. Otherwise users have to write there own channel program and submit them to do different tasks. I'm looking over the build failures, feel free to stop me if I'm the only person interested in this. |
cpalv
force-pushed
the
disablezcp
branch
5 times, most recently
from
b658d01 to
8292bef
Compare
github-actions
Bot
added
the
Status: Work in Progress
cpalv
force-pushed
the
disablezcp
branch
3 times, most recently
from
7a10020 to
825cc42
Compare
Signed-off-by: Eduardo Alvarado <edalv.4Q7GW1IdW1+0@gmail.com>
move lua and zcp files out of module source and add config check Signed-off-by: Eduardo Alvarado <edalv.4Q7GW1IdW1+0@gmail.com>
When zfs channel programs are disabled and multiple snapshots are sent to be destroyed. Each destruction will occur in its own transaction per dsl_sync_task. When enabled, they are destroyed as a group in one transaction. Also instead of collecting errors to return to user, the destroy routine will return on the first error found When channel programs are disable, the zfs channel program ioctl is not registered and returns CMD_UNAVAILABLE to user when attempting to execute a channel program Signed-off-by: Eduardo Alvarado <edalv.4Q7GW1IdW1+0@gmail.com>
Signed-off-by: Eduardo Alvarado <edalv.4Q7GW1IdW1+0@gmail.com>
behlendorf
left a comment
behlendorf
left a comment
There was a problem hiding this comment.
Yeah, this is something we should have added a while ago. There are definitely cases where it makes sense to be able to disable channel programs.
| } | ||
|
|
||
| return (0); | ||
| #endif |
There was a problem hiding this comment.
Originally the thinking was to convert more of the internal functionality to rely on channel programs. Clearly that never happened and only dsl_destroy_snapshots_nvl() was updated. In hindsight that's probably for the best. Rather than maintain two versions of this function let's just always use the non-channel programs version.
| rm -Rf build | ||
|
|
||
| ZFS_CONFIG=all | ||
| ZFS_DISABLE_ZCP= |
There was a problem hiding this comment.
This shouldn't be needed and can be dropped.
|
|
||
|
|
||
| AS_IF([test "x$enable_zcp" = xno], [ | ||
| KERNELCPPFLAGS="${KERNELCPPFLAGS} -DDISABLE_ZCP" |
There was a problem hiding this comment.
The AC_DEFINE() adds the DISABLE_ZCP define to zfs_config.h which should be sufficient. No need to add it to KERNELCPPFLAGS here, you can drop this.
| %D%/man8/zpool_influxdb.8 | ||
|
|
||
| dist_man_MANS += \ | ||
| $(zcp_manpg) |
There was a problem hiding this comment.
I think we can simplify this a bit.
if ZCP_ENABLED
dist_man_MANS += \
%D%/man8/zfs-program.8
endif
| module/zfs/zrlock.c \ | ||
| module/zfs/zthr.c | ||
|
|
||
| nodist_libzpool_la_SOURCES += \ |
There was a problem hiding this comment.
Let's move the guard around this block. I'd also recommend we invert the logic in rename it ZCP_ENABLED throughout the change, this is a bit more consistent with what's done elsewhere.
| } | ||
|
|
||
| static zfs_ioc_vec_t zfs_ioc_vec[ZFS_IOC_LAST - ZFS_IOC_FIRST]; | ||
| static zfs_ioc_vec_t zfs_ioc_vec[ZFS_IOC_LAST - ZFS_IOC_FIRST] = { 0 }; |
There was a problem hiding this comment.
No objection, but global variables are initialized to zero so this shouldn't be needed. Did this generate a warning?
| $(zcp_tests) | ||
|
|
||
| if DISABLE_ZCP | ||
| zcp_tests_scripts = \ |
There was a problem hiding this comment.
I don't see any harm in installing the tests even when channel program support is disabled. The important thing is that these tests are skipped when support is disabled. If you update the functional/channel_program/synctask_core/setup.ksh and functional/channel_program/lua_core/setup.ksh scripts to exit using log_unsupported when channel program support is disabled this will skip the test group. You'll also need to add at exception to tests/test-runner/bin/zts-report.py.in that the setup may fail with SKIP to prevent this from failing in the CI. Take a look at functional/chattr/setup.ksh for an example.
3 participants
Motivation and Context
Don't utilize zfs channel programs, disables unnecessary code
Description
Discussed on mailing list. Add a configure option to disable zfs channel programs. Default is to allow zfs channel programs
How Has This Been Tested?
Tested on local build
Types of changes
Checklist:
Signed-off-by.