Clarify and restrict dmu_tx_assign() errors

[behlendorf]

Motivation and Context

ztest could ASSERT in ztest_tx_assign() when an IO error was encountered. This occurs because it only checks for ERESTART and ENOSPC. It does not consider either the over quota or IO error cases. Let's fix that.

Rather than attempting to enumerate each possible error code all tx->tx_err's are converted to EIO. I couldn't find any cases where the additional detail was actually useful to the caller, so I opted to simply the interface. The intent is to make it easier for callers to reason about each possible error and handle it appropriately.

Description

There are three possible cases where dmu_tx_assign() may encounter a fatal error. When there is a true lack of free space (ENOSPC), when there is a lack of quota space (EDQUOT), or when data required to perform the transaction cannot be read from disk (EIO). See the dmu_tx_check_ioerr() function for additional details on the motivation for checking for I/O errors early.

Prior to this change dmu_tx_assign() would return the contents of tx->tx_err which covered a wide range of possible error codes (EIO, ECKSUM, ESRCH, etc). In practice, none of the callers could do anything useful with this level of detail and simply returned the error.

Therefore, this change converts all tx->tx_err errors to EIO, adds ASSERTs to dmu_tx_assign() to cover the only possible errors, and clarifies the function comment to include EIO as a possible fatal error.

How Has This Been Tested?

Locally compiled. It still need a full run through the test suite by the CI.

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)
• Quality assurance (non-breaking change which makes the code more robust against bugs)
• 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.

[robn]

Seems right. I dunno if it's worth noting that the call to dsl_dir_tempreserve_space() in dmu_tx_try_assign() is the only place ENOSPC and EDQUOT can come from. It's easy to figure out by implication I guess, and the asserts in dsl_dir_tempreserve_space() won't let anything else happen, and if it changed in the future, the other asserts would catch it all. So now I've said it all, I guess not :)

[behlendorf]

the asserts in dsl_dir_tempreserve_space() won't let anything else happen

Yeah, I may have gotten a bit carried away. I went ahead and removed most of the asserts in dsl_dir_tempreserve_space() since as you said they'll get caught later. I opted to keep the one that verifies that arc_tempreserve_space() can only safely return 0, EAGAIN, or ERESTART as a form of documentation.