host-bmc: Improving Logs (lg2)
This commit corrects the severity level of logs and also formats the
message string, fixing the ill-defined message string of the logs as
mentioned in the anti-pattern document [1]. Additionally, based on the
requirement this commit adds more debug information to logs.
[1]: https://github.com/openbmc/docs/blob/master/anti-patterns.md#ill-defined-data-structuring-in-lg2-message-strings
Change-Id: I75a2b54ac1d363cfb675f29e811ab341046fbccd
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 891a769..85740cf 100644
--- a/host-bmc/dbus_to_event_handler.cpp
+++ b/host-bmc/dbus_to_event_handler.cpp
@@ -41,8 +41,9 @@
if (rc != PLDM_SUCCESS)
{
instanceIdDb.free(mctp_eid, instanceId);
- error("Failed to encode_platform_event_message_req, rc = {RC}", "RC",
- rc);
+ error(
+ "Failed to encode platform event message request, response code '{RC}'",
+ "RC", rc);
return;
}
@@ -60,8 +61,8 @@
if (rc || completionCode)
{
error(
- "Failed to decode_platform_event_message_resp: rc = {RC}, cc = {CC}",
- "RC", rc, "CC", static_cast<unsigned>(completionCode));
+ "Failed to decode response of platform_event message, response code '{RC}' and completion code '{CC}'",
+ "RC", rc, "CC", completionCode);
}
};
@@ -70,7 +71,8 @@
std::move(requestMsg), std::move(platformEventMessageResponseHandler));
if (rc)
{
- error("Failed to send the platform event message");
+ error("Failed to send the platform event message, response code '{RC}'",
+ "RC", rc);
}
}
diff --git a/host-bmc/dbus_to_host_effecters.cpp b/host-bmc/dbus_to_host_effecters.cpp
index d5aeb58..0774685 100644
--- a/host-bmc/dbus_to_host_effecters.cpp
+++ b/host-bmc/dbus_to_host_effecters.cpp
@@ -40,16 +40,15 @@
fs::path jsonDir(jsonPath);
if (!fs::exists(jsonDir) || fs::is_empty(jsonDir))
{
- error("Host Effecter json path does not exist, DIR = {JSON_PATH}",
- "JSON_PATH", jsonPath.c_str());
+ error("Effecter json file for remote terminus '{PATH}' does not exist.",
+ "PATH", jsonPath);
return;
}
fs::path jsonFilePath = jsonDir / hostEffecterJson;
if (!fs::exists(jsonFilePath))
{
- error("json does not exist, PATH = {JSON_PATH}", "JSON_PATH",
- jsonFilePath.c_str());
+ error("Json at path '{PATH}' does not exist.", "PATH", jsonFilePath);
throw InternalFailure();
}
@@ -57,8 +56,7 @@
auto data = Json::parse(jsonFile, nullptr, false);
if (data.is_discarded())
{
- error("Parsing json file failed, FILE = {JSON_PATH}", "JSON_PATH",
- jsonFilePath.c_str());
+ error("Failed to parse json file {PATH}", "PATH", jsonFilePath);
throw InternalFailure();
}
const Json empty{};
@@ -101,9 +99,9 @@
if (dbusInfo.propertyValues.size() != states.size())
{
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);
+ "Number of states do not match with number of D-Bus property values in the json. Object path at '{PATH}' and property '{PROPERTY}' will not be monitored",
+ "PATH", dbusInfo.dbusMap.objectPath, "PROPERTY",
+ dbusInfo.dbusMap.propertyName);
continue;
}
for (const auto& s : states)
@@ -153,7 +151,13 @@
localOrRemote);
if (effecterId == PLDM_INVALID_EFFECTER_ID)
{
- error("Effecter id not found in pdr repo");
+ error(
+ "Effecter ID '{EFFECTERID}' of entity type '{TYPE}', entityInstance '{INSTANCE}' and containerID '{CONTAINER_ID}' not found in pdr repo",
+ "EFFECTERID", effecterId, "TYPE",
+ hostEffecterInfo[effecterInfoIndex].entityType, "INSTANCE",
+ hostEffecterInfo[effecterInfoIndex].entityInstance,
+ "CONTAINER_ID",
+ hostEffecterInfo[effecterInfoIndex].containerId);
return;
}
}
@@ -173,16 +177,17 @@
currHostState != Stages::OSRunning &&
currHostState != Stages::SystemSetup)
{
- info("Host is not up. Current host state: {CUR_HOST_STATE}",
- "CUR_HOST_STATE", currHostState);
+ info(
+ "Remote terminus is not up/active, current remote terminus state is: '{CURRENT_HOST_STATE}'",
+ "CURRENT_HOST_STATE", currHostState);
return;
}
}
catch (const sdbusplus::exception_t& e)
{
error(
- "Error in getting current host state. Will still continue to set the host effecter - {ERR_EXCEP}",
- "ERR_EXCEP", e.what());
+ "Error in getting current remote terminus state. Will still continue to set the remote terminus effecter, error - {ERROR}",
+ "ERROR", e);
}
uint8_t newState{};
try
@@ -192,7 +197,8 @@
}
catch (const std::out_of_range& e)
{
- error("New state not found in json: {ERROR}", "ERROR", e);
+ error("Failed to find new state '{NEW_STATE}' in json, error - {ERROR}",
+ "ERROR", e, "NEW_STATE", newState);
return;
}
@@ -216,12 +222,16 @@
}
catch (const std::runtime_error& e)
{
- error("Could not set host state effecter");
+ error(
+ "Failed to set remote terminus state effecter for effecter ID '{EFFECTERID}', error - {ERROR}",
+ "ERROR", e, "EFFECTERID", effecterId);
return;
}
if (rc != PLDM_SUCCESS)
{
- error("Could not set the host state effecter, rc= {RC}", "RC", rc);
+ error(
+ "Failed to set the remote terminus state effecter for effecter ID '{EFFECTERID}', response code '{RC}'",
+ "EFFECTERID", effecterId, "RC", rc);
}
}
@@ -267,8 +277,10 @@
if (rc != PLDM_SUCCESS)
{
- error("Message encode failure. PLDM error code = {RC}", "RC", lg2::hex,
- rc);
+ error(
+ "Failed to encode set state effecter states message for effecter ID '{EFFECTERID}' and instanceID '{INSTANCE}' with response code '{RC}'",
+ "EFFECTERID", effecterId, "INSTANCE", instanceId, "RC", lg2::hex,
+ rc);
instanceIdDb->free(mctpEid, instanceId);
return rc;
}
@@ -278,7 +290,7 @@
if (response == nullptr || !respMsgLen)
{
error(
- "Failed to receive response for setStateEffecterStates command");
+ "Failed to receive response for setting state effecter states.");
return;
}
uint8_t completionCode{};
@@ -286,15 +298,17 @@
&completionCode);
if (rc)
{
- error("Failed to decode setStateEffecterStates response, rc {RC}",
- "RC", rc);
+ error(
+ "Failed to decode response of set state effecter states, response code '{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));
+ error(
+ "Failed to set a remote terminus effecter, completion code '{CC}'",
+ "CC", completionCode);
pldm::utils::reportError(
"xyz.openbmc_project.bmc.pldm.SetHostEffecterFailed");
}
@@ -305,7 +319,9 @@
std::move(requestMsg), std::move(setStateEffecterStatesRespHandler));
if (rc)
{
- error("Failed to send request to set an effecter on Host");
+ error(
+ "Failed to send request to set an effecter on remote terminus for effecter ID '{EFFECTERID}', response code '{RC}'",
+ "EFFECTERID", effecterId, "RC", rc);
}
return rc;
}
diff --git a/host-bmc/dbus_to_host_effecters.hpp b/host-bmc/dbus_to_host_effecters.hpp
index 59cbd80..c06e8b0 100644
--- a/host-bmc/dbus_to_host_effecters.hpp
+++ b/host-bmc/dbus_to_host_effecters.hpp
@@ -96,8 +96,8 @@
catch (const std::exception& e)
{
error(
- "The json file does not exist or malformed, ERROR={ERR_EXCEP}",
- "ERR_EXCEP", e.what());
+ "The json file '{PATH}' does not exist or malformed, error - '{ERROR}'",
+ "PATH", jsonPath, "ERROR", e);
}
}
diff --git a/host-bmc/host_pdr_handler.cpp b/host-bmc/host_pdr_handler.cpp
index ce41d20..3f3b964 100644
--- a/host-bmc/host_pdr_handler.cpp
+++ b/host-bmc/host_pdr_handler.cpp
@@ -188,7 +188,8 @@
if (rc != PLDM_SUCCESS)
{
instanceIdDb.free(mctp_eid, instanceId);
- error("Failed to encode_get_pdr_req, rc = {RC}", "RC", rc);
+ error("Failed to encode get pdr request, response code '{RC}'", "RC",
+ rc);
return;
}
@@ -198,7 +199,9 @@
std::move(std::bind_front(&HostPDRHandler::processHostPDRs, this)));
if (rc)
{
- error("Failed to send the GetPDR request to Host");
+ error(
+ "Failed to send the getPDR request to remote terminus, response code '{RC}'",
+ "RC", rc);
}
}
@@ -208,7 +211,8 @@
auto rc = stateSensorHandler.eventAction(entry, state);
if (rc != PLDM_SUCCESS)
{
- error("Failed to fetch and update D-bus property, rc = {RC}", "RC", rc);
+ error("Failed to fetch and update D-bus property, response code '{RC}'",
+ "RC", rc);
return rc;
}
return PLDM_SUCCESS;
@@ -288,7 +292,7 @@
pldm_find_entity_ref_in_tree(entityTree, entities[0], &node);
if (node == nullptr)
{
- error("could not find referrence of the entity in the tree");
+ error("Failed to find referrence of the entity in the tree");
}
else
{
@@ -314,8 +318,8 @@
if (rc)
{
error(
- "Failed to add entity association PDR from node: {LIBPLDM_ERROR}",
- "LIBPLDM_ERROR", rc);
+ "Failed to add entity association PDR from node, response code '{RC}'",
+ "RC", rc);
}
}
}
@@ -369,8 +373,9 @@
&firstEntry, eventData, &actualSize, maxSize);
if (rc != PLDM_SUCCESS)
{
- error("Failed to encode_pldm_pdr_repository_chg_event_data, rc = {RC}",
- "RC", rc);
+ error(
+ "Failed to encode pldm pdr repository change event data, response code '{RC}'",
+ "RC", rc);
return;
}
auto instanceId = instanceIdDb.next(mctp_eid);
@@ -385,8 +390,9 @@
if (rc != PLDM_SUCCESS)
{
instanceIdDb.free(mctp_eid, instanceId);
- error("Failed to encode_platform_event_message_req, rc = {RC}", "RC",
- rc);
+ error(
+ "Failed to encode platform event message request, response code '{RC}'",
+ "RC", rc);
return;
}
@@ -407,8 +413,8 @@
if (rc || completionCode)
{
error(
- "Failed to decode_platform_event_message_resp: {RC}, cc = {CC}",
- "RC", rc, "CC", static_cast<unsigned>(completionCode));
+ "Failed to decode platform event message response, response code '{RC}' and completion code '{CC}'",
+ "RC", rc, "CC", completionCode);
}
};
@@ -417,7 +423,9 @@
std::move(requestMsg), std::move(platformEventMessageResponseHandler));
if (rc)
{
- error("Failed to send the PDR repository changed event request");
+ error(
+ "Failed to send the PDR repository changed event request, response code '{RC}'",
+ "RC", rc);
}
}
@@ -478,7 +486,9 @@
memcpy(responsePDRMsg.data(), response, respMsgLen + sizeof(pldm_msg_hdr));
if (rc != PLDM_SUCCESS)
{
- error("Failed to decode_get_pdr_resp, rc = {RC}", "RC", rc);
+ error(
+ "Failed to decode getPDR response for next record handle '{NEXT_RECORD_HANDLE}', response code '{RC}'",
+ "NEXT_RECORD_HANDLE", nextRecordHandle, "RC", rc);
return;
}
else
@@ -490,8 +500,11 @@
respCount, &transferCRC);
if (rc != PLDM_SUCCESS || completionCode != PLDM_SUCCESS)
{
- error("Failed to decode_get_pdr_resp: rc = {RC}, cc = {CC}", "RC",
- rc, "CC", static_cast<unsigned>(completionCode));
+ error(
+ "Failed to decode getPDR response for next record handle '{NEXT_RECORD_HANDLE}', next data transfer handle '{DATA_TRANSFER_HANDLE}' and transfer flag '{FLAG}', response code '{RC}' and completion code '{CC}'",
+ "NEXT_RECORD_HANDLE", nextRecordHandle, "DATA_TRANSFER_HANDLE",
+ nextDataTransferHandle, "FLAG", transferFlag, "RC", rc, "CC",
+ completionCode);
return;
}
else
@@ -694,7 +707,7 @@
PLDM_BASE, request);
if (rc != PLDM_SUCCESS)
{
- error("GetPLDMVersion encode failure. PLDM error code = {RC}", "RC",
+ error("Failed to encode GetPLDMVersion, response code {RC}", "RC",
lg2::hex, rc);
instanceIdDb.free(mctp_eid, instanceId);
return;
@@ -709,7 +722,7 @@
"Failed to receive response for getPLDMVersion command, Host seems to be off");
return;
}
- info("Getting the response. PLDM RC = {RC}", "RC", lg2::hex,
+ info("Getting the response code '{RC}'", "RC", lg2::hex,
static_cast<uint16_t>(response->payload[0]));
this->responseReceived = true;
getHostPDR();
@@ -719,7 +732,8 @@
std::move(getPLDMVersionHandler));
if (rc)
{
- error("Failed to discover Host state. Assuming Host as off");
+ error(
+ "Failed to discover remote terminus state. Assuming remote terminus as off");
}
}
@@ -737,7 +751,7 @@
if (!pdr)
{
- error("Failed to get State sensor PDR");
+ error("Failed to get state sensor PDR");
pldm::utils::reportError(
"xyz.openbmc_project.bmc.pldm.InternalFailure");
return;
@@ -770,8 +784,9 @@
{
instanceIdDb.free(mctp_eid, instanceId);
error(
- "Failed to encode_get_state_sensor_readings_req, rc = {RC}",
- "RC", rc);
+ "Failed to encode get state sensor readings request for sensorID '{SENSOR_ID}' and instanceID '{INSTANCE}', response code '{RC}'",
+ "SENSOR_ID", sensorId, "INSTANCE", instanceId, "RC",
+ rc);
pldm::utils::reportError(
"xyz.openbmc_project.bmc.pldm.InternalFailure");
return;
@@ -783,7 +798,8 @@
if (response == nullptr || !respMsgLen)
{
error(
- "Failed to receive response for getStateSensorReading command");
+ "Failed to receive response for get state sensor reading command for sensorID '{SENSOR_ID}' and instanceID '{INSTANCE}'",
+ "SENSOR_ID", sensorId, "INSTANCE", instanceId);
return;
}
std::array<get_sensor_state_field, 8> stateField{};
@@ -797,9 +813,9 @@
if (rc != PLDM_SUCCESS || completionCode != PLDM_SUCCESS)
{
error(
- "Failed to decode_get_state_sensor_readings_resp, rc = {RC} cc = {CC}",
- "RC", rc, "CC",
- static_cast<unsigned>(completionCode));
+ "Failed to decode get state sensor readings response for sensorID '{SENSOR_ID}' and instanceID '{INSTANCE}', response code'{RC}' and completion code '{CC}'",
+ "SENSOR_ID", sensorId, "INSTANCE", instanceId, "RC",
+ rc, "CC", completionCode);
pldm::utils::reportError(
"xyz.openbmc_project.bmc.pldm.InternalFailure");
}
@@ -849,7 +865,9 @@
if ((compositeSensorStates.size() > 1) &&
(sensorOffset > (compositeSensorStates.size() - 1)))
{
- error("Error Invalid data, Invalid sensor offset");
+ error(
+ "Error Invalid data, Invalid sensor offset '{SENSOR_OFFSET}'",
+ "SENSOR_OFFSET", sensorOffset);
return;
}
@@ -858,7 +876,9 @@
if (possibleStates.find(eventState) ==
possibleStates.end())
{
- error("Error invalid_data, Invalid event state");
+ error(
+ "Error invalid_data, Invalid event state '{STATE}'",
+ "STATE", eventState);
return;
}
const auto& [containerId, entityType,
@@ -880,7 +900,9 @@
if (rc != PLDM_SUCCESS)
{
error(
- "Failed to send request to get State sensor reading on Host");
+ "Failed to send request to get state sensor reading on remote terminus for sensorID '{SENSOR_ID}' and instanceID '{INSTANCE}', response code '{RC}'",
+ "SENSOR_ID", sensorId, "INSTANCE", instanceId, "RC",
+ rc);
}
}
}
@@ -901,8 +923,8 @@
if (rc != PLDM_SUCCESS)
{
instanceIdDb.free(mctp_eid, instanceId);
- lg2::error(
- "Failed to encode_get_fru_record_table_metadata_req, rc = {RC}",
+ error(
+ "Failed to encode get fru record table metadata request, response code '{RC}'",
"RC", lg2::hex, rc);
return;
}
@@ -912,8 +934,8 @@
size_t respMsgLen) {
if (response == nullptr || !respMsgLen)
{
- lg2::error(
- "Failed to receive response for the Get FRU Record Table Metadata");
+ error(
+ "Failed to receive response for the get fru record table metadata");
return;
}
@@ -931,8 +953,8 @@
if (rc != PLDM_SUCCESS || cc != PLDM_SUCCESS)
{
- lg2::error(
- "Faile to decode get fru record table metadata resp, Message Error: {RC}, cc: {CC}",
+ error(
+ "Failed to decode get fru record table metadata response, response code '{RC}' and completion code '{CC}'",
"RC", lg2::hex, rc, "CC", cc);
return;
}
@@ -947,7 +969,9 @@
std::move(getFruRecordTableMetadataResponseHandler));
if (rc != PLDM_SUCCESS)
{
- lg2::error("Failed to send the the Set State Effecter States request");
+ error(
+ "Failed to send the the set state effecter states request, response code '{RC}'",
+ "RC", rc);
}
return;
@@ -960,7 +984,7 @@
if (!totalTableRecords)
{
- lg2::error("Failed to get fru record table");
+ error("Failed to get fru record table");
return;
}
@@ -976,8 +1000,9 @@
if (rc != PLDM_SUCCESS)
{
instanceIdDb.free(mctp_eid, instanceId);
- lg2::error("Failed to encode_get_fru_record_table_req, rc = {RC}", "RC",
- lg2::hex, rc);
+ error(
+ "Failed to encode get fru record table request, response code '{RC}'",
+ "RC", lg2::hex, rc);
return;
}
@@ -986,8 +1011,7 @@
mctp_eid_t /*eid*/, const pldm_msg* response, size_t respMsgLen) {
if (response == nullptr || !respMsgLen)
{
- lg2::error(
- "Failed to receive response for the Get FRU Record Table");
+ error("Failed to receive response for the get fru record table");
return;
}
@@ -1005,8 +1029,8 @@
if (rc != PLDM_SUCCESS || cc != PLDM_SUCCESS)
{
- lg2::error(
- "Failed to decode get fru record table resp, Message Error: {RC}, cc: {CC}",
+ error(
+ "Failed to decode get fru record table resp, response code '{RC}' and completion code '{CC}'",
"RC", lg2::hex, rc, "CC", cc);
return;
}
@@ -1018,7 +1042,7 @@
{
fruRecordData.clear();
- lg2::error("failed to parse fru recrod data format.");
+ error("Failed to parse fru recrod data format.");
return;
}
@@ -1030,7 +1054,7 @@
std::move(requestMsg), std::move(getFruRecordTableResponseHandler));
if (rc != PLDM_SUCCESS)
{
- lg2::error("Failed to send the the Set State Effecter States request");
+ error("Failed to send the the set state effecter states request");
}
}