openbmc / phosphor-state-manager

Apache License 2.0
11 stars 21 forks source link

Inconsistent host/chassis power states #3

Open AlexanderAmelkin opened 5 years ago

AlexanderAmelkin commented 5 years ago

Currently, there are two items in REST that, being changed, directly affect the host power:

  1. /xyz/openbmc_project/state/chassis0/RequestedPowerTransition
  2. /xyz/openbmc_project/state/host0/RequestedHostTransition

The former, when set to xyz.openbmc_project.State.Chassis.Transition.Off, turns host power off without setting the latter to xyz.openbmc_project.State.Host.Transition.Off. That leads to restoration of host power upon AC power cycle or BMC reboot.

@geissonator argued in review comments to https://gerrit.openbmc-project.xyz/15061 that we must not "imply what the user wants" and thus setting RequestedPowerTransition must not alter RequestedHostTransition.

The problem is that there are user interfaces, such as WebUI and others (IPMI, SNMP, some 3rd-party CLI, etc.), that will all have to reimplement the same approach of setting two properties in the correct order if they want a single 'Forced shutdown' button. That is error prone. In fact, WebUI already has that button and it only sets RequestedPowerTransition and thus suffers from the AC power cycle problem.

What we propose here is introduction of a new value for RequestedHostTransition that would mean a forced host shutdown, will by design turn the chassis power off and not return it upon external power cycle or BMC reboot, and will be the single control handle that all the user interfaces will have to pull when they want a forced host shutdown.

Any arguments against it? We'd like to have a discussion before we put any efforts into development of this feature.

geissonator commented 5 years ago

One thing we want to be sure we're doing is moving towards an API that would support the Redfish API's.

curl -k -H "X-Auth-Token: $bmc_token" -X POST https://${bmc}/redfish/v1/Systems/1/Actions/ComputerSystem.Reset -d '{"ResetType": "On"}'

curl -k -H "X-Auth-Token: $bmc_token" -X POST https://${bmc}/redfish/v1/Systems/1/Actions/ComputerSystem.Reset -d '{"ResetType": "GracefulShutdown"}'

curl -k -H "X-Auth-Token: $bmc_token" -X POST https://${bmc}/redfish/v1/Systems/1/Actions/ComputerSystem.Reset -d '{"ResetType": "ForceOff"}'

curl -k -H "X-Auth-Token: $bmc_token" -X POST https://${bmc}/redfish/v1/Systems/1/Actions/ComputerSystem.Reset -d '{"ResetType": "GracefulRestart"}'

There definitely does not appear to be the concept of a host or chassis so your proposal does seem to get us closer to Redfish. A "System" in redfish seems to basically be an instance of our phosphor host state object.

https://github.com/openbmc/bmcweb/blob/947326618971c0f658f4a5348df2f47d8819b165/redfish-core/lib/systems.hpp#L570 is the implementation of redfish in bmcweb right now and you can see it going down the chassis vs. host path based on the command.

Would this new RequestedHostTransition also modify RequestedPowerTransition or that would be irrelevant since modifying RequestedPowerTransition would no longer be needed in the normal operation of the system?

Thoughts @edtanous @gtmills @dkodihal @anoo1 or @vishwabmc (or anyone else)?

AlexanderAmelkin commented 5 years ago

I'd say that RequestedPowerTransition is redundant and is not needed.