Reorder ZFS ioctls to fix cross-version compatibility

[lorenz]

Description

Fixes #8482

How Has This Been Tested?

If this is merged before 0.8 is released, it should have pretty much no impact on existing tooling (since there was never a released version of ZoL with the broken numbering) and avoids a lot of headaches when trying to support multiple releases.

I've tested the kernel side of this change, since it only changes constants and the original reordering didn't cause any issues I do not anticipate any on the userspace side either.

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.
• All new and existing tests passed.
• All commit messages are properly formatted and contain Signed-off-by.

[behlendorf]

@lorenz thanks for catching this! Yes, ZFS_IOC_CHANNEL_PROGRAM should have been added after ZFS_IOC_POOL_SYNC, I agree better to fix this now before it ends up in a tagged release.

You're going to need to update validate_ioc_values() in libzfs_input_check.c as part of this PR. This check was added to the test suite to specifically detect this kind of accidental reordering. (unfortunately it was added a little too late).

[lorenz]

I've updated that now as well, but haven't locally tested it. I'll just let the CI run.

[behlendorf]

Thanks. Might just be Github's rendering but it looks like there may be some cstyle white space issues.

[lorenz]

I've actually just force-pushed a fix for that, but GitHub seems a bit slow to pick it up.

[codecov]

Codecov Report

Merging #8484 into master will decrease coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #8484      +/-   ##
==========================================
- Coverage   78.56%   78.54%   -0.03%     
==========================================
  Files         380      380              
  Lines      116047   116047              
==========================================
- Hits        91177    91144      -33     
- Misses      24870    24903      +33

Flag	Coverage Δ
#kernel	79.02% <ø> (-0.02%)	⬇️
#user	66.98% <ø> (-0.08%)	⬇️

---

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 b46fd24...0f0fb7d. Read the comment docs.

[behlendorf]

LGTM. This will cause a little disruption for those using channel programs, zpool sync, and zfs recv on master if their user/kernel versions aren't in sync. But it will prevent these issues between the tagged 0.7 and 0.8 releases which is the most important thing.

[ahrens]

Do we have any automated tests to ensure that these changes aren't made in the future?

[behlendorf]

@ahrens we do. @don-brady added them, this is why it was necessary to update libzfs_input_check.c which contains to required order. Otherwise this PR would fail the test suite.

[ahrens]

I see, so I guess that the change that introduced the problem happened before libzfs_input_check.c was added? Seems like we would normally want to not allow any changes to existing lines in that file, otherwise someone could reorder the ioctls, and then change the test to pass!

[behlendorf]

@ahrens yes, that's exactly what happened. This issue predates adding the test case.