rrdp-dcmi: Detect more errors on discovery #6746
Conversation
Power management is an optional feature and not always available, even if the devices are present. Commands may fail with different error codes, which correspond to a reason, it's good to log this to be able to reference the IPMI 2.0 spec. Signed-off-by: Pau Ruiz Safont <[email protected]>
| Scanf.sscanf line " DCMI request failed because: %S@(%x)" | ||
| (fun reason code -> Some (Command (command, reason, code)) | ||
| ) | ||
| with _ -> None |
There was a problem hiding this comment.
If we can't parse the error message, shouldn't we try to log the full string as is?
There was a problem hiding this comment.
While I agree in principle we should log all error lines when there's an error, this parses all lines in the error output, which is not the same.
This means that printing all unrecognised error lines will spam the logs: we need to make sure first that the usual error lines which contain no useful information are ignored first.
There was a problem hiding this comment.
What is the overall logic?
- try several patterns until first success or fail if none matches
- try several patterns, report all successes, or fail if none matches
There was a problem hiding this comment.
When there are several patterns, the code should be using the first logic you mentioned. This is because not all possible output is known
| Scanf.sscanf line " DCMI request failed because: %S@(%x)" | ||
| (fun reason code -> Some (Command (command, reason, code)) | ||
| ) | ||
| with _ -> None |
There was a problem hiding this comment.
What is the overall logic?
- try several patterns until first success or fail if none matches
- try several patterns, report all successes, or fail if none matches
3 participants
Power management is an optional feature and not always available, even if the devices are present.
Commands may fail with different error codes, which correspond to a reason, it's good to log this to be able to reference the IPMI 2.0 spec.