Warn if volblocksize is smaller than ashift

[lundman]

Users easily created ZVOL with a smaller volblocksize than ashift, then find it
writes much more than expected. I didn't want to stop them from being able to do that, but
at least warn the users of this consequence.

Motivation and Context

Related to https://github.com/openzfsonwindows/ZFSin/issues/239 and initial commit
openzfsonwindows/ZFSin@ee8e361

Description

Simple warning after saving the ashift value.

How Has This Been Tested?

Tested on Windows. But the better way to add new code is to have it approved here, then trickle down. Also gives us a chance to massage the message into English that is more clear.

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.
• I have run the ZFS Test Suite with this change applied.
• All commit messages are properly formatted and contain Signed-off-by.

---

Such a setting results in write amplification and excessive space
wastage.

$ ./zfs create -s -V 3GB -b 2048 tank/vol
Warning: volblocksize (2048) < ashift (12 / 4096)
means all writes are amplified and space wasted.

Signed-off-by: Jorgen Lundman [email protected]

[lundman]

Ah printf differences eh, interesting.

[ahrens]

It would be nice to make the same check for recordsize for filesystems.

[behlendorf]

We've been explicitly adding a u_longlong_t cast to resolve the compiler warnings for the %llu types. Alternately, you could use zfs_nicebytes to pretty-print some of values with human readable suffixes. Either solution seems fine to me.

[DeHackEd]

# zpool list -o name,ashift dehacked
NAME      ASHIFT
dehacked       0
# zpool set ashift=9 dehacked
# zpool list -o name,ashift dehacked
NAME      ASHIFT
dehacked       9

Actual ashift on the one vdev here is 12. The "ashift" property itself means relatively little so that concerns me a bit.

[ahrens]

I didn't realize that the ashift property is disconnected from the vdevs that are in the pool. We should really be doing this based on spa_max_ashift.

[codecov-io]

Codecov Report

Merging #10199 into master will decrease coverage by 0.27%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##           master   #10199      +/-   ##
==========================================
- Coverage   79.48%   79.21%   -0.28%     
==========================================
  Files         387      387              
  Lines      123388   123392       +4     
==========================================
- Hits        98079    97740     -339     
- Misses      25309    25652     +343     

Flag	Coverage Δ
#kernel	79.93% <ø> (-0.04%)	⬇️
#user	64.95% <50.00%> (-1.16%)	⬇️

Impacted Files	Coverage Δ
lib/libzfs/libzfs_dataset.c	76.38% <50.00%> (-0.05%)	⬇️
module/zfs/vdev_indirect.c	73.50% <0.00%> (-10.50%)	⬇️
cmd/ztest/ztest.c	74.43% <0.00%> (-6.35%)	⬇️
cmd/zvol_id/zvol_id_main.c	76.31% <0.00%> (-5.27%)	⬇️
module/zfs/space_map.c	93.06% <0.00%> (-5.20%)	⬇️
module/zfs/spa_checkpoint.c	93.78% <0.00%> (-4.35%)	⬇️
module/lua/lmem.c	83.33% <0.00%> (-4.17%)	⬇️
module/zfs/vdev_initialize.c	94.92% <0.00%> (-2.86%)	⬇️
module/zcommon/zfs_fletcher.c	75.65% <0.00%> (-2.64%)	⬇️
module/zfs/zio_compress.c	89.74% <0.00%> (-2.57%)	⬇️
... and 51 more

---

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 20f2878...a605fd8. Read the comment docs.

[lundman]

spa_max_ashift isn't really reachable from userland right?

[ahrens]

@lundman I don't think so. We could expose that as a new (hidden?) pool property. Or in the zpool config, which is even more hidden, and in keeping with how we expose other pool "internals". But I'd prefer to move away from that since it's much more fragile and undocumented/undocumentable than properties.