Avoid core dump on invalid redaction bookmark

[ghost]

Motivation and Context

libzfs aborts and dumps core on EINVAL from the kernel when trying to
do a redacted send with a bookmark that is not a redaction bookmark.

Description

Move redaction bookmark validation to zfs_send_one in libzfs.

Check if the bookmark given for redactions is actually a redaction
bookmark. Print an error message and exit gracefully if it is not.

Don't abort on EINVAL in zfs_send_one.

How Has This Been Tested?

ZTS on FreeBSD

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)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the ZFS on Linux 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.

[pcd1193182]

Haven't done a detailed review yet, but my first thought is that this code would probably be better suited to being in libzfs itself, rather than in zfs_main.c. Why not just handle the EINVAL in libzfs_send_one ?

[ghost]

In general it seems most validation is done outside of libzfs. I had originally put the validation in libzfs but found that there was much more input validation happening in the zfs command rather than in the libzfs function, so I opted to obey that convention.

My first idea was rather than validating input, to simply handle EINVAL as you suggest. But EINVAL can be the result of a lot of things, so I didn't feel it would be appropriate to give a more specific error message that may not be correct. It seems more useful to check for the likely user error and give a specific reason why the command failed, leaving the generic message and abort in place to catch other, less likely, cases.

[pcd1193182]

Plenty of validation happens in libzfs, and I think that this would be better suited to inclusion in libzfs. Most of the validation that happens in zfs_main is basic validation of the general validity of a command, and very little or none of it involves inspecting the details of a particular dataset or bookmark.

EDIT: I looked at it a bit more, ZFS send definitely has more validation in zfs_main.c than in libzfs, but it is something of an outlier compared to other commands, and we should probably attempt to reverse that trend if possible.

As for handling EINVAL, it can be caused by many things, but it should still probably be handled. The error message can be fully generic. It shouldn't throw an internal error when we call zfs_send_one with an invalid set of arguments. We should probably just add it to the list of errors that just does zfs_error_aux(hdl, strerror(errno));.

[ghost]

• Move redaction bookmark validation to zfs_send_one in libzfs
• Don't abort on EINVAL in zfs_send_one

[codecov-io]

Codecov Report

Merging #10138 into master will increase coverage by <1%.
The diff coverage is 84%.

@@           Coverage Diff            @@
##           master   #10138    +/-   ##
========================================
+ Coverage      79%      79%   +<1%     
========================================
  Files         385      385            
  Lines      122435   122448    +13     
========================================
+ Hits        96681    96860   +179     
+ Misses      25754    25588   -166

Flag	Coverage Δ
#kernel	80% <ø> (ø)	⬆️
#user	66% <84%> (+1%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4d32aba...24d9cb5. Read the comment docs.