PLDM: Implementing Phosphor-Logging/LG2 logging
This commit adds changes in PLDM for implementing
structured LG2 logging, thereby moving away from
std::cout/cerr practice of logging which are
output streams and not logging mechanism.
PLDM now can make use of lg2 features like accurate
CODE LINE Number and CODE_FUNCTION Name and better
detailing in json object values which can be used in
log tracking.
More detailed logging change:
https://gist.github.com/riyadixitagra/c251685c1ba84248181891f7bc282395
Tested:
Ran a power off, on, cycle, and reset-reload.
Change-Id: I0485035f15f278c3fd172f0581b053c1c37f3a5b
Signed-off-by: Riya Dixit <riyadixitagra@gmail.com>
diff --git a/host-bmc/dbus_to_event_handler.cpp b/host-bmc/dbus_to_event_handler.cpp
index cf3064e..d1125f1 100644
--- a/host-bmc/dbus_to_event_handler.cpp
+++ b/host-bmc/dbus_to_event_handler.cpp
@@ -4,6 +4,10 @@
#include <libpldm/pldm.h>
+#include <phosphor-logging/lg2.hpp>
+
+PHOSPHOR_LOG2_USING;
+
namespace pldm
{
using namespace pldm::dbus_api;
@@ -40,8 +44,8 @@
if (rc != PLDM_SUCCESS)
{
requester.markFree(mctp_eid, instanceId);
- std::cerr << "Failed to encode_platform_event_message_req, rc = " << rc
- << std::endl;
+ error("Failed to encode_platform_event_message_req, rc = {RC}", "RC",
+ rc);
return;
}
@@ -50,8 +54,7 @@
size_t respMsgLen) {
if (response == nullptr || !respMsgLen)
{
- std::cerr
- << "Failed to receive response for platform event message \n";
+ error("Failed to receive response for platform event message");
return;
}
uint8_t completionCode{};
@@ -60,10 +63,9 @@
&completionCode, &status);
if (rc || completionCode)
{
- std::cerr << "Failed to decode_platform_event_message_resp: "
- << "rc=" << rc
- << ", cc=" << static_cast<unsigned>(completionCode)
- << std::endl;
+ error(
+ "Failed to decode_platform_event_message_resp: rc = {RC}, cc = {CC}",
+ "RC", rc, "CC", static_cast<unsigned>(completionCode));
}
};
@@ -72,7 +74,7 @@
std::move(requestMsg), std::move(platformEventMessageResponseHandler));
if (rc)
{
- std::cerr << "Failed to send the platform event message \n";
+ error("Failed to send the platform event message");
}
}
diff --git a/host-bmc/dbus_to_host_effecters.cpp b/host-bmc/dbus_to_host_effecters.cpp
index 89a8a95..32419a2 100644
--- a/host-bmc/dbus_to_host_effecters.cpp
+++ b/host-bmc/dbus_to_host_effecters.cpp
@@ -4,12 +4,15 @@
#include <libpldm/platform.h>
#include <libpldm/pldm.h>
+#include <phosphor-logging/lg2.hpp>
#include <xyz/openbmc_project/Common/error.hpp>
#include <xyz/openbmc_project/State/OperatingSystem/Status/server.hpp>
#include <fstream>
#include <iostream>
+PHOSPHOR_LOG2_USING;
+
using namespace pldm::utils;
namespace pldm
@@ -38,15 +41,16 @@
fs::path jsonDir(jsonPath);
if (!fs::exists(jsonDir) || fs::is_empty(jsonDir))
{
- std::cerr << "Host Effecter json path does not exist, DIR=" << jsonPath
- << "\n";
+ error("Host Effecter json path does not exist, DIR = {JSON_PATH}",
+ "JSON_PATH", jsonPath.c_str());
return;
}
fs::path jsonFilePath = jsonDir / hostEffecterJson;
if (!fs::exists(jsonFilePath))
{
- std::cerr << "json does not exist, PATH=" << jsonFilePath << "\n";
+ error("json does not exist, PATH = {JSON_PATH}", "JSON_PATH",
+ jsonFilePath.c_str());
throw InternalFailure();
}
@@ -54,7 +58,8 @@
auto data = Json::parse(jsonFile, nullptr, false);
if (data.is_discarded())
{
- std::cerr << "Parsing json file failed, FILE=" << jsonFilePath << "\n";
+ error("Parsing json file failed, FILE = {JSON_PATH}", "JSON_PATH",
+ jsonFilePath.c_str());
throw InternalFailure();
}
const Json empty{};
@@ -96,11 +101,10 @@
auto states = state.value("state_values", emptyStates);
if (dbusInfo.propertyValues.size() != states.size())
{
- std::cerr << "Number of states do not match with"
- << " number of D-Bus property values in the json. "
- << "Object path " << dbusInfo.dbusMap.objectPath
- << " and property " << dbusInfo.dbusMap.propertyName
- << " will not be monitored \n";
+ error(
+ "Number of states do not match with number of D-Bus property values in the json. Object path {OBJ_PATH} and property {PROP_NAME} will not be monitored",
+ "OBJ_PATH", dbusInfo.dbusMap.objectPath.c_str(),
+ "PROP_NAME", dbusInfo.dbusMap.propertyName);
continue;
}
for (const auto& s : states)
@@ -147,7 +151,7 @@
localOrRemote);
if (effecterId == PLDM_INVALID_EFFECTER_ID)
{
- std::cerr << "Effecter id not found in pdr repo \n";
+ error("Effecter id not found in pdr repo");
return;
}
}
@@ -169,16 +173,16 @@
(currHostState != "xyz.openbmc_project.State.Boot.Progress."
"ProgressStages.SystemSetup"))
{
- std::cout << "Host is not up. Current host state: "
- << currHostState.c_str() << "\n";
+ info("Host is not up. Current host state: {CUR_HOST_STATE}",
+ "CUR_HOST_STATE", currHostState.c_str());
return;
}
}
catch (const sdbusplus::exception_t& e)
{
- std::cerr << "Error in getting current host state. Will still "
- "continue to set the host effecter - "
- << e.what() << std::endl;
+ error(
+ "Error in getting current host state. Will still continue to set the host effecter - {ERR_EXCEP}",
+ "ERR_EXCEP", e.what());
}
uint8_t newState{};
try
@@ -188,8 +192,7 @@
}
catch (const std::out_of_range& e)
{
- std::cerr << "new state not found in json"
- << "\n";
+ error("New state not found in json");
return;
}
@@ -213,13 +216,12 @@
}
catch (const std::runtime_error& e)
{
- std::cerr << "Could not set host state effecter \n";
+ error("Could not set host state effecter");
return;
}
if (rc != PLDM_SUCCESS)
{
- std::cerr << "Could not set the host state effecter, rc= " << rc
- << " \n";
+ error("Could not set the host state effecter, rc= {RC}", "RC", rc);
}
}
@@ -265,46 +267,46 @@
if (rc != PLDM_SUCCESS)
{
- std::cerr << "Message encode failure. PLDM error code = " << std::hex
- << std::showbase << rc << "\n";
+ error("Message encode failure. PLDM error code = {RC}", "RC", lg2::hex,
+ rc);
requester->markFree(mctpEid, instanceId);
return rc;
}
- auto setStateEffecterStatesRespHandler =
- [](mctp_eid_t /*eid*/, const pldm_msg* response, size_t respMsgLen) {
- if (response == nullptr || !respMsgLen)
- {
- std::cerr << "Failed to receive response for "
- << "setStateEffecterStates command \n";
- return;
- }
- uint8_t completionCode{};
- auto rc = decode_set_state_effecter_states_resp(
- response, respMsgLen, &completionCode);
- if (rc)
- {
- std::cerr << "Failed to decode setStateEffecterStates response,"
- << " rc " << rc << "\n";
- pldm::utils::reportError(
- "xyz.openbmc_project.bmc.pldm.SetHostEffecterFailed");
- }
- if (completionCode)
- {
- std::cerr << "Failed to set a Host effecter "
- << ", cc=" << static_cast<unsigned>(completionCode)
- << "\n";
- pldm::utils::reportError(
- "xyz.openbmc_project.bmc.pldm.SetHostEffecterFailed");
- }
- };
+ auto setStateEffecterStatesRespHandler = [](mctp_eid_t /*eid*/,
+ const pldm_msg* response,
+ size_t respMsgLen) {
+ if (response == nullptr || !respMsgLen)
+ {
+ error(
+ "Failed to receive response for setStateEffecterStates command");
+ return;
+ }
+ uint8_t completionCode{};
+ auto rc = decode_set_state_effecter_states_resp(response, respMsgLen,
+ &completionCode);
+ if (rc)
+ {
+ error("Failed to decode setStateEffecterStates response, rc {RC}",
+ "RC", rc);
+ pldm::utils::reportError(
+ "xyz.openbmc_project.bmc.pldm.SetHostEffecterFailed");
+ }
+ if (completionCode)
+ {
+ error("Failed to set a Host effecter, cc = {CC}", "CC",
+ static_cast<unsigned>(completionCode));
+ pldm::utils::reportError(
+ "xyz.openbmc_project.bmc.pldm.SetHostEffecterFailed");
+ }
+ };
rc = handler->registerRequest(
mctpEid, instanceId, PLDM_PLATFORM, PLDM_SET_STATE_EFFECTER_STATES,
std::move(requestMsg), std::move(setStateEffecterStatesRespHandler));
if (rc)
{
- std::cerr << "Failed to send request to set an effecter on Host \n";
+ error("Failed to send request to set an effecter on Host");
}
return rc;
}
diff --git a/host-bmc/dbus_to_host_effecters.hpp b/host-bmc/dbus_to_host_effecters.hpp
index f8e165e..11721bb 100644
--- a/host-bmc/dbus_to_host_effecters.hpp
+++ b/host-bmc/dbus_to_host_effecters.hpp
@@ -5,10 +5,14 @@
#include "pldmd/dbus_impl_requester.hpp"
#include "requester/handler.hpp"
+#include <phosphor-logging/lg2.hpp>
+
#include <string>
#include <utility>
#include <vector>
+PHOSPHOR_LOG2_USING;
+
namespace pldm
{
@@ -91,8 +95,9 @@
}
catch (const std::exception& e)
{
- std::cerr << "The json file does not exist or malformed, ERROR="
- << e.what() << "\n";
+ error(
+ "The json file does not exist or malformed, ERROR={ERR_EXCEP}",
+ "ERR_EXCEP", e.what());
}
}
diff --git a/host-bmc/host_pdr_handler.cpp b/host-bmc/host_pdr_handler.cpp
index 08b78ad..cedebb0 100644
--- a/host-bmc/host_pdr_handler.cpp
+++ b/host-bmc/host_pdr_handler.cpp
@@ -6,6 +6,7 @@
#include <libpldm/pldm.h>
#include <nlohmann/json.hpp>
+#include <phosphor-logging/lg2.hpp>
#include <sdeventplus/clock.hpp>
#include <sdeventplus/exception.hpp>
#include <sdeventplus/source/io.hpp>
@@ -13,6 +14,8 @@
#include <fstream>
+PHOSPHOR_LOG2_USING;
+
namespace pldm
{
using namespace pldm::dbus_api;
@@ -65,7 +68,7 @@
auto data = Json::parse(jsonFile, nullptr, false);
if (data.is_discarded())
{
- std::cerr << "Parsing Host FRU json file failed" << std::endl;
+ error("Parsing Host FRU json file failed");
}
else
{
@@ -83,8 +86,8 @@
}
catch (const std::exception& e)
{
- std::cerr << "Parsing Host FRU json file failed, exception = "
- << e.what() << std::endl;
+ error("Parsing Host FRU json file failed, exception = {ERR_EXCEP}",
+ "ERR_EXCEP", e.what());
}
}
@@ -177,7 +180,7 @@
if (rc != PLDM_SUCCESS)
{
requester.markFree(mctp_eid, instanceId);
- std::cerr << "Failed to encode_get_pdr_req, rc = " << rc << std::endl;
+ error("Failed to encode_get_pdr_req, rc = {RC}", "RC", rc);
return;
}
@@ -187,7 +190,7 @@
std::move(std::bind_front(&HostPDRHandler::processHostPDRs, this)));
if (rc)
{
- std::cerr << "Failed to send the GetPDR request to Host \n";
+ error("Failed to send the GetPDR request to Host");
}
}
@@ -197,8 +200,7 @@
auto rc = stateSensorHandler.eventAction(entry, state);
if (rc != PLDM_SUCCESS)
{
- std::cerr << "Failed to fetch and update D-bus property, rc = " << rc
- << std::endl;
+ error("Failed to fetch and update D-bus property, rc = {RC}", "RC", rc);
return rc;
}
return PLDM_SUCCESS;
@@ -249,8 +251,7 @@
pldm_find_entity_ref_in_tree(entityTree, entities[0], &node);
if (node == nullptr)
{
- std::cerr
- << "\ncould not find referrence of the entity in the tree \n";
+ error("could not find referrence of the entity in the tree");
}
else
{
@@ -308,9 +309,8 @@
&firstEntry, eventData, &actualSize, maxSize);
if (rc != PLDM_SUCCESS)
{
- std::cerr
- << "Failed to encode_pldm_pdr_repository_chg_event_data, rc = "
- << rc << std::endl;
+ error("Failed to encode_pldm_pdr_repository_chg_event_data, rc = {RC}",
+ "RC", rc);
return;
}
auto instanceId = requester.getInstanceId(mctp_eid);
@@ -325,8 +325,8 @@
if (rc != PLDM_SUCCESS)
{
requester.markFree(mctp_eid, instanceId);
- std::cerr << "Failed to encode_platform_event_message_req, rc = " << rc
- << std::endl;
+ error("Failed to encode_platform_event_message_req, rc = {RC}", "RC",
+ rc);
return;
}
@@ -335,9 +335,8 @@
size_t respMsgLen) {
if (response == nullptr || !respMsgLen)
{
- std::cerr << "Failed to receive response for the PDR repository "
- "changed event"
- << "\n";
+ error(
+ "Failed to receive response for the PDR repository changed event");
return;
}
@@ -348,10 +347,9 @@
&completionCode, &status);
if (rc || completionCode)
{
- std::cerr << "Failed to decode_platform_event_message_resp: "
- << "rc=" << rc
- << ", cc=" << static_cast<unsigned>(completionCode)
- << std::endl;
+ error(
+ "Failed to decode_platform_event_message_resp: {RC}, cc = {CC}",
+ "RC", rc, "CC", static_cast<unsigned>(completionCode));
}
};
@@ -360,8 +358,7 @@
std::move(requestMsg), std::move(platformEventMessageResponseHandler));
if (rc)
{
- std::cerr << "Failed to send the PDR repository changed event request"
- << "\n";
+ error("Failed to send the PDR repository changed event request");
}
}
@@ -408,8 +405,7 @@
uint8_t transferCRC{};
if (response == nullptr || !respMsgLen)
{
- std::cerr << "Failed to receive response for the GetPDR"
- " command \n";
+ error("Failed to receive response for the GetPDR command");
return;
}
@@ -422,7 +418,7 @@
memcpy(responsePDRMsg.data(), response, respMsgLen + sizeof(pldm_msg_hdr));
if (rc != PLDM_SUCCESS)
{
- std::cerr << "Failed to decode_get_pdr_resp, rc = " << rc << std::endl;
+ error("Failed to decode_get_pdr_resp, rc = {RC}", "RC", rc);
return;
}
else
@@ -434,10 +430,8 @@
respCount, &transferCRC);
if (rc != PLDM_SUCCESS || completionCode != PLDM_SUCCESS)
{
- std::cerr << "Failed to decode_get_pdr_resp: "
- << "rc=" << rc
- << ", cc=" << static_cast<unsigned>(completionCode)
- << std::endl;
+ error("Failed to decode_get_pdr_resp: rc = {RC}, cc = {CC}", "RC",
+ rc, "CC", static_cast<unsigned>(completionCode));
return;
}
else
@@ -615,8 +609,8 @@
PLDM_BASE, request);
if (rc != PLDM_SUCCESS)
{
- std::cerr << "GetPLDMVersion encode failure. PLDM error code = "
- << std::hex << std::showbase << rc << "\n";
+ error("GetPLDMVersion encode failure. PLDM error code = {RC}", "RC",
+ lg2::hex, rc);
requester.markFree(mctp_eid, instanceId);
return;
}
@@ -626,13 +620,12 @@
size_t respMsgLen) {
if (response == nullptr || !respMsgLen)
{
- std::cerr << "Failed to receive response for "
- << "getPLDMVersion command, Host seems to be off \n";
+ error(
+ "Failed to receive response for getPLDMVersion command, Host seems to be off");
return;
}
- std::cout << "Getting the response. PLDM RC = " << std::hex
- << std::showbase
- << static_cast<uint16_t>(response->payload[0]) << "\n";
+ info("Getting the response. PLDM RC = {RC}", "RC", lg2::hex,
+ static_cast<uint16_t>(response->payload[0]));
this->responseReceived = true;
getHostPDR();
};
@@ -641,7 +634,7 @@
std::move(getPLDMVersionHandler));
if (rc)
{
- std::cerr << "Failed to discover Host state. Assuming Host as off \n";
+ error("Failed to discover Host state. Assuming Host as off");
}
}
@@ -659,7 +652,7 @@
if (!pdr)
{
- std::cerr << "Failed to get State sensor PDR" << std::endl;
+ error("Failed to get State sensor PDR");
pldm::utils::reportError(
"xyz.openbmc_project.bmc.pldm.InternalFailure");
return;
@@ -691,9 +684,9 @@
if (rc != PLDM_SUCCESS)
{
requester.markFree(mctp_eid, instanceId);
- std::cerr << "Failed to "
- "encode_get_state_sensor_readings_req, rc = "
- << rc << std::endl;
+ error(
+ "Failed to encode_get_state_sensor_readings_req, rc = {RC}",
+ "RC", rc);
pldm::utils::reportError(
"xyz.openbmc_project.bmc.pldm.InternalFailure");
return;
@@ -706,8 +699,8 @@
size_t respMsgLen) {
if (response == nullptr || !respMsgLen)
{
- std::cerr << "Failed to receive response for "
- "getStateSensorReading command \n";
+ error(
+ "Failed to receive response for getStateSensorReading command");
return;
}
std::array<get_sensor_state_field, 8> stateField{};
@@ -720,12 +713,10 @@
if (rc != PLDM_SUCCESS || completionCode != PLDM_SUCCESS)
{
- std::cerr
- << "Failed to "
- "decode_get_state_sensor_readings_resp, rc = "
- << rc
- << " cc=" << static_cast<unsigned>(completionCode)
- << std::endl;
+ error(
+ "Failed to decode_get_state_sensor_readings_resp, rc = {RC} cc = {CC}",
+ "RC", rc, "CC",
+ static_cast<unsigned>(completionCode));
pldm::utils::reportError(
"xyz.openbmc_project.bmc.pldm.InternalFailure");
}
@@ -765,16 +756,13 @@
}
catch (const std::out_of_range& e)
{
- std::cerr << "No mapping for the events"
- << std::endl;
+ error("No mapping for the events");
}
}
if (sensorOffset > compositeSensorStates.size())
{
- std::cerr
- << " Error Invalid data, Invalid sensor offset"
- << std::endl;
+ error("Error Invalid data, Invalid sensor offset");
return;
}
@@ -783,9 +771,7 @@
if (possibleStates.find(eventState) ==
possibleStates.end())
{
- std::cerr
- << " Error invalid_data, Invalid event state"
- << std::endl;
+ error("Error invalid_data, Invalid event state");
return;
}
const auto& [containerId, entityType, entityInstance] =
@@ -804,9 +790,8 @@
if (rc != PLDM_SUCCESS)
{
- std::cerr << " Failed to send request to get State sensor "
- "reading on Host "
- << std::endl;
+ error(
+ "Failed to send request to get State sensor reading on Host");
}
}
}
diff --git a/host-bmc/test/meson.build b/host-bmc/test/meson.build
index 0bb3b8b..e0973ec 100644
--- a/host-bmc/test/meson.build
+++ b/host-bmc/test/meson.build
@@ -22,6 +22,7 @@
libpldmutils,
nlohmann_json,
phosphor_dbus_interfaces,
+ phosphor_logging_dep,
sdbusplus,
sdeventplus]),
workdir: meson.current_source_dir())