Skip to content

Commit

Permalink
Remove TimeOwner Feature
Browse files Browse the repository at this point in the history
The TimeOwner feature is confusing from feedback from the community and
hence removing the feature.
Remove the TimeOwner feature in the phosphor-time-manager repo and
needed settings objects.

Tested: Manually set the date time on the web and successfully update
        the date time of BMC (eg: 2020/01/01 08:07:50).
        busctrl get-property xyz.openbmc_project.Time.Manager
                /xyz/openbmc_project/time/bmc
                xyz.openbmc_project.Time.EpochTime Elapsed
        t 1577837156385836

Refer: https://lists.ozlabs.org/pipermail/openbmc/2020-April/021409.html

Signed-off-by: George Liu <liuxiwei@inspur.com>
Change-Id: Id47eb0a03e0e94eeff29d2b77dccefb89cded7b8
  • Loading branch information
lxwinspur authored and vishwabmc committed May 19, 2020
1 parent c09ac3f commit 3c2f449
Show file tree
Hide file tree
Showing 27 changed files with 32 additions and 989 deletions.
1 change: 0 additions & 1 deletion Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ noinst_LTLIBRARIES = libtimemanager.la
libtimemanager_la_SOURCES = \
epoch_base.cpp \
bmc_epoch.cpp \
host_epoch.cpp \
manager.cpp \
utils.cpp \
settings.cpp \
Expand Down
53 changes: 14 additions & 39 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
# Introduction
`phosphor-time-manager` is the time manager service that implements D-Bus
interface `xyz/openbmc_project/Time/EpochTime.interface.yaml`.
The user can get or set the BMC's or HOST's time via this interface.
The user can get or set the BMC's time via this interface.

### General usage
The service `xyz.openbmc_project.Time.Manager` provides two objects on D-Bus:
The service `xyz.openbmc_project.Time.Manager` provides an object on D-Bus:
* /xyz/openbmc_project/time/bmc
* /xyz/openbmc_project/time/host

where each object implements interface `xyz.openbmc_project.Time.EpochTime`.

Expand All @@ -22,44 +21,33 @@ the time. For example on an authenticated session:
### With REST API on remote host
curl -b cjar -k https://${BMC_IP}/xyz/openbmc_project/time/bmc
```
* To set HOST's time:
* To set BMC's time:
```
### With busctl on BMC
busctl set-property xyz.openbmc_project.Time.Manager \
/xyz/openbmc_project/time/host xyz.openbmc_project.Time.EpochTime \
/xyz/openbmc_project/time/bmc xyz.openbmc_project.Time.EpochTime \
Elapsed t <value-in-microseconds>
### With REST API on remote host
curl -b cjar -k -H "Content-Type: application/json" -X PUT \
-d '{"data": 1487304700000000}' \
https://${BMC_IP}/xyz/openbmc_project/time/host/attr/Elapsed
https://${BMC_IP}/xyz/openbmc_project/time/bmc/attr/Elapsed
```

### Time settings
Getting BMC or HOST time is always allowed, but setting the time may not be
Getting BMC time is always allowed, but setting the time may not be
allowed depending on the below two settings in the settings manager.

* TimeSyncMethod
* NTP: The time is set via NTP server.
* MANUAL: The time is set manually.
* TimeOwner
* BMC: BMC owns the time and can set the time.
* HOST: Host owns the time and can set the time.
* SPLIT: BMC and Host own separate time.
* BOTH: Both BMC and Host can set the time.

A summary of which cases the time can be set on BMC or HOST:

Mode | Owner | Set BMC Time | Set Host Time
--------- | ----- | ------------- | -------------------
NTP | BMC | Fail to set | Not allowed
NTP | HOST | Not allowed | Not allowed
NTP | SPLIT | Fail to set | OK
NTP | BOTH | Fail to set | Not allowed
MANUAL | BMC | OK | Not allowed
MANUAL | HOST | Not allowed | OK
MANUAL | SPLIT | OK | OK
MANUAL | BOTH | OK | OK
Mode | Set BMC Time
--------- | -------------
NTP | Fail to set
MANUAL | OK

* To set an NTP [server](https://tf.nist.gov/tf-cgi/servers.cgi):
```
Expand Down Expand Up @@ -88,19 +76,6 @@ MANUAL | BOTH | OK | OK
https://${BMC_IP}/xyz/openbmc_project/time/sync_method/attr/TimeSyncMethod
```

* To change owner
```
### With busctl on BMC
busctl set-property xyz.openbmc_project.Settings \
/xyz/openbmc_project/time/owner xyz.openbmc_project.Time.Owner \
TimeOwner s xyz.openbmc_project.Time.Owner.Owners.BMC
### With REST API on remote host
curl -c cjar -b cjar -k -H "Content-Type: application/json" -X PUT -d \
'{"data": "xyz.openbmc_project.Time.Owner.Owners.BMC" }' \
https://${BMC_IP}/xyz/openbmc_project/time/owner/attr/TimeOwner
```

### Special note on changing NTP setting
Starting from OpenBMC 2.6 (with systemd v239), systemd's timedated introduces
a new beahvior that it checks the NTP services' status during setting time,
Expand All @@ -115,13 +90,13 @@ This results in [openbmc/openbmc#3459][1], and the related test cases are
updated to cooperate with this behavior change.

### Special note on host on
When the host is on, the changes of the above time mode/owner are not applied but
deferred. The changes of the mode/owner are saved to persistent storage.
When the host is on, the changes of the above time mode are not applied but
deferred. The changes of the mode are saved to persistent storage.

When the host is off, the saved mode/owner are read from persistent storage and are
When the host is off, the saved mode are read from persistent storage and are
applied.

Note: A user can set the time mode and owner in the settings daemon at any time,
Note: A user can set the time mode in the settings daemon at any time,
but the time manager applying them is governed by the above condition.


Expand Down
53 changes: 5 additions & 48 deletions bmc_epoch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include <phosphor-logging/elog.hpp>
#include <phosphor-logging/log.hpp>
#include <xyz/openbmc_project/Common/error.hpp>
#include <xyz/openbmc_project/Time/error.hpp>

// Need to do this since its not exported outside of the kernel.
// Refer : https://gist.github.com/lethean/446cea944b7441228298
Expand All @@ -27,9 +26,6 @@ namespace time
namespace server = sdbusplus::xyz::openbmc_project::Time::server;
using namespace phosphor::logging;

using NotAllowedError =
sdbusplus::xyz::openbmc_project::Time::Error::NotAllowed;

BmcEpoch::BmcEpoch(sdbusplus::bus::bus& bus, const char* objPath) :
EpochBase(bus, objPath), bus(bus)
{
Expand Down Expand Up @@ -84,73 +80,34 @@ BmcEpoch::~BmcEpoch()

uint64_t BmcEpoch::elapsed() const
{
// It does not needs to check owner when getting time
return getTime().count();
}

uint64_t BmcEpoch::elapsed(uint64_t value)
{
/*
Mode | Owner | Set BMC Time
----- | ----- | -------------
NTP | BMC | Fail to set
NTP | HOST | Not allowed
NTP | SPLIT | Fail to set
NTP | BOTH | Fail to set
MANUAL| BMC | OK
MANUAL| HOST | Not allowed
MANUAL| SPLIT | OK
MANUAL| BOTH | OK
Mode | Set BMC Time
----- | -------------
NTP | Fail to set
MANUAL| OK
*/
if (timeOwner == Owner::Host)
{
using namespace xyz::openbmc_project::Time;
elog<NotAllowedError>(
NotAllowed::OWNER(utils::ownerToStr(timeOwner).c_str()),
NotAllowed::SYNC_METHOD(utils::modeToStr(timeMode).c_str()),
NotAllowed::REASON(
"Setting BmcTime with HOST owner is not allowed"));
}

auto time = microseconds(value);
if (setTime(time))
{
notifyBmcTimeChange(time);
}
setTime(time);

server::EpochTime::elapsed(value);
return value;
}

void BmcEpoch::setBmcTimeChangeListener(BmcTimeChangeListener* listener)
{
timeChangeListener = listener;
}

void BmcEpoch::notifyBmcTimeChange(const microseconds& time)
{
// Notify listener if it exists
if (timeChangeListener)
{
timeChangeListener->onBmcTimeChanged(time);
}
}

int BmcEpoch::onTimeChange(sd_event_source* es, int fd, uint32_t /* revents */,
void* userdata)
{
auto bmcEpoch = static_cast<BmcEpoch*>(userdata);

std::array<char, 64> time{};

// We are not interested in the data here.
// So read until there is no new data here in the FD
while (read(fd, time.data(), time.max_size()) > 0)
;

log<level::INFO>("BMC system time is changed");
bmcEpoch->notifyBmcTimeChange(bmcEpoch->getTime());

return 0;
}

Expand Down
16 changes: 0 additions & 16 deletions bmc_epoch.hpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#pragma once

#include "bmc_time_change_listener.hpp"
#include "epoch_base.hpp"

#include <chrono>
Expand Down Expand Up @@ -39,25 +38,13 @@ class BmcEpoch : public EpochBase
**/
uint64_t elapsed(uint64_t value) override;

/** @brief Set the listner for bmc time change
*
* @param[in] listener - The pointer to the listener
*/
void setBmcTimeChangeListener(BmcTimeChangeListener* listener);

private:
/** @brief The fd for time change event */
int timeFd = -1;

/** @brief Initialize timerFd related resource */
void initialize();

/** @brief Notify the listeners that bmc time is changed
*
* @param[in] time - The epoch time in microseconds to notify
*/
void notifyBmcTimeChange(const microseconds& time);

/** @brief The callback function on system time change
*
* @param[in] es - Source of the event
Expand All @@ -84,9 +71,6 @@ class BmcEpoch : public EpochBase

/** @brief The event source on system time change */
SdEventSource timeChangeEventSource{nullptr, sdEventSourceDeleter};

/** @brief The listener for bmc time change */
BmcTimeChangeListener* timeChangeListener = nullptr;
};

} // namespace time
Expand Down
23 changes: 0 additions & 23 deletions bmc_time_change_listener.hpp

This file was deleted.

12 changes: 0 additions & 12 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -65,21 +65,9 @@ AC_ARG_VAR(OBJPATH_BMC, [The bmc epoch Dbus root])
AS_IF([test "x$OBJPATH_BMC" == "x"], [OBJPATH_BMC="/xyz/openbmc_project/time/bmc"])
AC_DEFINE_UNQUOTED([OBJPATH_BMC], ["$OBJPATH_BMC"], [The bmc epoch Dbus root])

AC_ARG_VAR(OBJPATH_HOST, [The host epoch Dbus root])
AS_IF([test "x$OBJPATH_HOST" == "x"], [OBJPATH_HOST="/xyz/openbmc_project/time/host"])
AC_DEFINE_UNQUOTED([OBJPATH_HOST], ["$OBJPATH_HOST"], [The host epoch Dbus root])

AC_ARG_VAR(HOST_OFFSET_FILE, [The file to save host time offset])
AS_IF([test "x$HOST_OFFSET_FILE" == "x"], [HOST_OFFSET_FILE="/var/lib/obmc/saved_host_offset"])
AC_DEFINE_UNQUOTED([HOST_OFFSET_FILE], ["$HOST_OFFSET_FILE"], [The file to save host time offset])

AC_ARG_VAR(DEFAULT_TIME_MODE, [The default time mode])
AS_IF([test "x$DEFAULT_TIME_MODE" == "x"], [DEFAULT_TIME_MODE=Mode::Manual])
AC_DEFINE_UNQUOTED([DEFAULT_TIME_MODE], [$DEFAULT_TIME_MODE], [The default time mode])

AC_ARG_VAR(DEFAULT_TIME_OWNER, [The default time owner])
AS_IF([test "x$DEFAULT_TIME_OWNER" == "x"], [DEFAULT_TIME_OWNER=Owner::Both])
AC_DEFINE_UNQUOTED([DEFAULT_TIME_OWNER], [$DEFAULT_TIME_OWNER], [The default time owner])

AC_CONFIG_FILES([Makefile test/Makefile])
AC_OUTPUT
5 changes: 0 additions & 5 deletions epoch_base.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,6 @@ void EpochBase::onModeChanged(Mode mode)
timeMode = mode;
}

void EpochBase::onOwnerChanged(Owner owner)
{
timeOwner = owner;
}

using namespace std::chrono;
bool EpochBase::setTime(const microseconds& usec)
{
Expand Down
6 changes: 0 additions & 6 deletions epoch_base.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,13 @@ class EpochBase : public sdbusplus::server::object::object<
/** @brief Notified on time mode changed */
void onModeChanged(Mode mode) override;

/** @brief Notified on time owner changed */
void onOwnerChanged(Owner owner) override;

protected:
/** @brief Persistent sdbusplus DBus connection */
sdbusplus::bus::bus& bus;

/** @brief The current time mode */
Mode timeMode = DEFAULT_TIME_MODE;

/** @brief The current time owner */
Owner timeOwner = DEFAULT_TIME_OWNER;

/** @brief Set current time to system
*
* This function set the time to system by invoking systemd
Expand Down
Loading

0 comments on commit 3c2f449

Please sign in to comment.