Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IH-543: Add IPMI DCMI based power reading rrdd plugin #5561

Merged

Conversation

edwintorok
Copy link
Contributor

@edwintorok edwintorok commented Apr 12, 2024

Tested on Dell PowerEdge R750, and a few other systems seem to have this data too, e.g. SuperMicro.

Draft pull request, because there are occasional issues where ipmitool fails to report anything (usually happens if you query it too often in a row), but seems fine with the 5s interval:

DCMI request failed because: Unspecified error (ff)

Starting a workload on an otherwise idle system resulted in an increase from 309W to 489W, so it seems to be working.

We might want to do a bit more sanity testing, e.g. there are known buggy firmwares that always return 0 W or 65535 W, and we should stop running in that case if that happens right after we call 'discover'.

Also XenCenter doesn't recognize the units (W), so that might need some work.

@edwintorok edwintorok force-pushed the private/edvint/dcmi branch 2 times, most recently from c58ff84 to f03cc14 Compare April 15, 2024 08:55
@edwintorok edwintorok changed the base branch from master to feature/perf April 15, 2024 08:55
@edwintorok edwintorok marked this pull request as ready for review April 15, 2024 08:55
ocaml/xcp-rrdd/bin/rrdp-dcmi/rrdp_dcmi.ml Outdated Show resolved Hide resolved
ocaml/xcp-rrdd/bin/rrdp-dcmi/rrdp_dcmi.ml Outdated Show resolved Hide resolved
ocaml/xcp-rrdd/bin/rrdp-dcmi/rrdp_dcmi.ml Show resolved Hide resolved
ocaml/xcp-rrdd/bin/rrdp-dcmi/rrdp_dcmi.ml Outdated Show resolved Hide resolved
@robhoes
Copy link
Member

robhoes commented Apr 18, 2024

Commits 2–5 essentially look like fixups... squash'em?

@robhoes
Copy link
Member

robhoes commented Apr 22, 2024

This is a combination of 6 commits.

That's not great title for a commit :)

@robhoes
Copy link
Member

robhoes commented Apr 22, 2024

CI says

File "ocaml/xcp-rrdd/bin/rrdp-dcmi/rrdp_dcmi.ml", line 45, characters 4-12:
45 | let ( let* ) = Option.bind
         ^^^^^^^^
Error (warning 32 [unused-value-declaration]): unused value let*.

@edwintorok
Copy link
Contributor Author

Thanks for spotting, updated.

Guard against buggy firmware by ignoring 0 or 65535 Watts
(although xcp-rrdd doesn't actually correctly ignore these yet).

Future-proof plugin by parsing Watts as float, although currently
it is always an integer in the DCMI spec.

Signed-off-by: Edwin Török <edwin.torok@cloud.com>
@edwintorok edwintorok merged commit 51e3159 into xapi-project:feature/perf Apr 26, 2024
13 checks passed
Copy link

pytype_reporter extracted 47 problem reports from pytype output

.

You can check the results of the job here

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.

6 participants