feat(quirks): add Dell PowerEdge XE9780 platform class - #202
feat(quirks): add Dell PowerEdge XE9780 platform class#202shevchenko-evgeny wants to merge 2 commits into
Conversation
The iDRAC service root reports only Vendor: "Dell", which the XE9780 shares with every other PowerEdge, so it cannot be told apart from the root payload alone. Classification now walks the computer system collection and looks for a member whose Model contains xe9780. The new platform inherits the existing Dell quirks and adds two of its own: - $expand responses are incomplete, so collections are fetched with plain GETs and members individually. - chassis UUID may carry an arbitrary string rather than a GUID; a read patch replaces such values with null so the resource still deserializes. Signed-off-by: Evgeny Shevchenko <eshevchenko@mirantis.com>
Signed-off-by: Evgeny Shevchenko <eshevchenko@mirantis.com>
|
|
||
| impl BmcQuirks { | ||
| pub fn new(root: &ServiceRoot) -> Self { | ||
| pub async fn new<B: Bmc>(root: &ServiceRoot, bmc: &B) -> Result<Self, Error<B>> { |
There was a problem hiding this comment.
This is quite a change. And it shows gaps in quirk platform implementation (maybe).
So i would rather think on how properly support such cases. I would not merge it as is, as it looks more like a very ugly crutch and design problem
There was a problem hiding this comment.
@yoks @poroh Could you please advise what potential changes could get this accepted?
I see your concerns, but I have no idea except to apply the quirks to the whole Dell fleet. My point is that it is better to be as targeted as possible. Honestly, I would even think about adding a BMC software version check before applying any quirks. A combination of Vendor, Model, and FW version looks reasonable to me. What do you think?
There was a problem hiding this comment.
I agree model targeting is preferable, but the current detection puts network I/O and feature dependencies inside BmcQuirks::new. It also deserializes ComputerSystem before the existing Dell patches are available so a known malformed LastResetTime can make member.get() fail and silently classify an XE9780 as generic Dell, leaving the new quirks disabled. Adding firmware matching would not solve that bootstrap problem and may be brittle unless the behavior is proven to be version specific.
Short term, could the safe invalid-UUID normalization, and possibly plain GET behavior, apply to Dell generally? Longer term, platform identity could be collected separately and passed to a non-I/O quirk resolver.
| _root: &ServiceRoot, | ||
| _bmc: &B, | ||
| ) -> Result<bool, Error<B>> { | ||
| Err(Error::MissingFeature("computer-systems".into())) |
There was a problem hiding this comment.
With computer-systems disabled, this helper is still called for every Dell BMC and returns an error, so ServiceRoot::new now fails for valid feature sets such as chassis, accounts, or update-service. Please fall back to generic Dell behavior when model probing is unavailable or return Ok(false) rather than treating an optional feature as fatal.
The iDRAC service root reports only Vendor: "Dell", which the XE9780 shares with every other PowerEdge, so it cannot be told apart from the root payload alone. Classification now walks the computer system collection and looks for a member whose Model contains xe9780. The new platform inherits the existing Dell quirks and adds two of its own:
relates to: NICO ticket