Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] zfs_create: round up volume size to multiple of bs #10196

Merged
merged 1 commit into from
Apr 25, 2020

Conversation

spaghetti-
Copy link
Contributor

@spaghetti- spaghetti- commented Apr 12, 2020

Round up the volume size requested in zfs create -V size to the next
higher multiple of the volblocksize. Updates the man page and adds a
test to verify the new behavior.

Reported-by: puffi puffi@users.noreply.github.com
Signed-off-by: Alex John alex@stty.io

Motivation and Context

Fixes #8541

Description

Adds a check in zfs_main after all the arguments have been parsed to check if its a volume. Then we check if the user has specified a blocksize, if not, set it to default and check if the volsize is a multiple of. If not, round it up using the P2ROUNDUP_TYPED macro and set the new volsize in the nvlist.

Documentation has been updated to reflect the above and a naive test added (which can be improved).

How Has This Been Tested?

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.

@codecov-io
Copy link

codecov-io commented Apr 12, 2020

Codecov Report

Merging #10196 into master will decrease coverage by 0.16%.
The diff coverage is 75.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10196      +/-   ##
==========================================
- Coverage   79.33%   79.17%   -0.17%     
==========================================
  Files         389      389              
  Lines      123038   123046       +8     
==========================================
- Hits        97611    97419     -192     
- Misses      25427    25627     +200     
Flag Coverage Δ
#kernel 79.85% <ø> (-0.06%) ⬇️
#user 65.56% <75.00%> (+0.49%) ⬆️
Impacted Files Coverage Δ
cmd/zfs/zfs_main.c 82.54% <75.00%> (+0.03%) ⬆️
cmd/zdb/zdb_il.c 30.86% <0.00%> (-24.08%) ⬇️
module/os/linux/spl/spl-kmem-cache.c 75.58% <0.00%> (-8.14%) ⬇️
cmd/ztest/ztest.c 75.68% <0.00%> (-4.68%) ⬇️
module/zfs/bpobj.c 86.86% <0.00%> (-4.29%) ⬇️
module/zfs/zio_compress.c 89.74% <0.00%> (-2.57%) ⬇️
module/zfs/dsl_synctask.c 92.40% <0.00%> (-2.54%) ⬇️
lib/libzpool/util.c 73.95% <0.00%> (-2.09%) ⬇️
module/zfs/txg.c 93.71% <0.00%> (-0.73%) ⬇️
cmd/zdb/zdb.c 80.13% <0.00%> (-0.53%) ⬇️
... and 45 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 6de3e59...de36bdf. Read the comment docs.

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Apr 13, 2020
Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Just two small nits, thank you for taking the time to tackle this issue. Please rebase this on master after addressing the comments.

cmd/zfs/zfs_main.c Outdated Show resolved Hide resolved
Round up the volume size requested in `zfs create -V size` to the next
higher multiple of the volblocksize. Updates the man page and adds a
test to verify the new behavior.

Reported-by: puffi <puffi@users.noreply.github.com>
Signed-off-by: Alex John <alex@stty.io>
@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Apr 24, 2020
@behlendorf behlendorf merged commit 47c9299 into openzfs:master Apr 25, 2020
as-com pushed a commit to as-com/zfs that referenced this pull request Jun 20, 2020
Round up the volume size requested in `zfs create -V size` to the next
higher multiple of the volblocksize. Updates the man page and adds a
test to verify the new behavior.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reported-by: puffi <puffi@users.noreply.github.com>
Signed-off-by: Alex John <alex@stty.io>
Closes openzfs#8541 
Closes openzfs#10196 
(cherry picked from commit 47c9299)
jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
Round up the volume size requested in `zfs create -V size` to the next
higher multiple of the volblocksize. Updates the man page and adds a
test to verify the new behavior.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reported-by: puffi <puffi@users.noreply.github.com>
Signed-off-by: Alex John <alex@stty.io>
Closes openzfs#8541 
Closes openzfs#10196
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

zfs create -V manpage and code discrepancy
3 participants