[rlaager]

Motivation and Context

This started as a change to eliminate the word "special" from the "spares" pseudo-vdev type, since "special" now has a specific meaning in the context of vdevs. It spiraled into a loosely-related pile of small documentation fixes.

Description

1. I eliminated almost all instances of the word "special" that are not referencing the allocation_classes feature.
2. I cleaned up zpool-features.5's references to commands. I fixed man page section references from 1M (Solaris) to 8 (Linux).
3. I updated a fsck reference in zdb.8 and eliminated the fsdb reference, since the latter does not exist on Linux.
4. I fixed the spelling of deferred in a man page and a code comment.
5. I consistently capitalized GUID in zpool-features.5.
6. I reworded the Edon-R dedup limitation in zpool-features.5 so it isn't worded as "You can not use this (except you can)".
7. I removed an old note about updating GRUB when bumping SPA_VERSION.
8. I updated a code comment to match the code. GRUB supports large_blocks.
9. I clarified GRUB's lack of support for the sha512, skein, and edonr features, including fixing contradictions (arguable contradictions, if you want to be pedantic) between different man pages.

If some of these commits should be squashed, I'm open to that, but I would really prefer that they not all be squashed into one commit.

How Has This Been Tested?

I viewed the man pages with man.

GRUB's large_blocks support can be confirmed here: http://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/fs/zfs/zfs.c#n276

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.

[jwittlincohen]

The suggested changes resolve #8446.

[rlaager]

Many more comments about GRUB are coming, but I updated the pull request to remove this from include/sys/fs/zfs.h:

 * When changing, be sure to teach GRUB how to read the new format!
 * See usr/src/grub/grub-0.97/stage2/{zfs-include/,fsys_zfs*}

[gmelikov]

LGTM, thanks!

[rlaager]

we shouldn't be talking about grub support inside zfs manpages. can this be removed entirely or replaced with some general reference to ensure proper bootloader support?

I was wondering if that might be the feedback.

I have just now submitted a separate pull request for that. See #8627. There is one commit in common to both. If both are accepted, we'll need to remove/revert the last commit here ("Clarify GRUB's lack of support for sha512, skein, edonr").

If this pull request is otherwise acceptable, I would suggest we accept this one as is. That way, we don't have to hold up these changes on the GRUB question. Then, I can add a commit to #8627 to remove the checksum note.

[rlaager]

@jwittlincohen wrote:

The suggested changes resolve #8446.

Thanks for mentioning that. I added a "Closes #8446" to the relevant commit.

[behlendorf]

Thanks for the cleanup. I'm happy to apply each of these as its own commit and not squash anything.

My only concern is that we should leave the GRUB commits for another PR. So I'd like to hold off on that commit. While removing them entirely makes sense for Linux I think they should probably be replaced by something which better explains the situation.

[rlaager]

My only concern is that we should leave the GRUB commits for another PR. So I'd like to hold off on that commit. While removing them entirely makes sense for Linux I think they should probably be replaced by something which better explains the situation.

Can you clarify this a bit? In one place you say commits plural and in another you say commit singular.

"Update a comment to match the code" should be the least objectionable. That's literally fixing the comment to match the code. I assume that can stay?

I think "Remove zfs.h comments about GRUB" is solid in any case. Nobody is going to be bumping SPA_VERSION because we do feature flags. And I assume ZPL_VERSION will only get bumped if it gets a features implementation. Any issues here?

I assume "Clarify GRUB's lack of support for sha512, skein, edonr" is the issue. The current man page language in zpool-features.5 is wrong and the two man pages contradict each other, leading to actual user confusion (#8446). We can either rip it out entirely or fix it. This commit fixes it, which maintains the status quo of discussing GRUB. I'm happy to rip it out entirely in #8627. Are you in favor of ripping it out entirely? If so, I'll remove the commit here and add a removal to #8627.

[behlendorf]

Sorry for being overly vague. Specifically my concern was tiny and only with 833310c which removes the SPA_VERSION comment entirely.

I think we do need keep some documentation here explaining that SPA_VERSION describes the on-disk format. It can explain that the pool version has been replaced by pool feature flags and then reference the longer discussion in module/zfs/zfeature.c. Outside this PR I think we should also consider adding a discussion somewhere describing GRUB and UEFI support for ZFS.

Since I thought this might end up becoming a more substantial documentation update I suggested deferring it to a different PR. Though, I'm OK with the minimal update I mentioned above since it's at least accurate.

This commit fixes it, which maintains the status quo of discussing GRUB.

This makes the most sense to me until we decide how best to discuss this issue.

[rlaager]

There is an existing comment at the top of the SPA_VERSION block:

/*
 * On-disk version number.
 */
#define SPA_VERSION_1                   1ULL

I agree it's good to reference the details in module/zfs/zfeature.c, so I added a comment to that effect. I referred to it as zfeature.c rather than the full path, as that seemed to be the practice based on another comment in the same file. I can change that if desired.

I also expanded the GUID capitalization from just zpool-features.5 to also include the comment in zfeature.c, in the interest of consistency. Obviously, variables in the code and properties still use lowercase "guid", but that's because they're always (by convention) lowercase.

[codecov]

Codecov Report

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

@@            Coverage Diff             @@
##           master    #8626      +/-   ##
==========================================
+ Coverage   78.76%   78.79%   +0.02%     
==========================================
  Files         381      381              
  Lines      117527   117527              
==========================================
+ Hits        92575    92605      +30     
+ Misses      24952    24922      -30

Flag	Coverage Δ
#kernel	79.37% <ø> (+0.02%)	⬆️
#user	67.24% <ø> (-0.32%)	⬇️

---

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 c2c6ead...ec834ef. Read the comment docs.