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

[vishwa]

On 08/17/2017 07:32 PM, Patrick Williams wrote:
> 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.
>
One of the comment from Deepak in the review was : "If something is 
ONETIME, then should we save the corresponding Mode / Source to the 
Flash and I said the answer was "Yes". I see what is being said here in 
the email is different from what the comment was before.

If a setting is ONETIME, then the host comes down and sets the property 
to DEFAULT and does not do anything with PERMANENT.

https://github.com/open-power/petitboot/blob/master/discover/platform-powerpc.c#L874

I don't think so we are introducing any new behavior here. All that was 
done before is being moved to this new D-Bus object.
If something needs to be treated PERMANENT, then it needs to be set by 
the user as it was before.

>> Regards,
>> Deepak
>>