linux-nvme / nvme-cli

NVMe management command line interface.
https://nvmexpress.org
GNU General Public License v2.0
1.45k stars 650 forks source link

plugins/wdc: fix json output for vs-nand-stats #2468

Closed bored-boy closed 3 weeks ago

bored-boy commented 3 weeks ago

Running nvme wdc vs-nand-stats will change the values of the user data fields depending on output format used. In regular stdout, the ordering is TLC min, TLC Max, SLC Min, SLC Max. However in json, the values get reassigned, what was SLC Min in stdout will become TLC >

Regular format (TLC Min, TLC Max, SLC Min, SLC Max) [root@ /tmp/]# nvme wdc vs-nand-stats /dev/nvme0n1 | grep User\ Data User Data Erase Counts - TLC Min 169 User Data Erase Counts - TLC Max 310 User Data Erase Counts - SLC Min 10719 User Data Erase Counts - SLC Max 11676

Json format (SLC Min, SLC Max, TLC Min, TLC Max) [root@ /tmp/]# nvme wdc vs-nand-stats /dev/nvme0n1 -o json | grep User\ Data "User Data Erase Counts - SLC Min" : 169, "User Data Erase Counts - SLC Max" : 310, "User Data Erase Counts - TLC Min" : 10719, "User Data Erase Counts - TLC Max" : 11676,

The difference can be traced to how the array subscripts are being used in the two output formats

stdout : https://t.ly/sRIDz

tlc_min == 0 tlc_max == 1 slc_min == 2 slc_max == 3

json : https://t.ly/pmU-m slc_min = 0 slc_max = 1 tlc_min = 2 tlc_max = 3

With the patch, we rename the fields to be the same for both stdout & json. Output comparison below

Regular format (TLC Min, TLC Max, SLC Min, SLC Max) [root@ /tmp/]# ./nvme wdc vs-nand-stats /dev/nvme0n1 | grep User\ Data User Data Erase Counts - TLC Min 169 User Data Erase Counts - TLC Max 310 User Data Erase Counts - SLC Min 10719 User Data Erase Counts - SLC Max 11676

Json format (TLC Min, TLC Max, SLC Min, SLC Max) [root@ /tmp/]# ./nvme wdc vs-nand-stats /dev/nvme0n1 -o json | grep User\ Data "User Data Erase Counts - TLC Min":169, "User Data Erase Counts - TLC Max":310, "User Data Erase Counts - SLC Min":10719, "User Data Erase Counts - SLC Max":11676,

Signed-off-by: Carl Moran carl.moran1@meta.com

igaw commented 3 weeks ago

@jeff-lien-wdc could you have a look? Thanks!

jeff-lien-wdc commented 3 weeks ago

@igaw The changes look good to me. It's good to merge.

igaw commented 3 weeks ago

@bored-boy could you update the commit message:

Alternatively, i can update the commit message myself, but I need an okay from you to add your Signed-off-by.

bored-boy commented 3 weeks ago

think it's updated to fulfill criteria

jeff-lien-wdc commented 3 weeks ago

@bored-boy I forgot to mention we need to update the wdc plugin version in the wdc-nvme.h part to make tracking the change easier. I will do that now.

jeff-lien-wdc commented 3 weeks ago

@bored-boy I cannot push changes into your forked repo so could you set the WDC_PLUGIN_VERSION to "2.9.3" in wdc-nvme.h? Thanks

jeff-lien-wdc commented 3 weeks ago

@bored-boy Thanks. All looks good to me now.

bored-boy commented 3 weeks ago

@jeff-lien-wdc I may have screwed up the request (not used to dealing with github), let me know if I need to resubmit

igaw commented 3 weeks ago

Yeah, submitting changes can be difficult, each project is handling things differently. sorry about that.

All looks good now. Thanks!