Skip to content

zpool: fix spare/cache indentation in list -v output - #18818

Open
kevans91 wants to merge 1 commit into
openzfs:masterfrom
kevans91:ke/pedantic
Open

zpool: fix spare/cache indentation in list -v output#18818
kevans91 wants to merge 1 commit into
openzfs:masterfrom
kevans91:ke/pedantic

Conversation

@kevans91

Copy link
Copy Markdown
Contributor

Motivation and Context

Normal vdevs come out right because of recursion into collect_list_stats with a greater depth, but spare/cache details are emitted at the top-level and need a bit more of a nudge in indentation to avoid blending in with another zpool.

Some details omitted for brevity. Before:

$ zpool list -v -o name,size,alloc,free,health
NAME             SIZE  ALLOC   FREE    HEALTH
storage         43.7T  2.66M  43.7T    ONLINE
  raidz2-0      21.8T  1.30M  21.8T    ONLINE
    [...]
  raidz2-1      21.8T  1.36M  21.8T    ONLINE
    [...]
spare               -      -      -         -
  gpt/disk6     3.64T      -      -     AVAIL
  gpt/disk7     3.64T      -      -     AVAIL
  gpt/disk14    3.64T      -      -     AVAIL
zroot            216G  1.40G   215G    ONLINE
  mirror-0       216G  1.40G   215G    ONLINE
    ada0p3       217G      -      -    ONLINE
    ada1p3       217G      -      -    ONLINE

After:

$ zpool list -v -o name,size,alloc,free,health
NAME             SIZE  ALLOC   FREE    HEALTH
storage         43.7T  2.66M  43.7T    ONLINE
  raidz2-0      21.8T  1.30M  21.8T    ONLINE
    [...]
  raidz2-1      21.8T  1.36M  21.8T    ONLINE
    [...]
  spare             -      -      -         -
    gpt/disk6   3.64T      -      -     AVAIL
    gpt/disk7   3.64T      -      -     AVAIL
    gpt/disk14  3.64T      -      -     AVAIL
zroot            216G  1.40G   215G    ONLINE
  mirror-0       216G  1.40G   215G    ONLINE
    ada0p3       217G      -      -    ONLINE
    ada1p3       217G      -      -    ONLINE

Description

Give print_line() the ability to print indentation and take it into account for the name and use the appropriate depth for both, accounting for the fact that we're missing a level of depth being at the top-level.

How Has This Been Tested?

Ran on FreeBSD 15.1 with spares configured, doesn't really affect non-spare setupss.

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)
  • Quality assurance (non-breaking change which makes the code more robust against bugs)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist

Normal vdevs come out right because of recursion into
`collect_list_stats` with a greater depth, but spare/cache details are
emitted at the top-level and need a bit more of a nudge in indentation
to avoid blending in with another zpool.

Some details omitted for brevity.  Before:
$ zpool list -v -o name,size,alloc,free,health
NAME             SIZE  ALLOC   FREE    HEALTH
storage         43.7T  2.66M  43.7T    ONLINE
  raidz2-0      21.8T  1.30M  21.8T    ONLINE
    [...]
  raidz2-1      21.8T  1.36M  21.8T    ONLINE
    [...]
spare               -      -      -         -
  gpt/disk6     3.64T      -      -     AVAIL
  gpt/disk7     3.64T      -      -     AVAIL
  gpt/disk14    3.64T      -      -     AVAIL
zroot            216G  1.40G   215G    ONLINE
  mirror-0       216G  1.40G   215G    ONLINE
    ada0p3       217G      -      -    ONLINE
    ada1p3       217G      -      -    ONLINE

After:
$ zpool list -v -o name,size,alloc,free,health
NAME             SIZE  ALLOC   FREE    HEALTH
storage         43.7T  2.66M  43.7T    ONLINE
  raidz2-0      21.8T  1.30M  21.8T    ONLINE
    [...]
  raidz2-1      21.8T  1.36M  21.8T    ONLINE
    [...]
  spare             -      -      -         -
    gpt/disk6   3.64T      -      -     AVAIL
    gpt/disk7   3.64T      -      -     AVAIL
    gpt/disk14  3.64T      -      -     AVAIL
zroot            216G  1.40G   215G    ONLINE
  mirror-0       216G  1.40G   215G    ONLINE
    ada0p3       217G      -      -    ONLINE
    ada1p3       217G      -      -    ONLINE

Signed-off-by: Kyle Evans <kevans@FreeBSD.org>
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Jul 16, 2026
@amotin

amotin commented Jul 17, 2026

Copy link
Copy Markdown
Member

What's about log, dedup and special vdevs? They left without indentation. Also spare/cache is not really a vdev layer to have single indentation same as some raidz2. I feel we need some more universal scheme.

@kevans91

Copy link
Copy Markdown
Contributor Author

What's about log, dedup and special vdevs? They left without indentation. Also spare/cache is not really a vdev layer to have single indentation same as some raidz2. I feel we need some more universal scheme.

This was purely a drive-by of the obvious ones as I was setting up a new pool, as the current formatting is a little misleading without any vertical spacing between pools. I guess my proposal would be bumping vdevs out by another two and using a vdev label before them?

@behlendorf

Copy link
Copy Markdown
Contributor

Also spare/cache is not really a vdev layer to have single indentation same as some raidz2.

@amotin I'm probably misunderstanding your comment, but I'd expect log, cache, spare, dedup and special vdevs all to be shown as on the same level as mirror/raidz/draid vdevs. These are all top-level vdevs from a user perspective and should be shown nested under a pool. This is also how the vdev tree is represented in the --json output.

For example:

# zpool list -v 
NAME         SIZE  ALLOC   FREE  CKPOINT  EXPANDSZ   FRAG    CAP  DEDUP    HEALTH  ALTROOT
tank        21.8T   480K  21.8T        -         -     0%     0%  1.00x    ONLINE  -
  mirror-0  7.27T      0  7.27T        -         -     0%  0.00%      -    ONLINE
    U70     7.28T      -      -        -         -      -      -      -    ONLINE
    U71     7.28T      -      -        -         -      -      -      -    ONLINE
  dedup         -      -      -        -         -      -      -      -         -
    U74     7.28T      0  7.27T        -         -     0%  0.00%      -    ONLINE
  special       -      -      -        -         -      -      -      -         -
    U75     7.28T   480K  7.27T        -         -     0%  0.00%      -    ONLINE
  logs          -      -      -        -         -      -      -      -         -
    U76     7.28T      0  7.27T        -         -     0%  0.00%      -    ONLINE
  cache         -      -      -        -         -      -      -      -         -
    U73     7.28T      0  7.28T        -         -     0%  0.00%      -    ONLINE
  spare         -      -      -        -         -      -      -      -         -
    U72     7.28T      -      -        -         -      -      -      -     AVAIL

And we'd want to give zpool status the same treatment.

# zpool status -v
  pool: tank1
 state: ONLINE
config:

	NAME        STATE     READ WRITE CKSUM
	tank1       ONLINE       0     0     0
	  mirror-0  ONLINE       0     0     0
	    U70     ONLINE       0     0     0
	    U71     ONLINE       0     0     0
	  dedup	
	    U74     ONLINE       0     0     0
	  special	
	    U75     ONLINE       0     0     0
	  logs	
	    U76     ONLINE       0     0     0
	  cache
	    U73     ONLINE       0     0     0
	  spares
	    U72     AVAIL   

errors: No known data errors

@amotin

amotin commented Jul 21, 2026

Copy link
Copy Markdown
Member

I'd expect log, cache, spare, dedup and special vdevs all to be shown as on the same level as mirror/raidz/draid vdevs. These are all top-level vdevs from a user perspective and should be shown nested under a pool.

@behlendorf I agree that showing them with the same indentation as a pool is wrong. But neither I particularly like making the class names look as top-level vdevs, since the top-level vdevs in that case are U72-U77.

@kevans91

Copy link
Copy Markdown
Contributor Author

I'd expect log, cache, spare, dedup and special vdevs all to be shown as on the same level as mirror/raidz/draid vdevs. These are all top-level vdevs from a user perspective and should be shown nested under a pool.

@behlendorf I agree that showing them with the same indentation as a pool is wrong. But neither I particularly like making the class names look as top-level vdevs, since the top-level vdevs in that case are U72-U77.

Would it help to stylize them differently or something? <cache>, etc.

Open to suggestions- I feel like I've stepped into a small hornet's nest that I'm not entirely qualified to poke at personally, but I'm willing to enact whatever you feel is best.

@behlendorf

Copy link
Copy Markdown
Contributor

But neither I particularly like making the class names look as top-level vdevs

That's fair, making them class names look like top-level vdevs is misleading as well. I'm not sure I have a great suggestion for how to improve the output. I'm open to suggestions, but until we can a concrete suggestion we're happy with I'd suggest we close this out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Status: Code Review Needed Ready for review and testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants