freebsd-nq/lib
Matthew Ahrens 610cb4fb8c
undocumented libzfs API changes broke "zfs list"
While OpenZFS does permit breaking changes to the libzfs API, we should
avoid these changes when reasonably possible, and take steps to mitigate
the impact to consumers when changes are necessary.

Commit e4288a8397 made a libzfs API change that is especially
difficult for consumers because there is no change to the function
signatures, only to their behavior.  Therefore, consumers can't notice
that there was a change at compile time.  Also, the API change was
incompletely and incorrectly documented.

The commit message mentions `zfs_get_prop()` [sic], but all callers of
`get_numeric_property()` are impacted: `zfs_prop_get()`,
`zfs_prop_get_numeric()`, and `zfs_prop_get_int()`.

`zfs_prop_get_int()` always calls `get_numeric_property(src=NULL)`, so
it assumes that the filesystem is not mounted.  This means that e.g.
`zfs_prop_get_int(ZFS_PROP_MOUNTED)` always returns 0.

The documentation says that to preserve the previous behavior, callers
should initialize `*src=ZPROP_SRC_NONE`, and some callers were changed
to do that.  However, the existing behavior is actually preserved by
initializing `*src=ZPROP_SRC_ALL`, not `NONE`.

The code comment above `zfs_prop_get()` says, "src: ... NULL will be
treated as ZPROP_SRC_ALL.".  However, the code actually treats NULL as
ZPROP_SRC_NONE.  i.e. `zfs_prop_get(src=NULL)` assumes that the
filesystem is not mounted.

There are several existing calls which use `src=NULL` which are impacted
by the API change, most noticeably those used by `zfs list`, which now
assumes that filesystems are not mounted.  For example,
`zfs list -o name,mounted` previously indicated whether a filesystem was
mounted or not, but now it always (incorrectly) indicates that the
filesystem is not mounted (`MOUNTED: no`).  Similarly, properties that
are set at mount time are ignored.  E.g. `zfs list -o name,atime` may
display an incorrect value if it was set at mount time.

To address these problems, this commit reverts commit e4288a8397:
"zfs get: don't lookup mount options when using "-s local""

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Matthew Ahrens <mahrens@delphix.com>
Closes #11999
2021-05-06 11:24:56 -07:00
..
libavl cppcheck: integrete cppcheck 2021-01-26 16:12:26 -08:00
libefi cppcheck: integrete cppcheck 2021-01-26 16:12:26 -08:00
libicp cppcheck: integrete cppcheck 2021-01-26 16:12:26 -08:00
libnvpair cppcheck: integrete cppcheck 2021-01-26 16:12:26 -08:00
libshare libshare: nfs: commonify nfs_enable_share() 2021-04-19 09:07:09 -07:00
libspl libspl: implement atomics in terms of atomics 2021-04-18 22:13:24 -07:00
libtpool cppcheck: integrete cppcheck 2021-01-26 16:12:26 -08:00
libunicode cppcheck: integrete cppcheck 2021-01-26 16:12:26 -08:00
libuutil libuutil: purge unused functions 2021-04-12 09:32:43 -07:00
libzfs undocumented libzfs API changes broke "zfs list" 2021-05-06 11:24:56 -07:00
libzfs_core libzfs{,_core}: set O_CLOEXEC on persistent (ZFS_DEV and MNTTAB) fds 2021-04-11 15:45:31 -07:00
libzfsbootenv Fix various typos 2021-04-02 18:52:15 -07:00
libzpool lib/: set O_CLOEXEC on all fds 2021-04-11 15:45:59 -07:00
libzstd cppcheck: integrete cppcheck 2021-01-26 16:12:26 -08:00
libzutil libzutil: fix dm_get_underlying_path() return if not a DM device 2021-04-30 15:04:19 -07:00
Makefile.am cppcheck: integrete cppcheck 2021-01-26 16:12:26 -08:00