Cause zpool scan resume commands to get logged in history

[pcd1193182]

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.

Motivation and Context

Currently, commands that resume a scrub/errorscrub from a paused state don't get logged in the pool history. This is because resumes actually return ECANCELED, instead of 0. This causes the tsd code in the common ioctl logic to not think the ioctl succeeded, which causes the log_history ioctl to fail with EPERM. However, for resuming a scrub from a paused state, ECANCELED is success.

Description

There are two options for how to deal with this. The first is the one that I implemented here; I can't find a good reason for dmu_scan to return ECANCELED on resume instead of 0, so let's just not. The only place we check for the ECANCELED value is in zpool_scan, where we just convert it back to zero. However, I am aware that this is changing an ioctl interface, which I believe is a breaking change. I don't think it's an important change, but maybe there is someone who relies on it.

The other option that could be implemented is to either allow ECANCELED specifically from dsl_scan in the common ioctl code, or add a generic facility to the common ioctl code that allows each command to specify whether or not success happened, regardless of the return values. I am open to feedback on which option people think would be better.

Note that the code in libzfs_pool.c that handles this error value has not been changed; this is so that when upgrading userland before upgrading the kernel, scan resumes don't mistakenly report that they failed when they actually succeeded.

How Has This Been Tested?

Manual testing for the issue fix, ran the ZFS sanity tests

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.

[tonyhutter]

@alek-p do you happen to remember the reason for ECANCELED in 0ea05c6?

[tonyhutter]

Similar: #17317

[alek-p]

@alek-p do you happen to remember the reason for ECANCELED in 0ea05c6?

Unfortunately, no. It's been a while, and I don't remember the context anymore. Briefly looking over the code, it doesn't seem necessary.

[tonyhutter]

@pcd1193182 probably safe to remove the Draft label

[behlendorf]

It looks like we lucked out here. This is a breaking change, but its fairly harmless as long as 1) we retain the ECANCELED check in zpool_scan(), and 2) no out of tree consumers are relying on the previous behavior. Let's just extend the comment in zpool_scan above the conditional to make sure we don't drop that check.