pldmd, requester, softoff & utilities: 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: I230e9e1404db8c92c81e2f872183d691c91ff16c
Signed-off-by: Riya Dixit <riyadixitagra@gmail.com>
diff --git a/pldmd/pldmd.cpp b/pldmd/pldmd.cpp
index 0adba8c..1030c8e 100644
--- a/pldmd/pldmd.cpp
+++ b/pldmd/pldmd.cpp
@@ -135,7 +135,9 @@
header.command = hdrFields.command;
if (PLDM_SUCCESS != pack_pldm_header(&header, responseHdr))
{
- error("Failed adding response header: {ERROR}", "ERROR", e);
+ error(
+ "Failed to add response header for processing Rx, error - {ERROR}",
+ "ERROR", e);
return std::nullopt;
}
response.insert(response.end(), completion_code);
@@ -352,8 +354,9 @@
(*response).size());
if (returnCode != PLDM_REQUESTER_SUCCESS)
{
- warning("Failed to send PLDM response: {RETURN_CODE}",
- "RETURN_CODE", returnCode);
+ warning(
+ "Failed to send pldmTransport message for TID '{TID}', response code '{RETURN_CODE}'",
+ "TID", TID, "RETURN_CODE", returnCode);
}
}
}
@@ -365,13 +368,16 @@
// recovery mechanism for this daemon is to restart, and hence exit
// the event loop, that will cause this daemon to exit with a
// failure code.
- error("io exiting");
+ error(
+ "MCTP daemon closed the socket, IO exiting with response code '{RC}'",
+ "RC", returnCode);
io.get_event().exit(0);
}
else
{
- warning("Failed to receive PLDM request: {RETURN_CODE}",
- "RETURN_CODE", returnCode);
+ warning(
+ "Failed to receive PLDM request for pldmTransport, response code '{RETURN_CODE}'",
+ "RETURN_CODE", returnCode);
}
/* Free requestMsg after using */
free(requestMsg);
diff --git a/requester/handler.hpp b/requester/handler.hpp
index 5fc5f7b..3b1eca5 100644
--- a/requester/handler.hpp
+++ b/requester/handler.hpp
@@ -145,16 +145,19 @@
auto eid = key.eid;
if (this->handlers.contains(key))
{
- error("The eid:InstanceID {EID}:{IID} is using.", "EID",
- (unsigned)key.eid, "IID", (unsigned)key.instanceId);
+ info(
+ "Instance ID expiry for EID '{EID}' using InstanceID '{INSTANCEID}'",
+ "EID", (unsigned)key.eid, "INSTANCEID",
+ (unsigned)key.instanceId);
auto& [request, responseHandler,
timerInstance] = this->handlers[key];
request->stop();
auto rc = timerInstance->stop();
if (rc)
{
- error("Failed to stop the instance ID expiry timer. RC = {RC}",
- "RC", static_cast<int>(rc));
+ error(
+ "Failed to stop the instance ID expiry timer, response code '{RC}'",
+ "RC", static_cast<int>(rc));
}
// Call response handler with an empty response to indicate no
// response
@@ -205,7 +208,9 @@
if (rc)
{
instanceIdDb.free(requestMsg->key.eid, requestMsg->key.instanceId);
- error("Failure to send the PLDM request message");
+ error(
+ "Failure to send the PLDM request message for polling endpoint queue, response code '{RC}'",
+ "RC", rc);
endpointMessageQueues[eid]->activeRequest = false;
return rc;
}
@@ -219,8 +224,8 @@
{
instanceIdDb.free(requestMsg->key.eid, requestMsg->key.instanceId);
error(
- "Failed to start the instance ID expiry timer. RC = {ERR_EXCEP}",
- "ERR_EXCEP", e.what());
+ "Failed to start the instance ID expiry timer, error - {ERROR}",
+ "ERROR", e);
endpointMessageQueues[eid]->activeRequest = false;
return PLDM_ERROR;
}
@@ -251,8 +256,9 @@
if (handlers.contains(key))
{
- error("The eid:InstanceID {EID}:{IID} is using.", "EID",
- (unsigned)eid, "IID", (unsigned)instanceId);
+ error(
+ "Register request for EID '{EID}' is using InstanceID '{INSTANCEID}'",
+ "EID", (unsigned)eid, "INSTANCEID", (unsigned)instanceId);
return PLDM_ERROR;
}
@@ -297,8 +303,9 @@
auto rc = timerInstance->stop();
if (rc)
{
- error("Failed to stop the instance ID expiry timer. RC = {RC}",
- "RC", static_cast<int>(rc));
+ error(
+ "Failed to stop the instance ID expiry timer, response code '{RC}'",
+ "RC", static_cast<int>(rc));
}
responseHandler(eid, response, respMsgLen);
instanceIdDb.free(key.eid, key.instanceId);
diff --git a/requester/mctp_endpoint_discovery.cpp b/requester/mctp_endpoint_discovery.cpp
index 0d63cb2..0d63ad4 100644
--- a/requester/mctp_endpoint_discovery.cpp
+++ b/requester/mctp_endpoint_discovery.cpp
@@ -48,8 +48,9 @@
}
catch (const sdbusplus::exception_t& e)
{
- error("getSubtree call failed with, {ERROR} {PATH} {INTERFACE}",
- "ERROR", e, "PATH", MCTPPath, "INTERFACE", MCTPInterface);
+ error(
+ "Failed to getSubtree call at path '{PATH}' and interface '{INTERFACE}', error - {ERROR} ",
+ "ERROR", e, "PATH", MCTPPath, "INTERFACE", MCTPInterface);
return;
}
@@ -76,8 +77,9 @@
if (std::find(types.begin(), types.end(), mctpTypePLDM) !=
types.end())
{
- info("Adding Endpoint networkId={NETWORK} EID={EID}",
- "NETWORK", networkId, "EID", unsigned(eid));
+ info(
+ "Adding Endpoint networkId '{NETWORK}' and EID '{EID}'",
+ "NETWORK", networkId, "EID", unsigned(eid));
mctpInfos.emplace_back(
MctpInfo(eid, emptyUUID, "", networkId));
}
@@ -86,7 +88,7 @@
catch (const sdbusplus::exception_t& e)
{
error(
- "Error reading MCTP Endpoint property, {ERROR} {SERVICE} {PATH}",
+ "Error reading MCTP Endpoint property at path '{PATH}' and service '{SERVICE}', error - {ERROR}",
"ERROR", e, "SERVICE", service, "PATH", path);
return;
}
@@ -109,8 +111,9 @@
}
catch (const sdbusplus::exception_t& e)
{
- error("Error reading MCTP Endpoint addedInterace message, {ERROR}",
- "ERROR", e);
+ error(
+ "Error reading MCTP Endpoint added interface message, error - {ERROR}",
+ "ERROR", e);
return;
}
@@ -130,8 +133,9 @@
if (std::find(types.begin(), types.end(), mctpTypePLDM) !=
types.end())
{
- info("Adding Endpoint networkId={NETWORK} EID={EID}",
- "NETWORK", networkId, "EID", unsigned(eid));
+ info(
+ "Adding Endpoint networkId '{NETWORK}' and EID '{EID}'",
+ "NETWORK", networkId, "EID", unsigned(eid));
mctpInfos.emplace_back(
MctpInfo(eid, emptyUUID, "", networkId));
}
@@ -165,8 +169,9 @@
}
for (const auto& mctpInfo : removedInfos)
{
- info("Removing Endpoint networkId={NETWORK} EID={EID}", "NETWORK",
- std::get<3>(mctpInfo), "EID", unsigned(std::get<0>(mctpInfo)));
+ info("Removing Endpoint networkId '{NETWORK}' and EID '{EID}'",
+ "NETWORK", std::get<3>(mctpInfo), "EID",
+ unsigned(std::get<0>(mctpInfo)));
existingMctpInfos.erase(std::remove(existingMctpInfos.begin(),
existingMctpInfos.end(), mctpInfo),
existingMctpInfos.end());
diff --git a/requester/request.hpp b/requester/request.hpp
index cb4e026..ff17886 100644
--- a/requester/request.hpp
+++ b/requester/request.hpp
@@ -75,8 +75,8 @@
}
catch (const std::runtime_error& e)
{
- error("Failed to start the request timer. RC = {ERR_EXCEP}",
- "ERR_EXCEP", e.what());
+ error("Failed to start the request timer, error - {ERROR}", "ERROR",
+ e);
return PLDM_ERROR;
}
@@ -89,8 +89,8 @@
auto rc = timer.stop();
if (rc)
{
- error("Failed to stop the request timer. RC = {RC}", "RC",
- static_cast<int>(rc));
+ error("Failed to stop the request timer, response code '{RC}'",
+ "RC", static_cast<int>(rc));
}
}
@@ -195,8 +195,9 @@
requestMsg.data(), requestMsg.size());
if (rc < 0)
{
- error("Failed to send PLDM message. RC = {RC}, errno = {ERR}", "RC",
- static_cast<int>(rc), "ERR", errno);
+ error(
+ "Failed to send pldmTransport message, response code '{RC}' and error - {ERROR}",
+ "RC", static_cast<int>(rc), "ERROR", errno);
return PLDM_ERROR;
}
return PLDM_SUCCESS;
diff --git a/softoff/main.cpp b/softoff/main.cpp
index cfbbcd2..ae81814 100644
--- a/softoff/main.cpp
+++ b/softoff/main.cpp
@@ -25,14 +25,14 @@
if (softPower.isError())
{
error(
- "Host failed to gracefully shutdown, exiting pldm-softpoweroff app");
+ "Failure in gracefully shutdown by remote terminus, exiting pldm-softpoweroff app");
return -1;
}
if (softPower.isCompleted())
{
error(
- "Host current state is not Running, exiting pldm-softpoweroff app");
+ "Remote terminus current state is not Running, exiting pldm-softpoweroff app");
return 0;
}
@@ -41,7 +41,7 @@
if (softPower.hostSoftOff(event))
{
error(
- "pldm-softpoweroff:Failure in sending soft off request to the host. Exiting pldm-softpoweroff app");
+ "Failure in sending soft off request to the remote terminus. Exiting pldm-softpoweroff app");
return -1;
}
@@ -50,7 +50,7 @@
pldm::utils::reportError(
"pldm soft off: Waiting for the host soft off timeout");
error(
- "PLDM host soft off: ERROR! Wait for the host soft off timeout. Exit the pldm-softpoweroff");
+ "ERROR! Waiting for the host soft off timeout. Exit the pldm-softpoweroff");
return -1;
}
diff --git a/softoff/softoff.cpp b/softoff/softoff.cpp
index df5807e..c522635 100644
--- a/softoff/softoff.cpp
+++ b/softoff/softoff.cpp
@@ -46,7 +46,7 @@
if (jsonData.is_discarded())
{
- error("Parsing softoff config JSON file failed");
+ error("Failed to parse softoff config JSON file");
return;
}
@@ -69,8 +69,8 @@
auto rc = getSensorInfo(entityType, stateSetId);
if (rc != PLDM_SUCCESS)
{
- error("Message get Sensor PDRs error. PLDM error code = {RC}",
- "RC", lg2::hex, static_cast<int>(rc));
+ error("Failed to get Sensor PDRs, response code '{RC}'", "RC",
+ lg2::hex, static_cast<int>(rc));
hasError = true;
return;
}
@@ -113,8 +113,9 @@
}
catch (const std::exception& e)
{
- error("PLDM host soft off: Can't get current host state: {ERROR}",
- "ERROR", e);
+ error(
+ "PLDM remote terminus soft off. Can't get current remote terminus state, error - {ERROR}",
+ "ERROR", e);
hasError = true;
return PLDM_ERROR;
}
@@ -141,8 +142,9 @@
auto rc = timer.stop();
if (rc < 0)
{
- error("PLDM soft off: Failure to STOP the timer. ERRNO={RC}", "RC",
- rc);
+ error(
+ "Failure to STOP the timer of PLDM soft off, response code '{RC}'",
+ "RC", rc);
}
// This marks the completion of pldm soft power off.
@@ -157,7 +159,9 @@
if (!fs::exists(softoffConfigJson) || fs::is_empty(softoffConfigJson))
{
- error("Parsing softoff config JSON file failed, File does not exist");
+ error(
+ "Failed to parse softoff config JSON file '{PATH}', file does not exist",
+ "PATH", softoffConfigJson);
return PLDM_ERROR;
}
@@ -195,8 +199,7 @@
}
catch (const sdbusplus::exception_t& e)
{
- error("PLDM soft off: Error get softPowerOff PDR,ERROR={ERR_EXCEP}",
- "ERR_EXCEP", e.what());
+ error("Failed to get softPowerOff PDR, error - {ERROR}", "ERROR", e);
return false;
}
return true;
@@ -259,8 +262,8 @@
}
catch (const sdbusplus::exception_t& e)
{
- error("PLDM soft off: Error get State Sensor PDR,ERROR={ERR_EXCEP}",
- "ERR_EXCEP", e.what());
+ error("Failed to get state sensor PDR during soft-off, error - {ERROR}",
+ "ERROR", e);
return PLDM_ERROR;
}
@@ -291,8 +294,9 @@
if (rc != PLDM_SUCCESS)
{
instanceIdDb.free(pldmTID, instanceID);
- error("Message encode failure. PLDM error code = {RC}", "RC", lg2::hex,
- static_cast<int>(rc));
+ error(
+ "Failed to encode set state effecter states request message, response code '{RC}'",
+ "RC", lg2::hex, static_cast<int>(rc));
return PLDM_ERROR;
}
@@ -303,7 +307,7 @@
{
instanceIdDb.free(pldmTID, instanceID);
error(
- "PLDM soft off: ERROR! Can't get the response for the PLDM request msg. Time out! Exit the pldm-softpoweroff");
+ "PLDM soft off failed, can't get the response for the PLDM request msg. Time out! Exit the pldm-softpoweroff");
exit(-1);
}
return;
@@ -331,8 +335,9 @@
auto rc = pldmTransport.recvMsg(pldmTID, responseMsg, responseMsgSize);
if (rc)
{
- error("Soft off: failed to recv pldm data. PLDM RC = {RC}", "RC",
- static_cast<int>(rc));
+ error(
+ "Failed to receive pldm data during soft-off, response code '{RC}'",
+ "RC", static_cast<int>(rc));
return;
}
@@ -357,7 +362,7 @@
if (response->payload[0] != PLDM_SUCCESS)
{
- error("Getting the wrong response. PLDM RC = {RC}", "RC",
+ error("Getting the wrong response, response code '{RC}'", "RC",
(unsigned)response->payload[0]);
exit(-1);
}
@@ -373,14 +378,14 @@
if (ret < 0)
{
error(
- "Failure to start Host soft off wait timer, ERRNO = {RET}. Exit the pldm-softpoweroff",
- "RET", ret);
+ "Failure to start remote terminus soft off wait timer, Exit the pldm-softpoweroff with response code:{NUM}",
+ "NUM", ret);
exit(-1);
}
else
{
error(
- "Timer started waiting for host soft off, TIMEOUT_IN_SEC = {TIMEOUT_SEC}",
+ "Timer started waiting for remote terminus soft off, timeout in sec '{TIMEOUT_SEC}'",
"TIMEOUT_SEC", SOFTOFF_TIMEOUT_SECONDS);
}
return;
@@ -393,8 +398,8 @@
{
instanceIdDb.free(pldmTID, instanceID);
error(
- "Failed to send message/receive response. RC = {RC}, errno = {ERR}",
- "RC", static_cast<int>(rc), "ERR", errno);
+ "Failed to send message/receive response, response code '{RC}' and error - {ERROR}",
+ "RC", static_cast<int>(rc), "ERROR", errno);
return PLDM_ERROR;
}
@@ -409,8 +414,8 @@
{
instanceIdDb.free(pldmTID, instanceID);
error(
- "PLDM host soft off: Failure in processing request.ERROR= {ERR_EXCEP}",
- "ERR_EXCEP", e.what());
+ "Failed to process request while remote terminus soft off, error - {ERROR}",
+ "ERROR", e);
return PLDM_ERROR;
}
}
diff --git a/utilities/requester/set_state_effecter.cpp b/utilities/requester/set_state_effecter.cpp
index 21c332d..19ffd7a 100644
--- a/utilities/requester/set_state_effecter.cpp
+++ b/utilities/requester/set_state_effecter.cpp
@@ -32,8 +32,9 @@
&stateField, request);
if (rc != PLDM_SUCCESS)
{
- error("Message encode failure. PLDM error code = {RC}", "RC", lg2::hex,
- rc);
+ error(
+ "Failed to encode set state effecter states request message, response code '{RC}'",
+ "RC", lg2::hex, rc);
return -1;
}
@@ -48,13 +49,14 @@
if (0 > rc)
{
error(
- "Failed to send message/receive response. RC = {RC}, errno = {ERR}",
- "RC", rc, "ERR", errno);
+ "Failed to send message/receive response, response code '{RC}' and error - {ERROR}",
+ "RC", rc, "ERROR", errno);
return -1;
}
pldm_msg* response = reinterpret_cast<pldm_msg*>(responseMsg);
- info("Done. PLDM RC = {RC}", "RC", lg2::hex,
- static_cast<uint16_t>(response->payload[0]));
+ info(
+ "Done! Got the response for PLDM send receive message request, response code '{RC}'",
+ "RC", lg2::hex, static_cast<uint16_t>(response->payload[0]));
free(responseMsg);
return 0;
diff --git a/utilities/requester/set_state_effecter_async.cpp b/utilities/requester/set_state_effecter_async.cpp
index 80b56c2..0116deb 100644
--- a/utilities/requester/set_state_effecter_async.cpp
+++ b/utilities/requester/set_state_effecter_async.cpp
@@ -39,8 +39,9 @@
&stateField, request);
if (rc != PLDM_SUCCESS)
{
- error("Message encode failure. PLDM error code = {RC}", "RC", lg2::hex,
- rc);
+ error(
+ "Failed to encode set state effecter states request, response code '{RC}'",
+ "RC", lg2::hex, rc);
return -1;
}
@@ -74,8 +75,9 @@
// We've got the response meant for the PLDM request msg that was sent
// out
io.set_enabled(Enabled::Off);
- info("Done. PLDM RC = {RC}", "RC", lg2::hex,
- static_cast<uint16_t>(response->payload[0]));
+ info(
+ "Done! Got the response for PLDM request message, response code '{RC}'",
+ "RC", lg2::hex, static_cast<uint16_t>(response->payload[0]));
free(responseMsg);
exit(EXIT_SUCCESS);
};
@@ -85,8 +87,8 @@
if (0 > rc)
{
error(
- "Failed to send message/receive response. RC = {RC} errno = {ERR}",
- "RC", rc, "ERR", errno);
+ "Failed to send message/receive response, response code '{RC}' and error - {ERROR}",
+ "RC", rc, "ERROR", errno);
return -1;
}