MFV r323530: 7431 ZFS Channel Programs

[avg]

Summary

https://github.com/illumos/illumos-gate/commit/dfc115332c94a2f62058ac7f2bce7631fbd20b3d
https://www.illumos.org/issues/7431
ZFS channel programs (ZCP) adds support for performing compound ZFS
administrative actions via Lua scripts in a sandboxed environment (with time
and memory limits). This initial commit includes both base support for running
ZCP scripts, and a small initial library of API calls which support getting
properties and listing, destroying, and promoting datasets. Testing: in
addition to the included unit tests, channel programs have been in use at
Delphix for several months for batch destroying filesystems.

For reference, a snippet from the new zfs-program manpage is included below.

1

     zfs program – executes ZFS channel programs

SYNOPSIS
     zfs program [-t timeout] [-m memory-limit] pool script

DESCRIPTION
     The ZFS channel program interface allows ZFS administrative operations to
     be run programmatically as a Lua script. The entire script is executed
     atomically, with no other administrative operations taking effect
     concurrently. A library of ZFS calls is made available to channel program
     scripts. Channel programs may only be run with root privileges.

     A modified version of the Lua 5.2 interpreter is used to run channel
     program scripts. The Lua 5.2 manual can be found at:

           http://www.lua.org/manual/5.2/

Also included in this change is MFV of r323533
8552 ZFS LUA code uses floating point math

https://github.com/illumos/illumos-gate/commit/916c8d881190bd2c3ca20d9fca919aecff504435
https://www.illumos.org/issues/8552
In the LUA interpreter used by "zfs program", the lua format() function
accidentally includes support for '%f' and friends, which can cause compilation
problems when building on platforms that don't support floating-point math in
the kernel (e.g. sparc). Support for '%f' friends (%f %e %E %g %G) should be
removed, since there's no way to supply a floating-point value anyway (all
numbers in ZFS LUA are int64_t's).

FreeBSD notes:

• zfs-program.8 manual page is taken almost as is from the vendor repository, no FreeBSD-ification done
• fixed multiple instances of NULL being used where an integer is expected
• replaced ETIME and ECHRNG with ETIMEDOUT and EDOM respectively

Test Plan

Only a basic smoke test.

[smh]

Few nits

cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c

2406

signed vs unsigned arg, seems to be a few of these.

cddl/contrib/opensolaris/lib/libzfs_core/common/libzfs_core.c

945

Do we need to note the difference in FreeBSD here?

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dataset.c

1800

Typo double for

2106

Style nit, no need for the else here.

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_destroy.c

548

result is leaked here

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dir.c

1015

else not needed

1027

else not needed

1054–1057

Declare at head of function?

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zcp.c

617

Since the returns are never reached and they are all the same why not remove them?

912

How about defining ECHRNG as EDOM and ETIME as ETIMEDOUT for FreeBSD, eliminating all the #ifdefs?

[avg]

@smh Steve, a blanket reply to most of your inline comments. Which solution would you prefer, fixing those issues in our tree now and then submitting them to OpenZFS? Or, taking the code as is, submitting patches to OpenZFS and then getting them via the import path?
Another note is that this import is not the latest snapshot of the relevant illumos / OpenZFS code, it's the original ZCP commit. So, some of the issues might have been fixed in subsequent commits that I am going to import immeadiately after this big change.
I guess that what I am trying to say is that it could be better to review the latest upstream code for those issues.

[avg]

cddl/contrib/opensolaris/lib/libzfs_core/common/libzfs_core.c

945

Most probably. But how? Just change the text to the FreeBSD reality?

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zcp.c

617

I guess, because compiler (at least, the one used by illumos) does not realize that the function is 'no return' and produces an error...

912

I tried to do that in a "smart" way, but failed. Here are some of the gory details:

hmm, I had this bright idea to create an illumos compatibility header for errno.h
it would add definitions for a couple of error codes that we don't have
and include_next our real error.h
but the reality is not that simple
because of c++
in particular, because zfsd is in c++
one of the missing error codes is ETIME, which I wanted to define as
#define ETIME ETIMEDOUT
turns out there is /usr/include/c++/v1/errno.h that has
#ifndef ETIME
#define ETIME 9935
#endif
there is also a number of other .h files under /usr/include/c++/v1/ that shadow the standard c headers
ctype.h, stdio,h, etc
so, if a c++ source file happens to include, directly or indirectly, one of the shadowed headers
then it really includes the c++ counterpart
and the c++ errno.h may get included before my compat errno.h
so, the compilation would fail
which is better, of course, than having ETIME defined to different values depending on the language or the order of inclusions
don't see any good solution for this, so I'll probably just replace ETIME in the imported source code

But I guess that the recipe would still work if those error codes are defined local to the source file.
To the source files, actually. Because the userland has to have matching definitions for those codes.

[avg]

• fix some style issues in the FreeBSD adaptation code
• add rS323534 into this MFV

[avg]

@smh Steven, sorry I committed this change despite your objection ("requested changes").
Could you please remove that flag, so that I can mark this request as closed?