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

[Mine]

Hi Vishwa,

On Wed, Jan 18, 2017 at 7:07 PM, vishwa <vishwa at linux.vnet.ibm.com> wrote:
> On 01/17/2017 01:21 PM, Mine wrote:
>>
>> Hi Patrick,
>>
>> On Tue, Jan 17, 2017 at 3:44 AM, Patrick Williams <patrick at stwcx.xyz>
>> wrote:
>>>
>>> On Fri, Jan 13, 2017 at 03:42:50PM +0800, Mine wrote:
>>>>
>>>> New Design
>>>>
>>>> Create two objects:
>>>
>>> You mean 'classes' or 'objects'?  I think you mean two classes, which
>>> initially we will only have 1 instance of each.  Please make sure the
>>> HostEpoch implementation can relatively-easily be enhanced to have
>>> multiple instances.
>>
>> I should be meant to create two "instances" on the bus, eventually it
>> looks
>> something like:
>> ```
>> xyz.openbmc_project.Time.Manager
>>      /xyz/openbmc_project/time/host_epoch
>>          org.freedesktop.DBus.Peer
>>          org.freedesktop.DBus.Introspectable
>>          org.freedesktop.DBus.Properties
>>          xyz.openbmc_project.Time.EpochTime
>>      /xyz/openbmc_project/time/bmc_epoch
>>          org.freedesktop.DBus.Peer
>>          org.freedesktop.DBus.Introspectable
>>          org.freedesktop.DBus.Properties
>>          xyz.openbmc_project.Time.EpochTime
>>
>> ```
>> The service's BUSNAME is still `xyz.openbmc_project.Time.Manager`, and
>> it creates two objects, one for BmcEpoch and the other for HostEpoch, who
>> both implement xyz.openbmc_project.Time.EpochTime interface.
>
> So TimeManager still owns the responsibility of maintaining offsets etc
> right ?

Not exactly, HostEpoch will own the offset and maintain it.
But yes, overall HostEpoch is part of TimeManager, so you can say
TimeManager owns the offset as well.

>
>>>>     - BmcEpoch
>>>>     - HostEpoch
>>>>
>>>> They both implements EpochTime interface.
>>>>
>>>> For BmcEpoch:
>>>>
>>>>     - When elapsed() is called, return BMC time;
>>>>     - When elapsed(us) is called, use above SetTime(“bmc”) logic
>>>>
>>>> For HostEpoch:
>>>>
>>>>     - When elapsed() is called, return HOST time;
>>>>     - When elapsed(us) is called, use above SetTime(“host”) logic.
>>>
>>> Seems ok.
>>>
>>> The errors also need to be converted to dbus-defined errors, right?
>>>
>>>> And there will be no “curr_time_mode/owner” or
>>>> “requested_time_mode/owner”
>>>> properties on DBUS.
>>>
>>> So these are only stored in phosphor-settingsd now or are they also used
>>> internally for decisions?  I believe the previous implementation had
>>> them exposed more for debug purposes.  Are you going to add them to the
>>> journal at least?
>>
>> Yes, the time_mode and time_owner are still stored in
>> phosphor-settingsd, and they
>> are used internally by phosphor-time-manager, which will register
>> callback for the
>> settings' change and handled accordingly.
>> The difference from the previous implementation is that we do not expose
>> these
>> settings in time-manager's DBus now.
>>
>> What do you mean by "add them to the journal"?
>
> I believe he is asking you to put that information in the journal traces
so
> that we can use them for debug. I think you don't need anything more to
> cater to that request as the original implementation already puts those as
> journal traces.

Sure.
The code will use sdbusplus's logging to send journal logs.

>>
>>
>>> --
>>> Patrick Williams
>>
>> _______________________________________________
>> openbmc mailing list
>> openbmc at lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/openbmc
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20170118/e3f6e4e5/attachment.html>