RFC: new design of phosphor-time-manager on sdbusplus

[Mine]

On Sat, Jan 21, 2017 at 3:08 AM, Patrick Williams <patrick at stwcx.xyz> wrote:
> On Thu, Jan 19, 2017 at 03:37:34PM +0800, Mine wrote:
>> Currently the time_mode/owner has 8 combinations, some of them are not valid.
>> Could you help to verify what's the expected behavior in each combinations?
>>
>> Mode      | Owner | Set BMC Time  | Set Host Time
>> ------------- | --------- | ----------------------- |
>> -------------------------------------
>> NTP        | BMC   | Not allowed      | Not allowed
>> NTP        | HOST | Invalid case     | Invalid case
>> NTP        | SPLIT | Not allowed      | OK, and just save offset
>> NTP        | BOTH | Not allowed     | OK, and set time to BMC
>> MANUAL | BMC   | OK                  | Not allowed
>> MANUAL | HOST | Not allowed      | OK, and set time to BMC
>> MANUAL | SPLIT | OK                  | OK, and just save offset
>> MANUAL | BOTH | OK                  | OK, and set time to BMC
>
> Why is 'Set BMC Time' not allowed in any NTP mode in your table?  There
> is no reason to stop 'Set BMC Time' except in the NTP/HOST case.

The current implementation uses this logic. (time-manager.cpp:300)
Actually the above table is the summary of the current implementation's logic.

>
>> If my understanding is correct, then we can see:
>> * NTP/HOST is invalid case, such case shall not exist, the current
>> implementation
>>   either hacks settingsd to change the mode to MANUAL, or just behaves
>> like MANUAL;
>
> When the 'host' owns the clock, 'ntp' vs 'manual' is irrelevant.

OK

>
>> * NTP/BOTH has the same behavior as MANULA/HOST, they can be merged.
>
> No, these are significantly different.  NTP/BOTH means that we run the
> NTP client on the BMC but allow either side to set the time.
> MANUAL/HOST means we do not run the NTP client and the BMC is prohibited
> from setting the time in any way.

Yup, with or without NTP means BMC will run or disable NTP client.
But the "same behavior" I mean here is to refer that if set BMC/HOST
time is allowed
or not, the current logic:
NTP/BOTH: setting BMC time is not allowed (you mentioned above that this shall
be allowed, but the current implementation does not allow it); setting
HOST time is allowed.
MANUAL/HOST: setting BMC time is not allowed, setting HOST time is allowed.
They are the same.

But if your suggestion that in NTP/BOTH setting BMC is allowed, then
the behavior will be
different.

>
>> That's why I propose to combine the settings to:
>> * NTP-BMC
>> * NTP-SPLIT
>> * NTP-BOTH (or MANUAL-HOST)
>> * MANUAL-BMC
>> * MANUAL-SPLIT
>> * MANUAL-BOTH
>>
>> Then we don't have to "hack" or "disagree" with settingsd, and the
>> logic could be
>> a little simpler.
>
> 1. NTP vs Manual controls if the NTP-client is running.
> 2. Owner controls who is allowed to set the time.

Sure. Let's keep the current mode and time separately.
But please confirm the behavior of setting BCM/HOST time in each
combination (the table)

>
> --
> Patrick Williams