zvol: check IO request type

[tonyhutter]

Motivation and Context

Check the ZVOL IO request type to make sure it's supported.

Description

ZVOLs don't support all block layer IO request types. Add a check for the IO types we do support. Also, remove references to
io_is_secure_erase() since they are not supported on ZVOLs.

How Has This Been Tested?

Updated test case

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.

[tonyhutter]

@amotin I updated the PR to check for all supported IO request types.

[amotin]

@tonyhutter This is a bit neater, but it still uses op_is_write() and op_is_sync(), which do not verify exact operation code, but only some aspects of the operation, such as data direction, need to sync caches in process, etc. BTW, bio_data_dir() used below also does not check for specific operation, but only a data direction. Do we know somehow for sure that bio, for example, will never have anything other than read and write?

[tonyhutter]

it still uses op_is_write() and op_is_sync(), which do not verify exact operation code, but only some aspects of the operation, such as data direction, need to sync caches in process, etc.

We can do the check in two ways:

1. Check the request opcode for REQ_OP_READ | REQ_OP_WRITE | REQ_OP_FLUSH | REQ_OP_DISCARD.
2. Use the kernel's built-in macros (op_is_write(), op_is_sync(), op_is_discard() ...).

I chose 2 with the idea that it's better to use the kernel's "API" for checking the request type than looking at the raw opcodes. The problem with 2 though is that the macros are scattershot and ambiguous. For example, there's no op_is_read(), and op_is_sync() assumes reads are synchronous.

I don't have a strong preference though. If you prefer we check the opcodes I'm fine with that.

Do we know somehow for sure that bio, for example, will never have anything other than read and write?

This PR is only checking requests since that's where we found the sync bug (#17765). If you want I can expand the scope to check BIOs as well.

[amotin]

Use of kernel APIs is great when possible. But we have to be sure that we use only APIs of that level and never look inside later. But op_is_write() is just too wide. It only covers data direction, and will cover discard, write zeroes, zone close and who knows what else that we don't support.

If you want I can expand the scope to check BIOs as well.

Unless BIOs are limited in types (I see bio_data_dir() is a wrapper around the same op_is_write()), I'd say the checks should be identical.

[tonyhutter]

@amotin I've reworked the code to look at the opcodes for both BIOs and requests. It feels cleaner now. Please take another look when you get a chance.

[amotin]

Looks better to me. I personally would integrate the two ifs together, having unsupported as else, but whatever. Meanwhile just one comment below: