Skip snapshot in zfs_iter_mounted()

[youzhongyang]

Signed-off-by: Youzhong Yang yyang@mathworks.com

The intention of the zfs_iter_mounted() is to traverse the dataset and its descendants, not the snapshots. The current code can cause a mounted snapshot to be included and thus zfs_open() on the snapshot with ZFS_TYPE_FILESYSTEM would print confusing message such as "cannot open 'rpool/fs@snap': snapshot delimiter '@' is not expected here".

See #12447.

Motivation and Context

Description

How Has This Been Tested?

Manually.

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.

[youzhongyang]

No idea why tests/functional/cli_root/zfs_rename/zfs_rename_007_pos failed on Fedora 33 x86_64.

Test: /usr/share/zfs/zfs-tests/tests/functional/cli_root/zfs_rename/zfs_rename_007_pos (run as root) [00:01] [FAIL]
17:17:44.75 ASSERTION: Rename dataset, verify that the data haven't changed.
17:17:44.80 SUCCESS: dd if=/dev/urandom of=/mnt/testdir/srcfile.526577 bs=512 count=2048
17:17:44.83 SUCCESS: zfs create testpool/testfs/fs.526577
17:17:44.84 SUCCESS: cp /mnt/testdir/srcfile.526577 /mnt/testdir/fs.526577/srcfile.526577
17:17:44.86 SUCCESS: zfs snapshot testpool/testfs/fs.526577@snap.526577
17:17:44.88 SUCCESS: zfs clone testpool/testfs/fs.526577@snap.526577 testpool/testfs/fsclone.526577
17:17:44.91 SUCCESS: zfs rename testpool/testfs/fs.526577 testpool/testfs/fs.526577-new
17:17:44.94 SUCCESS: zfs rename testpool/testfs/fsclone.526577 testpool/testfs/fsclone.526577-new
17:17:44.95 SUCCESS: diff /mnt/testdir/srcfile.526577 /mnt/testdir/fs.526577-new/srcfile.526577
17:17:44.96 SUCCESS: diff /mnt/testdir/srcfile.526577 /mnt/testdir/fsclone.526577-new/srcfile.526577
17:17:44.99 SUCCESS: zfs rename testpool/testfs/fs.526577-new testpool/testfs/fs.526577
17:17:45.01 SUCCESS: zfs rename testpool/testfs/fs.526577@snap.526577 testpool/testfs/fs.526577@snap.526577-new
17:17:45.03 SUCCESS: zfs clone testpool/testfs/fs.526577@snap.526577-new testpool/testfs/fsclone.526577
17:17:45.04 SUCCESS: diff /mnt/testdir/srcfile.526577 /mnt/testdir/fsclone.526577/srcfile.526577
17:17:45.07 SUCCESS: zfs create -V 100M testpool/testfs/vol.526577
17:17:45.35 SUCCESS: dd if=/mnt/testdir/srcfile.526577 of=/dev/zvol/testpool/testfs/vol.526577 bs=512 count=2048
17:17:45.36 SUCCESS: zfs snapshot testpool/testfs/vol.526577@snap.526577
17:17:45.38 SUCCESS: zfs clone testpool/testfs/vol.526577@snap.526577 testpool/testfs/volclone.526577
17:17:45.64 SUCCESS: zfs rename testpool/testfs/vol.526577 testpool/testfs/vol.526577-new
17:17:45.67 SUCCESS: zfs rename testpool/testfs/volclone.526577 testpool/testfs/volclone.526577-new
17:17:45.98 dd: failed to open '/dev/zvol/testpool/testfs/vol.526577-new': No such file or directory
17:17:45.98 ERROR: dd if=/dev/zvol/testpool/testfs/vol.526577-new of=/mnt/testdir/dstfile.526577 bs=512 count=2048 exited 1

[youzhongyang]

This is a trivial fix, not sure why it sits still with no feedback.

[behlendorf]

This logic was added as part of 0c6d093 which also added the tests/functional/cli_root/zfs_unmount/zfs_unmount_nested.ksh test case. It looks to me like there's a check for exactly this in the test. Can you look in to why the test case wasn't catching this and fix it as part of this PR.

[jwk404]

In #12358 there's an explicit request to enable mounting and unmounting of snapshots from the command line. I think this PR as it exists would essentially prevent that one. Rather than skipping snapshots, would it be better to suppress the message?I haven't tested it, but I suspect the changes to zfs_open in that PR would prevent the confusing message.

[youzhongyang]

In #12358 there's an explicit request to enable mounting and unmounting of snapshots from the command line. I think this PR as it exists would essentially prevent that one. Rather than skipping snapshots, would it be better to suppress the message?I haven't tested it, but I suspect the changes to zfs_open in that PR would prevent the confusing message.

I disagree. This line of code skips the snapshot. This PR does not change the functionality, but it suppresses the annoying stderr message.

[behlendorf]

Looks good. Thanks for the fix and explanation.

[jwk404]

In #12358 there's an explicit request to enable mounting and unmounting of snapshots from the command line. I think this PR as it exists would essentially prevent that one. Rather than skipping snapshots, would it be better to suppress the message?I haven't tested it, but I suspect the changes to zfs_open in that PR would prevent the confusing message.

I disagree. This line of code skips the snapshot. This PR does not change the functionality, but it suppresses the annoying stderr message.

I did not mean to imply that this PR changes current functionality. Apologies for being unclear. What I meant to say is that while continueing prior to zfs_open() does eliminate the error message, it poses a problem for the proposed change in #12358. The diffs in 12358 show the snapshot type being added into the types flag of the zfs_open() call. My question is whether that is enough to stifle the error and work in concert with 12358.

@lundman thoughts?

[lundman]

Ah hmm, sorry didn't see the mention - I'll have to check to see if it still works ok.

[lundman]

It didn't seem to break anything over here;

# git branch
* test_PR#12448

# ./scripts/cmd-macos.sh zfs mount BOOM/keep@one
BOOM/keep@one on /Volumes/BOOM/keep/.zfs/snapshot/one (zfs, local, read-only, journaled, noatime)
# ./scripts/cmd-macos.sh zfs unmount BOOM/keep@one
Unmount successful for /Volumes/BOOM/keep/.zfs/snapshot/one/

# ./scripts/cmd-macos.sh zfs mount BOOM/keep@one
# ./scripts/cmd-macos.sh zpool export -a
Unmount successful for /Volumes/BOOM/test
Unmount successful for /Volumes/BOOM/keep/.zfs/snapshot/one
Volume BOOM on disk2s1 unmounted