Skip to content

Commit 8d22a83

Browse files
author
Souvik Roy
committed
Outer try catch block in Parser updateVpdKeyword API (#539)
This commit adds an outer try catch block in Parser::updateVpdKeyword API. Currently, this API has multiple return statements for different error scenarios, which reduces the API body's readability and modularity. In order to streamline and handle all errors in a single location, an outer try catch has been added in the API body. Test: ''' Trigger this API by calling WriteKeyword API from D-Bus: root@testhostname2:~# busctl call com.ibm.VPD.Manager /com/ibm/VPD/Manager com.ibm.VPD.Manager ReadKeyword sv "/sys/bus/i2c/drivers/at24/8-0050/eeprom" \(ss\) "UTIL" "D0" v ay 1 1 root@testhostname2:~# busctl call com.ibm.VPD.Manager /com/ibm/VPD/Manager com.ibm.VPD.Manager WriteKeyword sv "/sys/bus/i2c/drivers/at24/8-0050/eeprom" \(ssay\) "UTIL" "D0" 1 0 i 1 root@testhostname2:~# busctl call com.ibm.VPD.Manager /com/ibm/VPD/Manager com.ibm.VPD.Manager ReadKeyword sv "/sys/bus/i2c/drivers/at24/8-0050/eeprom" \(ss\) "UTIL" "D0" v ay 1 0 ''' Change-Id: I66a6c95ca0daaebb0a6f5c80e28ad4e9706d776f Signed-off-by: Souvik Roy <[email protected]>
1 parent b3a36f0 commit 8d22a83

File tree

2 files changed

+121
-83
lines changed

2 files changed

+121
-83
lines changed

‎vpd-manager/include/constants.hpp

+3
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,9 @@ static constexpr auto STR_CMP_SUCCESS = 0;
174174
// Just a random value. Can be adjusted as required.
175175
static constexpr uint8_t MAX_THREADS = 10;
176176

177+
static constexpr auto FAILURE = -1;
178+
static constexpr auto SUCCESS = 0;
179+
177180
constexpr auto bmcStateService = "xyz.openbmc_project.State.BMC";
178181
constexpr auto bmcZeroStateObject = "/xyz/openbmc_project/state/bmc0";
179182
constexpr auto bmcStateInterface = "xyz.openbmc_project.State.BMC";

‎vpd-manager/src/parser.cpp

+118-83
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
#include "parser.hpp"
22

3+
#include "constants.hpp"
4+
35
#include <utility/dbus_utility.hpp>
46
#include <utility/json_utility.hpp>
57
#include <utility/vpd_specific_utility.hpp>
@@ -56,111 +58,144 @@ types::VPDMapVariant Parser::parse()
5658

5759
int Parser::updateVpdKeyword(const types::WriteVpdParams& i_paramsToWriteData)
5860
{
59-
// Update keyword's value on hardware
6061
int l_bytesUpdatedOnHardware = -1;
6162
try
6263
{
63-
std::shared_ptr<ParserInterface> l_vpdParserInstance =
64-
getVpdParserInstance();
65-
l_bytesUpdatedOnHardware =
66-
l_vpdParserInstance->writeKeywordOnHardware(i_paramsToWriteData);
67-
}
68-
catch (const std::exception& l_exception)
69-
{
70-
logging::logMessage(
71-
"Error while updating keyword's value on hardware path " +
72-
m_vpdFilePath + ", error: " + std::string(l_exception.what()));
73-
// TODO : Log PEL
74-
return -1;
75-
}
64+
// Update keyword's value on hardware
65+
try
66+
{
67+
std::shared_ptr<ParserInterface> l_vpdParserInstance =
68+
getVpdParserInstance();
69+
l_bytesUpdatedOnHardware =
70+
l_vpdParserInstance->writeKeywordOnHardware(
71+
i_paramsToWriteData);
72+
}
73+
catch (const std::exception& l_exception)
74+
{
75+
std::string l_errMsg(
76+
"Error while updating keyword's value on hardware path " +
77+
m_vpdFilePath + ", error: " + std::string(l_exception.what()));
7678

77-
auto [l_fruPath, l_inventoryObjPath, l_redundantFruPath] =
78-
jsonUtility::getAllPathsToUpdateKeyword(m_parsedJson, m_vpdFilePath);
79+
// TODO : Log PEL
7980

80-
// If inventory D-bus object path is present, update keyword's value on DBus
81-
if (!l_inventoryObjPath.empty())
82-
{
83-
types::Record l_recordName;
84-
std::string l_interfaceName;
85-
std::string l_propertyName;
86-
types::DbusVariantType l_keywordValue;
81+
throw std::runtime_error(l_errMsg);
82+
}
8783

88-
if (const types::IpzData* l_ipzData =
89-
std::get_if<types::IpzData>(&i_paramsToWriteData))
84+
auto [l_fruPath, l_inventoryObjPath, l_redundantFruPath] =
85+
jsonUtility::getAllPathsToUpdateKeyword(m_parsedJson,
86+
m_vpdFilePath);
87+
88+
// If inventory D-bus object path is present, update keyword's value on
89+
// DBus
90+
if (!l_inventoryObjPath.empty())
9091
{
91-
l_recordName = std::get<0>(*l_ipzData);
92-
l_interfaceName = constants::ipzVpdInf + l_recordName;
93-
l_propertyName = std::get<1>(*l_ipzData);
92+
types::Record l_recordName;
93+
std::string l_interfaceName;
94+
std::string l_propertyName;
95+
types::DbusVariantType l_keywordValue;
9496

95-
try
97+
if (const types::IpzData* l_ipzData =
98+
std::get_if<types::IpzData>(&i_paramsToWriteData))
9699
{
97-
// Read keyword's value from hardware to write the same on
98-
// D-bus.
99-
std::shared_ptr<ParserInterface> l_vpdParserInstance =
100-
getVpdParserInstance();
101-
102-
logging::logMessage("Performing VPD read on " + m_vpdFilePath);
103-
104-
l_keywordValue = l_vpdParserInstance->readKeywordFromHardware(
105-
types::ReadVpdParams(
106-
std::make_tuple(l_recordName, l_propertyName)));
100+
l_recordName = std::get<0>(*l_ipzData);
101+
l_interfaceName = constants::ipzVpdInf + l_recordName;
102+
l_propertyName = std::get<1>(*l_ipzData);
103+
104+
try
105+
{
106+
// Read keyword's value from hardware to write the same on
107+
// D-bus.
108+
std::shared_ptr<ParserInterface> l_vpdParserInstance =
109+
getVpdParserInstance();
110+
111+
logging::logMessage("Performing VPD read on " +
112+
m_vpdFilePath);
113+
114+
l_keywordValue =
115+
l_vpdParserInstance->readKeywordFromHardware(
116+
types::ReadVpdParams(
117+
std::make_tuple(l_recordName, l_propertyName)));
118+
}
119+
catch (const std::exception& l_exception)
120+
{
121+
// Unable to read keyword's value from hardware.
122+
std::string l_errMsg(
123+
"Error while reading keyword's value from hadware path " +
124+
m_vpdFilePath +
125+
", error: " + std::string(l_exception.what()));
126+
127+
// TODO: Log PEL
128+
129+
throw std::runtime_error(l_errMsg);
130+
}
107131
}
108-
catch (const std::exception& l_exception)
132+
else
109133
{
110-
// Unable to read keyword's value from hardware.
111-
logging::logMessage(
112-
"Error while reading keyword's value from hadware path " +
113-
m_vpdFilePath +
114-
", error: " + std::string(l_exception.what()));
115-
// TODO: Log PEL
116-
return -1;
134+
// Input parameter type provided isn't compatible to perform
135+
// update.
136+
std::string l_errMsg(
137+
"Input parameter type isn't compatible to update keyword's value on DBus for object path: " +
138+
l_inventoryObjPath);
139+
throw std::runtime_error(l_errMsg);
117140
}
118-
}
119-
else
120-
{
121-
// Input parameter type provided isn't compatible to perform update.
122-
logging::logMessage(
123-
"Input parameter type isn't compatible to update keyword's value on DBus for object path: " +
124-
l_inventoryObjPath);
125-
return -1;
126-
}
127141

128-
// Get D-bus name for the given keyword
129-
l_propertyName =
130-
vpdSpecificUtility::getDbusPropNameForGivenKw(l_propertyName);
142+
// Get D-bus name for the given keyword
143+
l_propertyName =
144+
vpdSpecificUtility::getDbusPropNameForGivenKw(l_propertyName);
131145

132-
// Create D-bus object map
133-
types::ObjectMap l_dbusObjMap = {std::make_pair(
134-
l_inventoryObjPath,
135-
types::InterfaceMap{std::make_pair(
136-
l_interfaceName, types::PropertyMap{std::make_pair(
137-
l_propertyName, l_keywordValue)})})};
146+
// Create D-bus object map
147+
types::ObjectMap l_dbusObjMap = {std::make_pair(
148+
l_inventoryObjPath,
149+
types::InterfaceMap{std::make_pair(
150+
l_interfaceName, types::PropertyMap{std::make_pair(
151+
l_propertyName, l_keywordValue)})})};
138152

139-
// Call PIM's Notify method to perform update
140-
if (!dbusUtility::callPIM(std::move(l_dbusObjMap)))
153+
// Call PIM's Notify method to perform update
154+
if (!dbusUtility::callPIM(std::move(l_dbusObjMap)))
155+
{
156+
// Call to PIM's Notify method failed.
157+
std::string l_errMsg("Notify PIM is failed for object path: " +
158+
l_inventoryObjPath);
159+
throw std::runtime_error(l_errMsg);
160+
}
161+
}
162+
163+
// Update keyword's value on redundant hardware if present
164+
if (!l_redundantFruPath.empty())
141165
{
142-
// Call to PIM's Notify method failed.
143-
logging::logMessage("Notify PIM is failed for object path: " +
144-
l_inventoryObjPath);
145-
return -1;
166+
if (updateVpdKeywordOnRedundantPath(l_redundantFruPath,
167+
i_paramsToWriteData) < 0)
168+
{
169+
std::string l_errMsg(
170+
"Error while updating keyword's value on redundant path " +
171+
l_redundantFruPath);
172+
throw std::runtime_error(l_errMsg);
173+
}
146174
}
147-
}
148175

149-
// Update keyword's value on redundant hardware if present
150-
if (!l_redundantFruPath.empty())
176+
// TODO: Check if revert is required when any of the writes fails.
177+
// TODO: Handle error logging
178+
}
179+
catch (const std::exception& l_ex)
151180
{
152-
if (updateVpdKeywordOnRedundantPath(l_redundantFruPath,
153-
i_paramsToWriteData) < 0)
181+
std::string l_keywordIdentifier{};
182+
if (const types::IpzData* l_ipzData =
183+
std::get_if<types::IpzData>(&i_paramsToWriteData))
154184
{
155-
logging::logMessage(
156-
"Error while updating keyword's value on redundant path " +
157-
l_redundantFruPath);
158-
return -1;
185+
l_keywordIdentifier = std::get<0>(*l_ipzData) + ":" +
186+
std::get<1>(*l_ipzData);
159187
}
160-
}
188+
else if (const types::KwData* l_kwData =
189+
std::get_if<types::KwData>(&i_paramsToWriteData))
190+
{
191+
l_keywordIdentifier = std::get<0>(*l_kwData);
192+
}
193+
logging::logMessage(
194+
"Update VPD Keyword failed for : " + l_keywordIdentifier +
195+
" failed due to error: " + l_ex.what());
161196

162-
// TODO: Check if revert is required when any of the writes fails.
163-
// TODO: Handle error logging
197+
return constants::FAILURE;
198+
}
164199

165200
// All updates are successful.
166201
return l_bytesUpdatedOnHardware;

0 commit comments

Comments
 (0)