commit 33752c75e8bdadf13d983e94667de72f64ba3537
author Lei YU <mine260309@gmail.com> Thu Jun 07 17:06:58 2018 +0800
committer Lei YU <mine260309@gmail.com> Wed Jun 13 11:30:20 2018 +0800
tree 7b0fb9252dbf6db6d7e9c241a62656b15089f88e
parent 85c04d664dfaf5f7ecf7e76a9abd225b1e76d7e1

Throw excpetion when it is not allowed to set time

When it is not allowed to set time depending on the time setting,
previously we only log an error and continue.
Now sdbusplus supports errors on properties, so we can throw exception
on such case.

Tested: Verify in unittest that exception is thrown when it is not
        allowed to set time.
        Verify in BMC that busctl gets the error message when it is not
        allowed to set time.
Change-Id: I4a04d1aa8c081abf0f9fd449118dc1107e12f689
Signed-off-by: Lei YU <mine260309@gmail.com>

bmc_epoch.cpp

diff --git a/bmc_epoch.cpp b/bmc_epoch.cpp
index c043df4..c06c464 100644
--- a/bmc_epoch.cpp
+++ b/bmc_epoch.cpp
@@ -24,6 +24,7 @@
 {
 namespace server = sdbusplus::xyz::openbmc_project::Time::server;
 using namespace phosphor::logging;
+using namespace sdbusplus::xyz::openbmc_project::Common::Error;
 
 BmcEpoch::BmcEpoch(sdbusplus::bus::bus& bus,
                    const char* objPath)
@@ -107,8 +108,7 @@
     if (timeOwner == Owner::Host)
     {
         log<level::ERR>("Setting BmcTime with HOST owner is not allowed");
-        // TODO: throw NotAllowed exception
-        return 0;
+        elog<InsufficientPermission>();
     }
 
     auto time = microseconds(value);

host_epoch.cpp

diff --git a/host_epoch.cpp b/host_epoch.cpp
index 8415f15..ea3026c 100644
--- a/host_epoch.cpp
+++ b/host_epoch.cpp
@@ -1,12 +1,15 @@
 #include "host_epoch.hpp"
 #include "utils.hpp"
 
+#include <phosphor-logging/elog-errors.hpp>
 #include <phosphor-logging/log.hpp>
+#include <xyz/openbmc_project/Common/error.hpp>
 
 namespace phosphor
 {
 namespace time
 {
+using namespace sdbusplus::xyz::openbmc_project::Common::Error;
 using namespace sdbusplus::xyz::openbmc_project::Time;
 using namespace phosphor::logging;
 using namespace std::chrono;
@@ -51,8 +54,7 @@
          && (timeOwner == Owner::Host || timeOwner == Owner::Both)))
     {
         log<level::ERR>("Setting HostTime is not allowed");
-        // TODO: throw NotAllowed exception
-        return 0;
+        elog<InsufficientPermission>();
     }
 
     auto time = microseconds(value);

test/TestBmcEpoch.cpp

diff --git a/test/TestBmcEpoch.cpp b/test/TestBmcEpoch.cpp
index 32592e6..2d0bc72 100644
--- a/test/TestBmcEpoch.cpp
+++ b/test/TestBmcEpoch.cpp
@@ -1,12 +1,13 @@
-#include <sdbusplus/bus.hpp>
-#include <gtest/gtest.h>
-#include <memory>
-
 #include "bmc_epoch.hpp"
 #include "config.h"
 #include "types.hpp"
 #include "mocked_bmc_time_change_listener.hpp"
 
+#include <gtest/gtest.h>
+#include <memory>
+#include <sdbusplus/bus.hpp>
+#include <xyz/openbmc_project/Common/error.hpp>
+
 namespace phosphor
 {
 namespace time
@@ -14,6 +15,8 @@
 
 using ::testing::_;
 using namespace std::chrono;
+using InsufficientPermission =
+    sdbusplus::xyz::openbmc_project::Common::Error::InsufficientPermission;
 
 class TestBmcEpoch : public testing::Test
 {
@@ -88,8 +91,9 @@
     // In Host owner, setting time is not allowed
     setTimeMode(Mode::Manual);
     setTimeOwner(Owner::Host);
-    auto ret = bmcEpoch->elapsed(epochNow);
-    EXPECT_EQ(0, ret);
+    EXPECT_THROW(
+        bmcEpoch->elapsed(epochNow),
+        InsufficientPermission);
 }
 
 TEST_F(TestBmcEpoch, setElapsedOK)

test/TestHostEpoch.cpp

diff --git a/test/TestHostEpoch.cpp b/test/TestHostEpoch.cpp
index a492b9a..6c460ad 100644
--- a/test/TestHostEpoch.cpp
+++ b/test/TestHostEpoch.cpp
@@ -1,11 +1,14 @@
-#include <sdbusplus/bus.hpp>
-#include <gtest/gtest.h>
-
 #include "host_epoch.hpp"
 #include "utils.hpp"
 #include "config.h"
 #include "types.hpp"
 
+#include <xyz/openbmc_project/Common/error.hpp>
+
+#include <sdbusplus/bus.hpp>
+#include <gtest/gtest.h>
+
+
 namespace phosphor
 {
 namespace time
@@ -13,6 +16,8 @@
 
 using namespace std::chrono;
 using namespace std::chrono_literals;
+using InsufficientPermission =
+    sdbusplus::xyz::openbmc_project::Common::Error::InsufficientPermission;
 
 const constexpr microseconds USEC_ZERO{0};
 
@@ -73,9 +78,10 @@
             // Set time is not allowed,
             // so verify offset is still 0 after set time
             microseconds diff = 1min;
-            hostEpoch.elapsed(hostEpoch.elapsed() + diff.count());
+            EXPECT_THROW(
+                hostEpoch.elapsed(hostEpoch.elapsed() + diff.count()),
+                InsufficientPermission);
             EXPECT_EQ(0, getOffset().count());
-            // TODO: when gmock is ready, check there is no call to timedatectl
         }
 
         void checkSetSplitTimeInFuture()