Bump libzfs.so revision

[behlendorf]

Motivation and Context

Prior to releasing OpenZFS 2.1 it needs to be determined how we want to
bump the libzfs.so library version. This change proposes just bumping the
library revision since the changes do not modify any existing interface. The
OpenZFS 2.0 binaries can use the new library as a drop in replacement.

That said, there's a good argument to be made we should really be bumping
the version instead since the new features in 2.1 do require the updated library.
However, I'd like to avoid unnecessarily inflating the version if possible.

cc: @aerusso @rlaager

Description

The added functions and changes variables are used internally and
not part of the library interface. Bump the libzfs.so revision as
advised by the libtool guidelines due to the source change. The
specific changes are:

Added functions (2):

• boolean_t zpool_is_draid_spare(const char *);
• zpool_compat_status_t zpool_load_compat(const char *,
  boolean_t *, char *, char *);

Changes variables (2):

• spa_feature_table[SPA_FEATURES] - Added SPA_FEATURE_DRAID.
• zpool_prop_table[ZPOOL_NUM_PROPS] - Added ZPOOL_PROP_COMPATIBILITY.

How Has This Been Tested?

Locally compiled, and visually inspected the updated revision number.
Manual compatibility test is still needed.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the OpenZFS code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• I have run the ZFS Test Suite with this change applied.
• All commit messages are properly formatted and contain Signed-off-by.

[rlaager]

https://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html

This looks wrong to me. We are in scenario 2 (in the bottom list on that page), right?

Programs using the previous version may use the new version as drop-in replacement, but programs using the new version may use APIs not present in the previous one. In other words, a program linking against the new version may fail with “unresolved symbols” if linking against the old version at runtime: set revision to 0, bump current and age.

This should be 4:0:0 -> 5:0:1.

[behlendorf]

This looks wrong to me. We are in scenario 2 (in the bottom list on that page), right?

It depends on exactly what we want to consider to be part of the library interface, and what we don't. Right now, the automated ABI checks consider any function listed in the header files (i.e. include/libzfs.h) or any visible type or variable to be part of the public interface. That might be what we want, in which case we'd want to bump the version (4:0:0) -> 5:0:0). But since we get to define what's part of that public API we could alternately exclude the two new functions (which are only used by zpool) and then just bump the revision (4:0:0 -> 4:1:0). If we go the latter route we'd of course want to find some mechanism to annotate which functions are considered to be part of the public API.

I wanted to open this PR to get some feedback on which approach to take. Bumping the version (as you suggest) seems like the right technical thing to do. It also probably means in practice we'll be bumping the libzfs version for each new OpenZFS release. i.e. OpenZFS 2.1 will provide a libzfs5 package instead of a libzfs4 package, perhaps this isn't a big deal. I was initially thinking it might be nice to try and avoid that since the library is almost entirely compatible except for those two functions. Older zfs and zpool binaries can use the newer library without issue, but newer binaries would not be able to use the older library.

[rlaager]

Right now, the automated ABI checks consider any function listed in the header files (i.e. include/libzfs.h) or any visible type or variable to be part of the public interface.

That sounds right at first glance.

That might be what we want, in which case we'd want to bump the version (4:0:0) -> 5:0:0).

4:0:0 -> 5:0:0 is only appropriate if you have broken backwards compatibility. That is, if OpenZFS is using semantic versioning, 4:0:0 -> 5:0:0 would be OpenZFS 2.0.0 -> OpenZFS 3.0.0. If you only extended the API (which is what you have described here), then it's 4:0:0 -> 5:0:1 and OpenZFS 2.0.0 -> OpenZFS 2.1.0.

Put differently, the mapping between semver and libtool is current:micro:minor where current is incremented for both major and minor version bumps. This is how Pidgin/libpurple has handled this for years.

(Whether OpenZFS is actually using semver or not is irrelevant. I'm just using it as semver is relatively familiar to a log of people.)

we could alternately exclude the two new functions (which are only used by zpool)

I'm not sure whether it is reasonable or not for libzfs to export private APIs for use by zpool. If it is, then packaging for zpool must tightly depending on the exact match of libzfs. That said, that is exactly what the Debian zfsutils-linux package is doing today.

If we go the latter route we'd of course want to find some mechanism to annotate which functions are considered to be part of the public API.

Agreed.

[behlendorf]

I've gone ahead and updated the PR to bump the version and the age for libzfs.so as described in the libtool documentation. While I was at it I also bumped the version for libzpool.so (but not the age). This isn't a public library but since it has seen significant change it seemed appropriate to update it as well. The various other libraries remain fully compatible with previous versions.

[rlaager]

LGTM, assuming that "significant change" for libzpool means backwards incompatibility. I did not verify that.

[behlendorf]

"significant change" for libzpool means backwards incompatibility.

That's definitely the case. libzpool is only used by ztest and zdb and it's not intended for use by any external consumers. Arguably it should be version 0.x.x since we've never made any promises about compatibility.

[aerusso]

Not directly related to this merge, but can we guarantee interoperability between different kernel versions and library versions? I know that at least some header files are directly included in both the libraries and the kernel modules---suggesting that changes in those header files could break interoperability.

Given how few programs external to ZFS use the C API, I think a more pressing concern may be kernel-library version interoperability.

But, more directly: I agree with @rlaager that, if the changes are not backwards compatible, we need to bump the version.

[behlendorf]

The changes to libzfs.so are backwards compatible, but to be clear we're in the case @rlaager mentioned above.

Programs using the previous version may use the new version as drop-in replacement, but programs using the new version may use APIs not present in the previous one. In other words, a program linking against the new version may fail with “unresolved symbols” if linking against the old version at runtime: set revision to 0, bump current and age.

So a version bump looks like the right thing to do.

can we guarantee interoperability between different kernel versions and library versions?

We're in good shape here. The user / kernel interface is backwards compatible and older versions of the library will only exercise those backwards compatible interfaces. Furthermore, since the 0.8 release we've had infrastructure in place for the library to check what features are supported by the kernel and only use those which are supported by the kmods. So newer versions of the library will also interoperate with older kernel modules. A useful error message is printed if you try and use a feature not supported by the running kmods.