Reduce journal logs
Presently the code was creating the elog during reading of
the values from the config parser which creates
the journal log internally("Internal error Occured")
Apart from creating elog,Code also logs into the journal
more descriptive message saying which section and key
was not found.
The code which catches the exception that also logs
into the journal.
In the changed approach instead of throwing exception
we are returning the return code as not finding a section
or key in the network config file is not an error.
Change-Id: Ib89a3af6541fdf93fb5fa9a8e583d6c6ed88da79
Signed-off-by: Ratan Gupta <ratagupt@in.ibm.com>
diff --git a/config_parser.cpp b/config_parser.cpp
index 9ab601c..3852b6c 100644
--- a/config_parser.cpp
+++ b/config_parser.cpp
@@ -1,7 +1,5 @@
#include "config_parser.hpp"
-#include "xyz/openbmc_project/Common/error.hpp"
#include <phosphor-logging/log.hpp>
-#include <phosphor-logging/elog-errors.hpp>
#include <fstream>
#include <string>
@@ -18,7 +16,6 @@
{
using namespace phosphor::logging;
-using namespace sdbusplus::xyz::openbmc_project::Common::Error;
Parser::Parser(const fs::path& filePath)
{
@@ -26,58 +23,66 @@
}
-KeyValues Parser::getSection(const std::string& section)
+std::tuple<ReturnCode, KeyValueMap> Parser::getSection(const std::string& section)
{
auto it = sections.find(section);
if (it == sections.end())
{
- log<level::ERR>("ConfigParser: Section not found",
- entry("SECTION=%s",section));
- elog<InternalFailure>();
+ KeyValueMap keyValues;
+ return std::make_tuple(ReturnCode::SECTION_NOT_FOUND,
+ std::move(keyValues));
}
- return it->second;
+
+ return std::make_tuple(ReturnCode::SUCCESS, it->second);
}
-std::vector<std::string> Parser::getValues(const std::string& section,
- const std::string& key)
+std::tuple<ReturnCode, ValueList> Parser::getValues(const std::string& section,
+ const std::string& key)
{
- std::vector<std::string> values;
- auto keyValues = getSection(section);
+ ValueList values;
+ KeyValueMap keyValues {};
+ auto rc = ReturnCode::SUCCESS;
+
+ std::tie(rc, keyValues) = getSection(section);
+ if (rc != ReturnCode::SUCCESS)
+ {
+ return std::make_tuple(rc, std::move(values));
+ }
+
auto it = keyValues.find(key);
if (it == keyValues.end())
{
- log<level::ERR>("ConfigParser: Key not found",
- entry("KEY=%s",key));
- elog<InternalFailure>();
+ return std::make_tuple(ReturnCode::KEY_NOT_FOUND, std::move(values));
}
+
for (; it != keyValues.end() && key == it->first; it++)
{
values.push_back(it->second);
}
- return values;
+
+ return std::make_tuple(ReturnCode::SUCCESS, std::move(values));
}
bool Parser::isValueExist(const std::string& section, const std::string& key,
const std::string& value)
{
- try
+ auto rc = ReturnCode::SUCCESS;
+ ValueList values;
+ std::tie(rc, values) = getValues(section, key);
+
+ if (rc != ReturnCode::SUCCESS)
{
- auto values = getValues(section, key);
- auto it = std::find(values.begin(), values.end(), value);
- return it != std::end(values) ? true : false;
+ return false;
}
- catch (InternalFailure& e)
- {
- commit<InternalFailure>();
- }
- return false;
+ auto it = std::find(values.begin(), values.end(), value);
+ return it != std::end(values) ? true : false;
}
void Parser::setValue(const std::string& section, const std::string& key,
const std::string& value)
{
- KeyValues values;
+ KeyValueMap values;
auto it = sections.find(section);
if (it != sections.end())
{
diff --git a/config_parser.hpp b/config_parser.hpp
index bf5cb2d..f3e4a70 100644
--- a/config_parser.hpp
+++ b/config_parser.hpp
@@ -2,6 +2,7 @@
#include <string>
#include <map>
+#include <tuple>
#include <unordered_map>
#include <vector>
#include <experimental/filesystem>
@@ -14,9 +15,18 @@
{
using Section = std::string;
-using KeyValues = std::multimap<std::string, std::string>;
+using KeyValueMap = std::multimap<std::string, std::string>;
+using ValueList = std::vector<std::string>;
+
namespace fs = std::experimental::filesystem;
+enum class ReturnCode
+{
+ SUCCESS = 0x0,
+ SECTION_NOT_FOUND = 0x1,
+ KEY_NOT_FOUND = 0x2,
+};
+
class Parser
{
public:
@@ -32,11 +42,13 @@
/** @brief Get the values of the given key and section.
* @param[in] section - section name.
* @param[in] key - key to look for.
- * @returns the values associated with the key.
+ * @returns the tuple of return code and the
+ * values associated with the key.
*/
- std::vector<std::string> getValues(const std::string& section,
- const std::string& key);
+ std::tuple<ReturnCode, ValueList> getValues(
+ const std::string& section,
+ const std::string& key);
/** @brief Set the value of the given key and section.
* @param[in] section - section name.
@@ -64,10 +76,10 @@
/** @brief Get all the key values of the given section.
* @param[in] section - section name.
- * @returns the map of the key and value.
+ * @returns the tuple of return code and the map of (key,value).
*/
- KeyValues getSection(const std::string& section);
+ std::tuple<ReturnCode, KeyValueMap> getSection(const std::string& section);
/** @brief checks that whether the value exist in the
* given section.
@@ -80,7 +92,7 @@
bool isValueExist(const std::string& section, const std::string& key,
const std::string& value);
- std::unordered_map<Section, KeyValues> sections;
+ std::unordered_map<Section, KeyValueMap> sections;
fs::path filePath;
};
diff --git a/ethernet_interface.cpp b/ethernet_interface.cpp
index 15f40fb..7d13c1c 100644
--- a/ethernet_interface.cpp
+++ b/ethernet_interface.cpp
@@ -398,14 +398,14 @@
systemd::config::networkFileSuffix;
confPath /= fileName;
ServerList servers;
- try
+ config::Parser parser(confPath.string());
+ auto rc = config::ReturnCode::SUCCESS;
+
+ std::tie(rc, servers) = parser.getValues("Network", "DNS");
+ if (rc != config::ReturnCode::SUCCESS)
{
- config::Parser parser(confPath.string());
- servers = parser.getValues("Network", "DNS");
- }
- catch (InternalFailure& e)
- {
- log<level::INFO>("Exception getting DNS value from conf file");
+ log<level::DEBUG>("Unable to get the value for network[DNS]",
+ entry("RC=%d", rc));
}
return servers;
}
@@ -484,18 +484,20 @@
fs::path confPath = manager.getConfDir();
std::string fileName = systemd::config::networkFilePrefix + interfaceName() +
- systemd::config::networkFileSuffix;
+ systemd::config::networkFileSuffix;
confPath /= fileName;
+
ServerList servers;
- try
+ config::Parser parser(confPath.string());
+ auto rc = config::ReturnCode::SUCCESS;
+
+ std::tie(rc, servers) = parser.getValues("Network", "NTP");
+ if (rc != config::ReturnCode::SUCCESS)
{
- config::Parser parser(confPath.string());
- servers = parser.getValues("Network", "NTP");
+ log<level::DEBUG>("Unable to get the value for Network[NTP]",
+ entry("rc=%d", rc));
}
- catch (InternalFailure& e)
- {
- log<level::INFO>("Unable to find the NTP server configuration.");
- }
+
return servers;
}
diff --git a/test/test_config_parser.cpp b/test/test_config_parser.cpp
index a07390b..1c902d2 100644
--- a/test/test_config_parser.cpp
+++ b/test/test_config_parser.cpp
@@ -46,17 +46,20 @@
TEST_F(TestConfigParser, ReadConfigDataFromFile)
{
- auto values = parser.getValues("Network", "DHCP");
+ config::ReturnCode rc = config::ReturnCode::SUCCESS;
+ config::ValueList values;
+
+ std::tie(rc, values) = parser.getValues("Network", "DHCP");
std::string expectedValue = "true";
bool found = isValueFound(values, expectedValue);
EXPECT_EQ(found, true);
- values = parser.getValues("DHCP", "ClientIdentifier");
+ std::tie(rc, values) = parser.getValues("DHCP", "ClientIdentifier");
expectedValue = "mac";
found = isValueFound(values, expectedValue);
EXPECT_EQ(found, true);
- values = parser.getValues("Match", "Name");
+ std::tie(rc, values) = parser.getValues("Match", "Name");
expectedValue = "eth0";
found = isValueFound(values, expectedValue);
EXPECT_EQ(found, true);
@@ -64,35 +67,20 @@
TEST_F(TestConfigParser, SectionNotExist)
{
- using namespace sdbusplus::xyz::openbmc_project::Common::Error;
- bool caughtException = false;
- try
- {
- parser.getValues("abc", "ipaddress");
- }
- catch (const std::exception& e)
- {
- caughtException = true;
- }
- EXPECT_EQ(true, caughtException);
+ config::ReturnCode rc = config::ReturnCode::SUCCESS;
+ config::ValueList values;
+ std::tie(rc, values) = parser.getValues("abc", "ipaddress");
+ EXPECT_EQ(config::ReturnCode::SECTION_NOT_FOUND, rc);
}
TEST_F(TestConfigParser, KeyNotFound)
{
- using namespace sdbusplus::xyz::openbmc_project::Common::Error;
- bool caughtException = false;
- try
- {
- parser.getValues("Network", "abc");
- }
- catch (const std::exception& e)
- {
- caughtException = true;
- }
- EXPECT_EQ(true, caughtException);
+ config::ReturnCode rc = config::ReturnCode::SUCCESS;
+ config::ValueList values;
+ std::tie(rc, values) = parser.getValues("Network", "abc");
+ EXPECT_EQ(config::ReturnCode::KEY_NOT_FOUND, rc);
remove("/tmp/eth0.network");
}
}//namespace network
}//namespace phosphor
-
diff --git a/test/test_ethernet_interface.cpp b/test/test_ethernet_interface.cpp
index 27fbb67..2181006 100644
--- a/test/test_ethernet_interface.cpp
+++ b/test/test_ethernet_interface.cpp
@@ -197,7 +197,9 @@
fs::path filePath = confDir;
filePath /= "00-bmc-test0.network";
config::Parser parser(filePath.string());
- auto values = parser.getValues("Network", "DNS");
+ config::ReturnCode rc = config::ReturnCode::SUCCESS;
+ config::ValueList values;
+ std::tie(rc, values) = parser.getValues("Network", "DNS");
EXPECT_EQ(servers , values);
validateResolvFile(values);
@@ -210,7 +212,9 @@
fs::path filePath = confDir;
filePath /= "00-bmc-test0.network";
config::Parser parser(filePath.string());
- auto values = parser.getValues("Network", "NTP");
+ config::ReturnCode rc = config::ReturnCode::SUCCESS;
+ config::ValueList values;
+ std::tie(rc, values) = parser.getValues("Network", "NTP");
EXPECT_EQ(servers , values);
}
diff --git a/test/test_vlan_interface.cpp b/test/test_vlan_interface.cpp
index 9711a1a..00384f1 100644
--- a/test/test_vlan_interface.cpp
+++ b/test/test_vlan_interface.cpp
@@ -123,17 +123,20 @@
filePath /= "test0.50.netdev";
config::Parser parser(filePath.string());
- auto values = parser.getValues("NetDev", "Name");
+ config::ReturnCode rc = config::ReturnCode::SUCCESS;
+ config::ValueList values;
+
+ std::tie(rc, values) = parser.getValues("NetDev", "Name");
std::string expectedValue = "test0.50";
bool found = isValueFound(values, expectedValue);
EXPECT_EQ(found, true);
- values = parser.getValues("NetDev", "Kind");
+ std::tie(rc, values) = parser.getValues("NetDev", "Kind");
expectedValue = "vlan";
found = isValueFound(values, expectedValue);
EXPECT_EQ(found, true);
- values = parser.getValues("VLAN", "Id");
+ std::tie(rc, values) = parser.getValues("VLAN", "Id");
expectedValue = "50";
found = isValueFound(values, expectedValue);
EXPECT_EQ(found, true);
@@ -163,17 +166,20 @@
fs::path filePath = confDir;
filePath /= "test0.50.netdev";
config::Parser parser(filePath.string());
- auto values = parser.getValues("NetDev", "Name");
+ config::ReturnCode rc = config::ReturnCode::SUCCESS;
+ config::ValueList values;
+
+ std::tie(rc, values) = parser.getValues("NetDev", "Name");
std::string expectedValue = "test0.50";
bool found = isValueFound(values, expectedValue);
EXPECT_EQ(found, true);
- values = parser.getValues("NetDev", "Kind");
+ std::tie(rc, values) = parser.getValues("NetDev", "Kind");
expectedValue = "vlan";
found = isValueFound(values, expectedValue);
EXPECT_EQ(found, true);
- values = parser.getValues("VLAN", "Id");
+ std::tie(rc, values) = parser.getValues("VLAN", "Id");
expectedValue = "50";
found = isValueFound(values, expectedValue);
EXPECT_EQ(found, true);
@@ -181,12 +187,12 @@
filePath = confDir;
filePath /= "test0.60.netdev";
parser.setFile(filePath.string());
- values = parser.getValues("NetDev", "Name");
+ std::tie(rc, values) = parser.getValues("NetDev", "Name");
expectedValue = "test0.60";
found = isValueFound(values, expectedValue);
EXPECT_EQ(found, true);
- values = parser.getValues("VLAN", "Id");
+ std::tie(rc, values) = parser.getValues("VLAN", "Id");
expectedValue = "60";
found = isValueFound(values, expectedValue);
EXPECT_EQ(found, true);
diff --git a/timer.cpp b/timer.cpp
index afd4b8d..7052775 100644
--- a/timer.cpp
+++ b/timer.cpp
@@ -72,7 +72,6 @@
timer->userCallBack();
}
- log<level::INFO>("Timer expired");
sd_event_source_set_enabled(eventSource, SD_EVENT_OFF);
return 0;
}
diff --git a/util.cpp b/util.cpp
index 5e606a7..4b234f8 100644
--- a/util.cpp
+++ b/util.cpp
@@ -411,19 +411,21 @@
systemd::config::networkFileSuffix;
confPath /= fileName;
- try
+ auto rc = config::ReturnCode::SUCCESS;
+ config::ValueList values;
+ config::Parser parser(confPath.string());
+
+ std::tie(rc, values) = parser.getValues("Network", "DHCP");
+ if (rc != config::ReturnCode::SUCCESS)
{
- config::Parser parser(confPath.string());
- auto values = parser.getValues("Network", "DHCP");
- // There will be only single value for DHCP key.
- if (values[0] == "true")
- {
- dhcp = true;
- }
+ log<level::DEBUG>("Unable to get the value for Network[DHCP]",
+ entry("RC=%d", rc));
+ return dhcp;
}
- catch (InternalFailure& e)
+ // There will be only single value for DHCP key.
+ if (values[0] == "true")
{
- log<level::INFO>("Exception occurred during getting of DHCP value");
+ dhcp = true;
}
return dhcp;
}