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

Fix column width in 'zpool iostat -v' and 'zpool list -v' #13811

Merged
merged 1 commit into from Sep 6, 2022

Conversation

npc203
Copy link
Contributor

@npc203 npc203 commented Aug 29, 2022

This commit fixes a minor spacing issue caused when enumerating vdev names, which originated from #13031

Signed-off-by: Samuel Wycliffe samuelwycliffe@gmail.com

Motivation and Context

The tabular spacing was off by a few characters after enumerating vdev names, in both zpool iostat -v and zpool list -v .
This PR fixes it in the same way as it is in zpool status as seen here

Description

Before:
image

After:
image

How Has This Been Tested?

As this is a very minor visual change, I've done the changes and compiled it in-tree (using make cmd).
Manually ran the commands before and after, as shown above.

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)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

@behlendorf
Copy link
Contributor

cc: @akashb-22 would you mind reviewing this followup to your previous PR.

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Aug 29, 2022
@akashb-22
Copy link
Contributor

@npc203 Thanks for noticing this, As I generally use files for testing, I haven't noticed this till now
without the patch I still see,

# zpool iostat -v
                                     capacity     operations     bandwidth
pool                               alloc   free   read  write   read  write
---------------------------------  -----  -----  -----  -----  -----  -----
pool-oss0                          1.21M  17.0G      1    206  14.7K  1.02M
  draid2:16d:20c:2s-0              1.21M  17.0G      1    206  14.7K  1.02M
    /root/test/files/draid/file1       -      -      0     10    751  53.1K
    /root/test/files/draid/file2       -      -      0     10    751  52.0K
...

but, for some cases like when I use smaller vdev names(sd* / nvmeX) with draid as top-level vdev, I see the iostat/list width breaks(ie: enumerated vdev names breaks the column width) like in your case,

# zpool iostat -v
                       capacity     operations     bandwidth
pool                 alloc   free   read  write   read  write
-------------------  -----  -----  -----  -----  -----  -----
pool-oss0            1.21M  17.0G      1    201  14.0K   977K
  draid2:16d:20c:2s-0  1.21M  17.0G      1    201  14.0K   977K
    /tmp/file1           -      -      0     10    714  49.5K
    /tmp/file2           -      -      0      9    714  48.6K
...

With this patch applied, column spacing in zpool iostat/list O/P looks to be perfect.

Copy link
Contributor

@akashb-22 akashb-22 left a comment

Choose a reason for hiding this comment

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

I think you need to fix the commit message according to style guidelines.

This commit fixes a minor spacing issue caused when
enumerating vdev names, which originated from openzfs#13031

Signed-off-by: Samuel Wycliffe <samuelwycliffe@gmail.com>
@npc203
Copy link
Contributor Author

npc203 commented Aug 30, 2022

Ah, pardon my ignorance. I was so into fixing the code 80 line length limit, that I skimmed through the commit style check section. Fixed.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Sep 2, 2022
@behlendorf behlendorf merged commit 7c0e394 into openzfs:master Sep 6, 2022
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Sep 15, 2022
This commit fixes a minor spacing issue caused when
enumerating vdev names, which originated from openzfs#13031

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Akash B <akash-b@hpe.com>
Signed-off-by: Samuel Wycliffe <samuelwycliffe@gmail.com>
Closes openzfs#13811
beren12 pushed a commit to beren12/zfs that referenced this pull request Sep 19, 2022
This commit fixes a minor spacing issue caused when
enumerating vdev names, which originated from openzfs#13031

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Akash B <akash-b@hpe.com>
Signed-off-by: Samuel Wycliffe <samuelwycliffe@gmail.com>
Closes openzfs#13811
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.

None yet

3 participants