rust-libzfs

rust-libzfs copied to clipboard

I've updated the API to support libzfs as of zfsonlinux/zfs@2e5dc449c1a65e0b0bf730fd69c9b5804bd57ee8 (current master) which happens to coincide with the libzfs API as shipped in FreeBSD 11.2. I used Debian GNU/Linux 9 with kernel 4.9.88-1+deb9u1 and ZoL master to test on Linux. I generalized the Linux support in the build system to use the include paths in the pkg-config output and output from a small shell script which queries gcc to find the system include directories.

I haven't put a regenerated libzfs-sys/src/bindings.rs in this PR as my generated bindings don't seem to pretty-print and I haven't investigated why not.

A single code location (libzfs-sys/build.rs:115) presupposes the path to the ZFS source on Linux and assumes that you are running against the master of ZoL. Maybe this should be configurable via an environment variable or cargo parameter or similar.

Speaking of environment variables, I could not figure out how env::set_var("LIBCLANG_PATH", "/opt/llvm-5.0.0/lib64/"); affects anything after reading https://github.com/KyleMayes/clang-sys/blob/67f7d8c25eff694d7ba58ff42da6e5b502413b7d/README.md and related source which only uses LIBCLANG_PATH at clang-sys build time.

The line fn list_gcc_include_paths() -> impl Iterator<Item = PathBuf> { uses a rust 1.26+ feature. I'm not sure what the previously preferred solution is to this usage if compatibility for rust < 1.26 is desired.

Probably the most confusing issue I had was with boolean/boolean_t. Use of boolean only seems to work if the declaration is typedef enum boolean { ... } boolean_t but I could only find typedef enum { ... } boolean_t forms in the version history. Using boolean_t is only slightly longer and appears to work universally after NEED_SOLARIS_BOOLEAN is declared for FreeBSD.

Closes #63.

It looks like LIBCLANG_PATH is somehow used and necessary for the CI instances to find their libclang install. I've added it back with a comment about this.

Unfortunately, the latest release of ZoL and the release of ZFS in FreeBSD 11.2 are not compatible. I'm not sure how you'd like to proceed with ZoL version dependency. Due to this and the hard-coded ZoL source path, the CI will always fail on this patchset due to 'missing' headers.

Unfortunately, the latest release of ZoL and the release of ZFS in FreeBSD 11.2 are not compatible.

What incompatibilities are you seeing?

A single code location (libzfs-sys/build.rs:115) presupposes the path to the ZFS source on Linux and assumes that you are running against the master of ZoL. Maybe this should be configurable via an environment variable or cargo parameter or similar.

Yes, we should probably be a bit more generic about how we build now that we want to support multiple OS / releases

Thanks for doing this @dsheets

For reference, I've tested quickly on CentOS 7, using latest ZOL 0.7.9 RPM and got following issues when compiling:

error[E0531]: cannot find unit struct/variant or constant `zfs_prop_t_ZPROP_INVAL` in module `sys`
  --> libzfs/src/zfs.rs:65:18
   |
65 |             sys::zfs_prop_t_ZPROP_INVAL => self.user_props()
   |                  ^^^^^^^^^^^^^^^^^^^^^^ did you mean `zfs_prop_t_ZFS_PROP_BAD`?

error[E0599]: no associated item named `B_TRUE` found for type `u32` in the current scope
  --> libzfs/src/zfs.rs:50:17
   |
50 |                 sys::boolean_t::B_TRUE,
   |                 ^^^^^^^^^^^^^^^^^^^^^^ associated item not found in `u32`

error[E0599]: no associated item named `B_TRUE` found for type `u32` in the current scope
  --> libzfs/src/zfs.rs:51:17
   |
51 |                 sys::boolean_t::B_TRUE,
   |                 ^^^^^^^^^^^^^^^^^^^^^^ associated item not found in `u32`

error[E0599]: no associated item named `B_TRUE` found for type `u32` in the current scope
  --> libzfs/src/zfs.rs:85:25
   |
85 |                         sys::boolean_t::B_TRUE,
   |                         ^^^^^^^^^^^^^^^^^^^^^^ associated item not found in `u32`

error[E0599]: no associated item named `B_FALSE` found for type `u32` in the current scope
  --> libzfs/src/zpool.rs:59:17
   |
59 |                 sys::boolean_t::B_FALSE,
   |                 ^^^^^^^^^^^^^^^^^^^^^^^ associated item not found in `u32`

error[E0599]: no associated item named `B_FALSE` found for type `u32` in the current scope
   --> libzfs/src/zpool.rs:144:67
    |
144 |         let code = unsafe { sys::zpool_disable_datasets(self.raw, sys::boolean_t::B_FALSE) };
    |                                                                   ^^^^^^^^^^^^^^^^^^^^^^^ associated item not found in `u32`

error[E0599]: no associated item named `B_FALSE` found for type `u32` in the current scope
   --> libzfs/src/zpool.rs:152:57
    |
152 |         let code = unsafe { sys::zpool_export(self.raw, sys::boolean_t::B_FALSE, ptr::null_mut()) };
    |                                                         ^^^^^^^^^^^^^^^^^^^^^^^ associated item not found in `u32`

With binding changes, boolean is now generated for me as:

pub const boolean_B_FALSE: boolean = 0;
pub const boolean_B_TRUE: boolean = 1;
pub type boolean = u32;
pub use self::boolean as boolean_t;

Which seems as though there is no linkage between boolean type and B_TRUE / B_FALSE.

Previously, the generation was:

pub mod boolean {
    pub type Type = u32;
    pub const B_FALSE: Type = 0;
    pub const B_TRUE: Type = 1;
}
pub use self::boolean::Type as boolean_t;

FWIW, ZOL boolean def is here: https://github.com/zfsonlinux/zfs/blob/2e5dc449c1a65e0b0bf730fd69c9b5804bd57ee8/tests/zfs-tests/tests/functional/checksum/edonr_test.c#L42

sys::zfs_prop_t_ZPROP_INVAL seems to be a newer addition post 0.7.9.

It seems like we will need to account for OS, binding differences. I guess this could be done either by shipping with a library of bindings and selecting the correct one at runtime, or by being more generic and accounting for more build paths when a user downloads the crate.

If we go the second way pointed out by @jgrund:

It seems like we will need to account for OS, binding differences. I guess this could be done [...] by being more generic and accounting for more build paths when a user downloads the crate.

Something that could be done here is generating separate bindings for each libzfs version and make them modules behind feature flags (e.g. zol-0.7.9 or fbsd-11.2). The actual version could then be probed in build.rs and exposed with https://github.com/rust-lang/cargo/pull/1505.

The obvious disadvantage here would be that the configuration flags exposed by build.rs are local to the crate, which would mean that libzfs-sys as well as the compatibility wrapper around it would have to be in the same crate.