Design proposal on removing /org/openbmc/settings/boot_policy"

[Patrick Williams]

On Thu, Aug 17, 2017 at 03:06:47PM +0530, Deepak Kodihalli wrote:
> On 15/08/17 12:48 am, Patrick Williams wrote:
> > On Thu, Aug 03, 2017 at 04:55:21PM +0530, vishwa wrote:
> >> Now, the proposal is to remove
> >> '"/org/openbmc/settings/host0:boot_policy"' and then put this as a
> >> boolean into 'persist' property into:
> >>
> >> /xyz/openbmc_project/Control/Boot/Mode and
> >> /xyz/openbmc_project/Control/Boot/Source.
> >
> > I understand we have two different interfaces for these two different
> > properties but do we want them on a single object path?  It seems like
> > having multiple properties on /xyz/.../control/hostN/boot would be
> > better, but this will preclude using the same property name in both
> > interfaces.
> 
> The settings app creates distinct objects 
> (https://github.com/openbmc/openbmc/blob/master/meta-phosphor/common/recipes-phosphor/settings/phosphor-settings-defaults/defaults.yaml). 
> One of the reasons for doing this was that it is possible to enable or 
> disable settings not applicable to a system. 

> If the thought is that 
> these settings (mode and source) will always co-exist, we can have one 
> object, although the settings app today has a limitation that it assumes 
> each settings object implements a single settings interface, so that 
> would need to change.

I think the multiple interface / single object limitation of settingsd
is what is causing us to go down this path.  Having two interfaces is
fine to me, but we probably should enhance settingsd to allow same
object path with two different interfaces.

> 
> >> IPMID would then look at this new boolean to see if its ONETIME (
> >> boolean : 0 ) or PERMANENT ( boolean : 1 ) and respond to Get-Boot-Options.
> >
> > Why are we not instead creating two objects?  This proposal creates a
> > bit of undefined behavior in my mind:
> >
> > 1. Since only one dbus property ca be updated at a time, which order is
> > the user suppose to update?
> 
> Why would the order matter to the user?

Other aspects of your reply indicate that it doesn't matter because we
are persisting everything.  I don't think this is the right solution
though; more below.

> 
> > 2. What happens when the property is ONETIME (false), the value of Mode
> > itself is changed, and then the property is changed to PERMANENT (true)?
> > Does this restore the old value persisted or does it now persist the
> > previous ONETIME value?
> 
> The changed value of the Mode property is persisted.

So 'persist' property as defined here is a meaningless?  Or at least it
doesn't have the same meaning as the word in English does?  It seems
like you are _really_ letting the specific IPMI behavior of this
particular host firmware dictate the DBus object definition.  This is a
huge problem.

> 
> > 3. As a user, how can I identify what the PERMANENT value is if someone
> > has temporarily done a ONETIME?  I have to wait until the ONETIME is
> > consumed?
> 
> Yes, and the settings app doesn't do anything today to restore the 
> permanent value.
> 
> > It seems like having an optional, perhaps dynamically create, second
> > object to separate PERMANENT and ONETIME would take care of this,
> > wouldn't it?  Is there a reason you did not want to have two settings
> > objects to represent this?
> 
> What I've understood from Vishwa is that what's happening with 
> host-ipmid is, the host consumes the ONETIME setting and then restores a 
> value to be used as PERMANENT and also sets the policy as PERMANENT. The 
> settings application at the moment has no logic to specially handle 
> ONETIME vs PERMANENT. It's based on the assumption that a user will 
> consume the ONETIME setting, and then that (or possibly another) user 
> will restore the PERMANENT setting.
> 

We should not have a design that relies exclusively on some host
firmware (or user after-the-fact) behavior.  If something is a "onetime"
/ "temporary" setting, it should not be overwriting the "permanent"
value we are storing.  If we ever should revert to a non-temporary
version it should always be the previous permanent.

> Regards,
> Deepak
> 

-- 
Patrick Williams
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20170817/5dada874/attachment.sig>