BMC and Host State Management Refactor

[Andrew Jeffery]

On Mon, 2016-11-21 at 20:28 -0600, Andrew Geissler wrote:
> > On Sun, Nov 20, 2016 at 11:55 PM, Joel Stanley <joel at jms.id.au> wrote:
> > Hi Andrew and Josh,
> > 
> > On Sat, Nov 19, 2016 at 7:01 AM, Andrew Geissler <geissonator at gmail.com> wrote:
> > > Josh and I are working two stories this sprint that deal with
> > > refactoring the bmc and host state management code out of skeleton
> > > (#772/#783).  Here’s the proposal on this work.
> > 
> > Thanks for sending out your plan, this is great. I have a few comments
> > that came up as I was reading.
> > 
> > > The overall design for both state management objects is that they will
> > > provide a set of properties on which to operate.
> > > - DesiredState
> > > - CurrentState
> > > 
> > > CurrentState will be a read only property.
> > 
> > You've chosen to make the desired and current states be separate,
> > which works. Another option would be to have them be the same list of
> > states, so you know that when current==desired you're not waiting on
> > anything to happen. What do you think?
> > 
> 
> Hmmm, I'm thinking from a DBUS/REST api perspective here.  2 seems
> more intuitive, but also I don't think I understand your proposal
> fully :) 

I think you might be misinterpreting. I don't think Joel was suggesting
you eliminate one of the DesiredState or CurrentState "variables",
rather that the /types/ of the CurrentState and DesiredState variables
be equal. That is, that the same set of states can be assigned to both.

>  What would this look like from an implementation perspective
> and DBUS/REST interfaces?

No different, just the set of possible states would be consistent
between the two variables.

Alternatively you could define transitions on the states and rename
DesiredState to Transition, or something similar. This might better
align with what you're proposing (a state machine).

> 
> > > 
> > > The host control will have these additional properties:
> > > - DesiredPowerState
> > > - CurrentPowerState
> > > 
> > > Valid states to request for OpenBMC (DesiredState)
> > > - READY, REBOOT
> > > Valid states to be in for OpenBMC (CurrentState)
> > > - NOT_READY, READY (note this proposal removes BASE_APPS and BMC_STARTING).

What does it mean to request a READY state? When can you do this? Can
you request it whilst the BMC is in NOT_READY? Requesting READY in
READY doesn't seem productive.

If there are restrictions on combinations of source and target state it
might be better to make use of the transition concept to spell it out
(i.e. describe a formal state machine).

> > 
> > Does this need to consider states where the device is updating? That
> > is, it is not attempting to be ready to control the host, but it is
> > turned on and able to accept new firmware?
> > 
> 
> I think READY implies available for code update?  But good point on
> code updates, when a code update is being performed, a FW_UPDATE state
> seems reasonable.
> 
> > > 
> > > READY implies all services started and running successfully (i.e. we
> > > reached obmc-standby.target)
> > > 
> > > 
> > > Valid states to request for Host (DesiredState)
> > > - OFF, ON, REBOOT
> > 
> > This might need to distinguish between soft-off/soft-reboot and
> > printer-on-fire OFF requests.
> > 
> 
> Yeah, the focus of this story was to keep similar function as what's
> in place now, but refactor into c++ and the new sdbusplus interfaces.
> We do probably need more work done with things like you say here.
> We've tended to call the printer-on-fire off's EMERGENCY_ type event
> requests.  At a minimum, I'll be sure these are tracked in future
> stories.
> 
> > > Validate states to be in for Host (CurrentState)
> > > - OFF, BOOTING, BOOTED
> > 
> > STANDBY, BOOTING, RUNNING?
> > 
> > The bikeshed should be blue.
> > 
> 
> Agree

Okay so the bikeshed is blue, but what about the state names?? ;)

Andrew
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20161122/bf453809/attachment.sig>