Skip to content
  • Matthew Ahrens's avatar
    undocumented libzfs API changes broke "zfs list" · 610cb4fb
    Matthew Ahrens authored
    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 e4288a83 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 e4288a83
    
    :
    "zfs get: don't lookup mount options when using "-s local""
    
    Reviewed-by: default avatarBrian Behlendorf <behlendorf1@llnl.gov>
    Signed-off-by: default avatarMatthew Ahrens <mahrens@delphix.com>
    Closes #11999 
    610cb4fb