search

linux-nvme / nvme-cli

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

ocp: OCP 2.5 Telemetry DA 1 and 2 Parsing Updates #2393

Closed jeff-lien-wdc closed 3 months ago

jeff-lien-wdc commented 3 months ago

This commit will provide the first phase of changes needed for parsing of OCP 2.5 Telemetry Log Pages 7h and 8h, Data Areas 1 and 2. It will decode the predefined classes for Data Area 1 and 2 Statistics, and Event FIFO's.

jeff-lien-wdc commented 3 months ago

@igaw I did add 2 new files in this PR (ocp-telemetry-decode.c and h) and now getting this checkpatch failure: WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?

How do I fix it?

igaw commented 3 months ago

You can ignore this specific failure. We don't have a MAINTAINERS file (yet). Maybe we could add one and make github smart to add the reviewers to the PRs... just joking.

igaw commented 3 months ago

@arthurshau please have a look.

jeff-lien-wdc commented 3 months ago

You can ignore this specific failure. We don't have a MAINTAINERS file (yet). Maybe we could add one and make github smart to add the reviewers to the PRs... just joking.

Ok, thanks for clarifying. I looked for a MAINTAINERS file but couldn't find one. I see I still have a few build failures to still fix. I'll fix them up now.

igaw commented 3 months ago

checkpatch is from the Linux kernel, thus it expects the layout etc from the Linux project. Not all makes sense in our context. I consider checkpatch a helper to make my reviews simpler, but it has also drawbacks as sometimes has some very stupid ideas how some code should look like. So it's good advice to use some sense when to follow the warnings/errors and when not.

arthurshau commented 3 months ago

LGTM!

igaw commented 3 months ago

Thanks!

So if I get this right, this supersedes https://github.com/linux-nvme/nvme-cli/pull/2374 correct?

VigneshwaranSaravana commented 3 months ago

https://github.com/linux-nvme/nvme-cli/pull/2374 implementation merged with this PR

VigneshwaranSaravana commented 2 months ago

@igaw - We have observed some issue in this PR merged code (like NUMD not in divisible by 512 and segmentation issue while printing statistic info). We are planning to fix this issue.

jeff-lien-wdc commented 2 months ago

@VigneshwaranSaravana Please include me as a reviewer in your PR. Thanks.