`nvme list -o json`: always show all fields

[igaw]

Hmm, not good. I think we need to do the same thing we did for the ocp plugin: 6a73c83

I am going to revert the offending commit as it clearly breaks the existing parsers.

Originally posted by @igaw in #2749

[ikegami-t]

Hi, the nvme list -o josn output is not changed by the commit: 6a73c83 mentioned.

1. Before the commit: 6a73c83 (f258dca) output

tokunori@tokunori-desktop:~/nvme-cli-2$ nvme-build list -o json
{
  "Devices":[
    {
      "HostNQN":"nqn.2014-08.org.nvmexpress:uuid:3659a1a8-2d7d-0000-0000-000000000000",
      "HostID":"3659a1a8-2d7d-0000-0000-000000000000",
      "Subsystems":[
        {
          "Subsystem":"nvme-subsys0",
          "SubsystemNQN":"nqn.2019-22.com.intel:nvm-subsystem-sn-btnh922508ps1p0b",
          "Controllers":[
            {
              "Controller":"nvme0",
              "Cntlid":"1",
              "SerialNumber":"BTNH922508PS1P0B",
              "ModelNumber":"INTEL SSDPEKNW010T8",
              "Firmware":"002C",
              "Transport":"pcie",
              "Address":"0000:0d:00.0",
              "Slot":"",
              "Namespaces":[
                {
                  "NameSpace":"nvme0n1",
                  "Generic":"ng0n1",
                  "NSID":1,
                  "UsedBytes":1024209543168,
                  "MaximumLBA":2000409264,
                  "PhysicalSize":1024209543168,
                  "SectorSize":512
                }
              ],
              "Paths":[]
            }
          ],
          "Namespaces":[]
        }
      ]
    }
  ]
}

2. The commit: 6a73c83 output (Completely same with the previous commit: f258dca output)

tokunori@tokunori-desktop:~/nvme-cli-3$ nvme-build list -o json
{
  "Devices":[
    {
      "HostNQN":"nqn.2014-08.org.nvmexpress:uuid:3659a1a8-2d7d-0000-0000-000000000000",
      "HostID":"3659a1a8-2d7d-0000-0000-000000000000",
      "Subsystems":[
        {
          "Subsystem":"nvme-subsys0",
          "SubsystemNQN":"nqn.2019-22.com.intel:nvm-subsystem-sn-btnh922508ps1p0b",
          "Controllers":[
            {
              "Controller":"nvme0",
              "Cntlid":"1",
              "SerialNumber":"BTNH922508PS1P0B",
              "ModelNumber":"INTEL SSDPEKNW010T8",
              "Firmware":"002C",
              "Transport":"pcie",
              "Address":"0000:0d:00.0",
              "Slot":"",
              "Namespaces":[
                {
                  "NameSpace":"nvme0n1",
                  "Generic":"ng0n1",
                  "NSID":1,
                  "UsedBytes":1024209543168,
                  "MaximumLBA":2000409264,
                  "PhysicalSize":1024209543168,
                  "SectorSize":512
                }
              ],
              "Paths":[]
            }
          ],
          "Namespaces":[]
        }
      ]
    }
  ]
}

3. FYI: the current commit: 4a66443 output (Reverted the commit: 929f461)

tokunori@tokunori-desktop:~/nvme-cli$ nvme-build list -o json
{
  "Devices":[
    {
      "NameSpace":1,
      "DevicePath":"/dev/nvme0n1",
      "GenericPath":"/dev/ng0n1",
      "Firmware":"002C",
      "ModelNumber":"INTEL SSDPEKNW010T8",
      "SerialNumber":"BTNH922508PS1P0B",
      "UsedBytes":1024209543168,
      "MaximumLBA":2000409264,
      "PhysicalSize":1024209543168,
      "SectorSize":512
    }
  ]
}

Also the commit: 6a73c83 just changed the ocp smart-add-log command to add the parameter --output-format-version as below. But the changes themselves seem as you mentioned as the command output changed anyway so do you mention to fix the ocp smart-add-log command output? Note: Sorry it is not able to confirm the actual output on my local environment currently.

tokunori@tokunori-desktop:~/nvme-cli-3$ nvme-build ocp smart-add-log /dev/nvme0 -o json -h
Usage: nvme ocp smart-add-log <device> [OPTIONS]

Retrieve the extended SMART health data.

Options:
  [  --output-format=<FMT>, -o <FMT> ]  --- output Format: normal|json
  [  --output-format-version=<NUM> ]    --- output Format version: 1|2

[igaw]

Not sure if I undestood you correctly. You double checked the output for the different versions, right? And the last commit produces the same result as the first one before we started to play with the --verbose output option? If so, the last output look okay IIRC.

The commit 6a73c83 shows how it would be possible to change the output format without breaking existing users. The current output format is v1 so to speak. So if we want to change (not just extend) the verbose output, the new output would be version v2. For a user which wants to v2 he could just nvme list -v --output-format-version 2.

[ikegami-t]

Not sure if I undestood you correctly. You double checked the output for the different versions, right? And the last commit produces the same result as the first one before we started to play with the --verbose output option? If so, the last output look okay IIRC.

Yes the latest code last output look looks okay since already reverted the the commit: 929f461 from the latest code.

The commit 6a73c83 shows how it would be possible to change the output format without breaking existing users. The current output format is v1 so to speak. So if we want to change (not just extend) the verbose output, the new output would be version v2. For a user which wants to v2 he could just nvme list -v --output-format-version 2.

I think that the commit: 6a73c83 is not changed for the nvme list command but changed for the ocp smart-add-log comamnd as added the paramter parameter --output-format-version. (But seems the default output behavior changed from the code changes but not sure.)

[igaw]

Not sure if I undestood you correctly. You double checked the output for the different versions, right? And the last commit produces the same result as the first one before we started to play with the --verbose output option? If so, the last output look okay IIRC.

Yes the latest code last output look okay since already reverted the the commit: 929f461 from the latest code.

Okay, thanks for double checking!

The commit 6a73c83 shows how it would be possible to change the output format without breaking existing users. The current output format is v1 so to speak. So if we want to change (not just extend) the verbose output, the new output would be version v2. For a user which wants to v2 he could just nvme list -v --output-format-version 2.

I think that the commit: 6a73c83 is not changed for the nvme list command but changed for the ocp smart-add-log comamnd as added the paramter --output-format-version. (But seems the default output behavior changed from the code changes but not sure.)

Yes, this is just an example how we could implement this feature for nvme list. Hope this clarifies it :)

[ikegami-t]

Okay, thanks for double checking!

Thank you.

Yes, this is just an example how we could implement this feature for nvme list. Hope this clarifies it :)

Understood. Thanks again.

[ikegami-t]

I just considered to add the --output-format-version option for the PR: #2538 changes reverted from master. The changes just enabled the --verbose option output for the json format. Also some other commands for example nvme smart-log the --human-readable option also outpouts as same with the --verbose option. So if added the --output-format-version option the following command options output will be same but is this really expected behavior?

• nvme list

1. --output-format=json --verbose
2. --output-format=json --output-format-version=2

• nvme smart-log

1. --output-format=json --verbose
2. --output-format=json --human-readable
3. --output-format=json --output-format-version=2

By the way if added the --output-format-version option then the output changed from the --verbose (and --human-readable) option (s) then seems cause an issue for maintenance as the similar outputs functions existed and complicated. (Already nvme ocp smart-add-log may caused as added the version 2 output by the commit: 6a73c83 as we need to update the both version functions if any output change happened.)

(Added)
Just confirmed the suggestion below so understood as in future the older version outputs will be deleted by the major version up.
#2577 (comment)

Obvious it means additional maintenance overhead. When we do the next major version update we just rip out the older version then.

[ikegami-t]

Just created PR: #2796 for this enhancement issue.

[igaw]

The PR looks really good, simple and to really intrusive.

[ikegami-t]

Can we close the issue for now as the PR: #2796 merged?

[igaw]

Yep, this one should be done now. Thanks @ikegami-t for taking care of it!