ibm-openbmc / openbmc

https://github.com
Other
22 stars 52 forks source link

assetTag: Add persistent configuration of AssetTag for IPS #238

Closed lxwinspur closed 2 years ago

lxwinspur commented 2 years ago

IPS has a requirement that when restoring the factory settings, it is not allowed to restore the AssetTag property. There is a solution to persist the AssetTag property by executing the set/getEnv method.

Signed-off-by: George Liu liuxiwei@inspur.com

jenkins-openbmc-ibm commented 2 years ago

Can one of the admins verify this patch?

edwin-wang commented 2 years ago

Hey @santoshpuranik @geissonator could you help review this patch for persistent AssetTag for IPS. IPS's proposal is storing AssetTag to BMC object and update it once system reset if subModel is "J0". It has been tested on IPS's sample system.

lxwinspur commented 2 years ago

Hey @santoshpuranik @geissonator could you help review this patch for persistent AssetTag for IPS. IPS's proposal is storing AssetTag to BMC object and update it once system reset if subModel is "J0". It has been tested on IPS's sample system.

Well, Could you kindly explain why can't we use env to do a persistent storage mechanism? Because that's the only way I can think of at the moment, and it's been tested. Or could you share another way? Thanks.

geissonator commented 2 years ago

Having dependencies between u-boot and firmware is very dangerous. We do it for code updates and factory reset initiation because there was no better option. But something like this should just use the standard system keyword backup in vpd if it is really needed.

Another potential item is that @anoo1 has been looking into factory resets that preserve targeted items, like network settings. Maybe we could roll this into that design and have a "special" factory reset for IPS.

But could we not just make the factory reset default whatever IPS wants?

lxwinspur commented 2 years ago

Having dependencies between u-boot and firmware is very dangerous. We do it for code updates and factory reset initiation because there was no better option. But something like this should just use the standard system keyword backup in vpd if it is really needed.

So, Will some custom VPD keys be reserved for the IPS?

Another potential item is that @anoo1 has been looking into factory resets that preserve targeted items, like network settings. Maybe we could roll this into that design and have a "special" factory reset for IPS.

Sounds good, So when will this feature be merged into the IBM branch?

But could we not just make the factory reset default whatever IPS wants?

Like the first question I asked, if some VPD keys are reserved for the IPS, I think we can improve this patch and update the VPD key instead of the way to set uboot env to achieve our needs?

santoshpuranik commented 2 years ago

So, Will some custom VPD keys be reserved for the IPS?

This is even more tricky to achieve. We would have to reserve a keyword specifically for IPS, make sure that it is the right size and make sure it is not used anywhere else - This could be a nightmare to handle if parts are shared between IPS and IBM branded systems. I am not in favor of doing this.

Like the first question I asked, if some VPD keys are reserved for the IPS, I think we can improve this patch and update the VPD key instead of the way to set uboot env to achieve our needs?

I think Andrew's suggestion was to fix a reasonable default asset tag that the system can take when factory reset. Something like Server--. This eliminates the need to set aside any VPD keyword, and we can use existing code to default this when it detects a factory reset, keying off of the J0 brand. Surely, if this works for IBM systems, this is something that IPS can settle on as a reasonable middle-ground?

lxwinspur commented 2 years ago

After internal discussions at IPS, it was decided to unify with IBM's existing approach and keep the current solution, so we can close this issue