Show cache devices in spa -v output #253

PaulZ-98 · 2020-11-04T18:24:04Z

Show cache devices in spa -v output
Fixes #114

Pull request checklist

Please check if your PR fulfills the following requirements:

Tests for the changes have been added (for bug fixes / features)
Docs have been reviewed and added / updated if needed (for bug fixes / features)
Build was run locally and any changes were pushed
Lint has passed locally and any fixes were made for failures

Pull request type

Please check the type of change your PR introduces:

What is the current behavior?

spa -v does not list cache vdev

Issue Number: 114

What is the new behavior?

root@ub-20:~/sdb# zpool status
  pool: tank
 state: ONLINE
config:

	NAME           STATE     READ WRITE CKSUM
	tank           ONLINE       0     0     0
	  /root/file1  ONLINE       0     0     0
	logs	
	  /root/file4  ONLINE       0     0     0
	cache
	  /root/file2  ONLINE       0     0     0
	spares
	  /root/file3  AVAIL   

errors: No known data errors
root@ub-20:~/sdb# sdb
sdb: could not get debugging information for:
/usr/lib/modules/5.4.0-52-generic/misc/vboxguest.ko (libdwfl error: No DWARF information found)
/usr/lib/modules/5.4.0-52-generic/misc/vboxvideo.ko (libdwfl error: No DWARF information found)
sdb> spa -v
ADDR               NAME
------------------------------------------------------------
0xffff88ff177f4000 tank
      ADDR               STATE   AUX  DESCRIPTION
      ------------------------------------------------------------
      0xffff88ff1d788000 HEALTHY NONE  root
      0xffff88ff1d7a0000 HEALTHY NONE    /root/file1
      0xffff88ff1d6cc000 HEALTHY NONE    /root/file4
      0xffff88ff1d628000 HEALTHY NONE  /root/file2      <== previously this was not shown
sdb>

Does this introduce a breaking change?

Yes
No

sdimitro · 2020-11-05T19:59:05Z

Hey @PaulZ-98 !

Thanks for posting a PR for this and sorry for the delayed response! It's great that this patch gets to show cache devices! My only problem with it is that the output looks a bit awkward and also you can really tell that the vdev in question is a cache device.

In illumos, MDB's output did a pretty good job:

> ::spa -v
ADDR                 STATE NAME
ffffff03b69bf000    ACTIVE meta-domain

    ADDR             STATE     AUX          DESCRIPTION
    ffffff037a495800 HEALTHY   -            root
    ffffff0385117800 HEALTHY   -              raidz
    ffffff037a5ca800 HEALTHY   -                /tmp/dks0
    ffffff03850c1800 HEALTHY   -                /tmp/dks1
    ffffff038502a000 HEALTHY   -                /tmp/dks2
    -                -         -            cache
    ffffff03850c3800 HEALTHY   -              /dev/dsk/c2t1d0s0

I think we can do the same. Do you think this would be doable in this PR? Maybe within the pretty_print method?

BTW good job figuring out the regression testing infrastructure by yourself! :)

PaulZ-98 · 2020-11-06T17:37:09Z

Pushed a change with the keyword "cache" in the output (tried to match mdb).

sdb> spa -v
ADDR               NAME
------------------------------------------------------------
0xffff88ff177f4000 tank
      ADDR               STATE   AUX  DESCRIPTION
      ------------------------------------------------------------
      0xffff88ff1d788000 HEALTHY NONE  root
      0xffff88ff1d7a0000 HEALTHY NONE    /root/file1
      0xffff88ff1d6cc000 HEALTHY NONE    /root/file4
      -                  -       -     cache
      0xffff88ff1d628000 HEALTHY NONE    /root/file2

codecov-io · 2020-11-06T17:45:52Z

Codecov Report

Merging #253 into master will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #253      +/-   ##
==========================================
+ Coverage   87.37%   87.40%   +0.02%     
==========================================
  Files          60       60              
  Lines        2496     2501       +5     
==========================================
+ Hits         2181     2186       +5     
  Misses        315      315

Impacted Files	Coverage Δ
sdb/commands/zfs/vdev.py	`90.16% <100.00%> (+0.87%)`	⬆️

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 f0aa360...c96444a. Read the comment docs.

sdimitro · 2020-11-06T17:57:51Z

sdb/commands/zfs/vdev.py

        else:
            yield from self.from_vdev(spa.spa_root_vdev)

+        for j in range(0, spa.spa_l2cache.sav_count):


for j in range(spa.spa_l2cache.sav_count)

no need for 0,

sdimitro · 2020-11-06T18:01:19Z

sdb/commands/zfs/vdev.py

+            if vdev.vdev_isl2cache != 0:
+                print("".ljust(indent), "-".ljust(18), "-".ljust(7),
+                      "-".ljust(4), "".ljust(level), "cache")
+                level = 2


I may be wrong but won't this print the - - - cache header line above each cache device entry?

In MDB this would be printed once:

> ::spa -v ADDR STATE NAME ffffff03afc1b000 ACTIVE meta-domain ADDR STATE AUX DESCRIPTION ffffff0385108800 HEALTHY - root ffffff03850ba800 HEALTHY - raidz ffffff0384fd7000 HEALTHY - /tmp/dks0 ffffff0384fee000 HEALTHY - /tmp/dks1 ffffff0385101800 HEALTHY - /tmp/dks2 - - - cache ffffff037a48c800 HEALTHY - /dev/dsk/c2t1d0s0 ffffff03850c7800 HEALTHY - /dev/dsk/c2t2d0s0 - - - spares ffffff038269a000 HEALTHY - /dev/dsk/c2t3d0s0 ffffff0375690000 ACTIVE rpool ffffff037571b000 HEALTHY - root ffffff03758c7800 HEALTHY - /dev/dsk/c2t0d0s0

Dang! That's true.

Ok. Let me know if there was a better way to code it up. Here's the output.

sdb> spa -v ADDR NAME ------------------------------------------------------------ 0xffff88ff177f4000 tank ADDR STATE AUX DESCRIPTION ------------------------------------------------------------ 0xffff88ff1d788000 HEALTHY NONE root 0xffff88ff1d7a0000 HEALTHY NONE /root/file1 0xffff88ff1d6cc000 HEALTHY NONE /root/file4 - - - cache 0xffff88ff1d628000 HEALTHY NONE /root/file2 0xffff88ff1d3ac000 HEALTHY NONE /root/file5 sdb>

In my pool "tank", /root/file4 is a log device, so I think had better add code for the log keyword preceding the vdev just like for l2arc vdev(s).

root@ub-20:~/sdb# zpool status pool: tank state: ONLINE config: NAME STATE READ WRITE CKSUM tank ONLINE 0 0 0 /root/file1 ONLINE 0 0 0 logs /root/file4 ONLINE 0 0 0 cache /root/file2 ONLINE 0 0 0 /root/file5 ONLINE 0 0 0 spares /root/file3 AVAIL

Added code for log and spares.

sdb> spa -v ADDR NAME ------------------------------------------------------------ 0xffff88ff1d718000 tank ADDR STATE AUX DESCRIPTION ------------------------------------------------------------ 0xffff88ff297d0000 HEALTHY NONE root 0xffff88ff17014000 HEALTHY NONE /root/file1 - - - logs 0xffff88ff1212c000 HEALTHY NONE mirror 0xffff88ff1d570000 HEALTHY NONE /root/file4 0xffff88ff1d548000 HEALTHY NONE /root/file6 - - - cache 0xffff88ff1d550000 HEALTHY NONE /root/file2 0xffff88ff1d5d8000 HEALTHY NONE /root/file5 - - - spares 0xffff88ff297cc000 HEALTHY NONE /root/file3 sdb> root@ub-20:~/sdb# zpool status pool: tank state: ONLINE config: NAME STATE READ WRITE CKSUM tank ONLINE 0 0 0 /root/file1 ONLINE 0 0 0 logs mirror-1 ONLINE 0 0 0 /root/file4 ONLINE 0 0 0 /root/file6 ONLINE 0 0 0 cache /root/file2 ONLINE 0 0 0 /root/file5 ONLINE 0 0 0 spares /root/file3 AVAIL errors: No known data errors root@ub-20:~/sdb#

sdimitro

Apologies for the delay in this! @PaulZ-98 This looks ready to get in! Could you please rebase with the latest master?

Fixes delphix#114

PaulZ-98 force-pushed the spa_cache branch 4 times, most recently from 1f6060b to 529a069 Compare November 5, 2020 15:25

PaulZ-98 force-pushed the spa_cache branch 2 times, most recently from d767035 to e62d63b Compare November 6, 2020 17:26

PaulZ-98 force-pushed the spa_cache branch from e62d63b to c96444a Compare November 6, 2020 17:41

sdimitro suggested changes Nov 6, 2020

View reviewed changes

PaulZ-98 force-pushed the spa_cache branch 3 times, most recently from 46ad091 to 8ca849d Compare November 6, 2020 22:11

sdimitro approved these changes Mar 24, 2021

View reviewed changes

sdimitro requested a review from ahrens March 24, 2021 15:02

ahrens approved these changes Mar 24, 2021

View reviewed changes

Show cache devices in spa -v output.

07d35d4

Fixes delphix#114

PaulZ-98 force-pushed the spa_cache branch from 8ca849d to 07d35d4 Compare March 29, 2021 18:50

sdimitro merged commit 192ec72 into delphix:master Mar 29, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Show cache devices in spa -v output #253

Show cache devices in spa -v output #253

Uh oh!

PaulZ-98 commented Nov 4, 2020 •

edited

Loading

Uh oh!

sdimitro commented Nov 5, 2020 •

edited

Loading

Uh oh!

PaulZ-98 commented Nov 6, 2020

Uh oh!

codecov-io commented Nov 6, 2020 •

edited

Loading

Uh oh!

sdimitro Nov 6, 2020

Uh oh!

sdimitro Nov 6, 2020

Uh oh!

PaulZ-98 Nov 6, 2020

Uh oh!

PaulZ-98 Nov 6, 2020

Uh oh!

PaulZ-98 Nov 6, 2020

Uh oh!

PaulZ-98 Nov 6, 2020

Uh oh!

sdimitro left a comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

Show cache devices in spa -v output #253

Show cache devices in spa -v output #253

Uh oh!

Conversation

PaulZ-98 commented Nov 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request checklist

Pull request type

What is the current behavior?

What is the new behavior?

Does this introduce a breaking change?

Uh oh!

sdimitro commented Nov 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PaulZ-98 commented Nov 6, 2020

Uh oh!

codecov-io commented Nov 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sdimitro Nov 6, 2020

Choose a reason for hiding this comment

Uh oh!

sdimitro Nov 6, 2020

Choose a reason for hiding this comment

Uh oh!

PaulZ-98 Nov 6, 2020

Choose a reason for hiding this comment

Uh oh!

PaulZ-98 Nov 6, 2020

Choose a reason for hiding this comment

Uh oh!

PaulZ-98 Nov 6, 2020

Choose a reason for hiding this comment

Uh oh!

PaulZ-98 Nov 6, 2020

Choose a reason for hiding this comment

Uh oh!

sdimitro left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

PaulZ-98 commented Nov 4, 2020 •

edited

Loading

sdimitro commented Nov 5, 2020 •

edited

Loading

codecov-io commented Nov 6, 2020 •

edited

Loading