Skip to content

Conversation

@jeff-lien-sndk
Copy link
Contributor

Added option to specify output-file
The output_format input parm should use -f and the output_file parm should use -o to match the internal-log command.
The output file initialization will add .bin to the passed in output file parm.

OPT_FMT("output-format", 'o', &cfg.output_format,
OPT_FMT("output-format", 'f', &cfg.output_format,
"output Format:normal|json|binary"),
OPT_FILE("output-file", 'o', &cfg.output_file, output_file),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure about the -o. nvme-cli switched over to use -o for --output-format everywhere. It was a bit painful but now it is there and it is consistent. This here will introduce a regression and people will certainly complain.

Copy link
Collaborator

Choose a reason for hiding this comment

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

btw, we don't have a man page for this command either ;)

Copy link
Contributor Author

@jeff-lien-sndk jeff-lien-sndk Aug 22, 2025

Choose a reason for hiding this comment

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

I will create a man page for this command.
As for the -o change, I can change it back, but then think we need to swap the -o and -f parms in the ocp internal-log command. Currently that command uses -f for output-format and -o for output-file.
image

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, I see. I thought that the ocp plugin also uses the -o consistently for the output format. I think it would be good if we stick throughout nvme-cli for a few global arguments

  • -v/--verbose
  • -o/--output-format
  • --output-format-version
  • -t/--timeout
  • --dry-run

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@igaw
Updated and fixed the man page.
changed output-format back to use -o and output-file to use -f.

@jeff-lien-sndk jeff-lien-sndk force-pushed the telemetry-string-log branch 3 times, most recently from 68051e7 to 3063bda Compare August 25, 2025 17:56
Added option to specify output-file
Fix telemetry-string-log output file initialization

Signed-off-by: jeff-lien-sndk <[email protected]>

Reviewed-by: brandon-paupore-sndk <[email protected]>
Renamed nvme-ocp-telemetry-string-log-page.txt to
nvme-ocp-telemetry-string-log.txt so it now matches the
command name.
Added the output-filename options to man page documentation.

Signed-off-by: jeff-lien-sndk <[email protected]>
@igaw
Copy link
Collaborator

igaw commented Aug 27, 2025

Thanks!

@igaw igaw merged commit 2e112db into linux-nvme:master Aug 27, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants