It would be nice if thermal zones were primarily identified by the Name string, using the Zone Index number merely as a fallback.
Currently, it is the other way around.
When IPMI was the only external interface, zone index numbers were required, and the only way to access our IPMI thermal zone extensions: https://github.com/openbmc/phosphor-pid-control/blob/master/ipmi.md
However, now that we have Redfish, this limitation no longer exists, and we should be using names, not numbers, with Redfish.
- The old
config.json JSON format does not support the Name field, so thermal zones can not be named when using that format.
- The new Entity Manager JSON format treats the
ZoneIndex field, the thermal zone number, as optional. This behavior is correct, but the absence of the zone number can lead to problems elsewhere.
- Because Entity Manager JSON parsing is in a random unpredictable order, not top to bottom through the file as it is with the old
config.json parsing, zone numbers will not be consistent from run to run, and they will frequently be different than what the user expected them to be. This causes problems, and this is a good enough reason on its own to want to deprecate index numbers in favor of consistent names.
- The D-Bus objects are named
zone0, zone1, and so on, using only the zone number to influence their naming. They should instead have used the Name field to influence their naming, however, this would break many configurations that people already have in the field. Maybe we can instantiate two copies, to avoid breaking anybody. Name one copy after the zone number, as before, and name one copy after the Name string.
- When instantiating D-Bus objects from user-given names, as always, be careful. Apply D-Bus object name escaping rules. Watch for duplicates, watch for nonexistent names (they are optional in the old
config.json format), and watch for names that would conflict with the legacy zone0 automatic naming format.
- There needs to be an easy and reliable way to look up the zone number given the name, and look up the name given the zone number. This will make the conversion process much easier. More fields should be added to the existing D-Bus objects, to add this.
- Other programs need to have changes made similarly, and be audited to ensure that they are referencing zone names instead of zone index numbers, whenever possible. The program
bmcweb is one example, as it sets up Redfish endpoints named Zone_0, Zone_1, and so on, again using only the zone number to influence their naming.
- When making these fixes, to this and other programs, there may be D-Bus interface changes necessary, which would have a ripple effect.
The good news is that phosphor-pid-control cleanly takes care of a conflicting or missing zone number, and already auto-assigns the next available free zone number in sequence, starting with zero and going up from there:
|
// Auto-assign next unused zone number, using 0-based numbering |
As an example of how the reliance on zone numbers causes problems elsewhere, I made this fix to bmcweb in order to avoid an error when the ZoneIndex field did not appear in the Entity Manager JSON:
https://gbmc-review.googlesource.com/c/gbmcweb/+/13314
Being able to use zone names, instead of relying on zone index numbers, will go a long way towards gaining acceptance of a standard Redfish equivalent to our existing IPMI thermal zone extensions.
It would be nice if thermal zones were primarily identified by the
Namestring, using the Zone Index number merely as a fallback.Currently, it is the other way around.
When IPMI was the only external interface, zone index numbers were required, and the only way to access our IPMI thermal zone extensions: https://github.com/openbmc/phosphor-pid-control/blob/master/ipmi.md
However, now that we have Redfish, this limitation no longer exists, and we should be using names, not numbers, with Redfish.
config.jsonJSON format does not support theNamefield, so thermal zones can not be named when using that format.ZoneIndexfield, the thermal zone number, as optional. This behavior is correct, but the absence of the zone number can lead to problems elsewhere.config.jsonparsing, zone numbers will not be consistent from run to run, and they will frequently be different than what the user expected them to be. This causes problems, and this is a good enough reason on its own to want to deprecate index numbers in favor of consistent names.zone0,zone1, and so on, using only the zone number to influence their naming. They should instead have used theNamefield to influence their naming, however, this would break many configurations that people already have in the field. Maybe we can instantiate two copies, to avoid breaking anybody. Name one copy after the zone number, as before, and name one copy after theNamestring.config.jsonformat), and watch for names that would conflict with the legacyzone0automatic naming format.bmcwebis one example, as it sets up Redfish endpoints namedZone_0,Zone_1, and so on, again using only the zone number to influence their naming.The good news is that
phosphor-pid-controlcleanly takes care of a conflicting or missing zone number, and already auto-assigns the next available free zone number in sequence, starting with zero and going up from there:phosphor-pid-control/dbus/dbusutil.cpp
Line 73 in 3f0f7bc
As an example of how the reliance on zone numbers causes problems elsewhere, I made this fix to
bmcwebin order to avoid an error when theZoneIndexfield did not appear in the Entity Manager JSON:https://gbmc-review.googlesource.com/c/gbmcweb/+/13314
Being able to use zone names, instead of relying on zone index numbers, will go a long way towards gaining acceptance of a standard Redfish equivalent to our existing IPMI thermal zone extensions.