commit 76f2c60aeddc5ded6d5d3642d06165365355a50a
author Riya Dixit <riyadixitagra@gmail.com> Thu Mar 28 07:34:12 2024 -0500
committer ManojKiran Eda <manojkiran.eda@gmail.com> Mon May 13 06:59:58 2024 +0000
tree cfe2d3aa8d4d5692c876cbc6a9b9d52d8246cb34
parent d6e10ada58b41970933eacfd7750a467602f4f38

common & fw-update: 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: I1481c025b6dc6a9200a13de38a4fe55b81bb25ea
Signed-off-by: Riya Dixit <riyadixitagra@gmail.com>

fw-update/update_manager.cpp

diff --git a/fw-update/update_manager.cpp b/fw-update/update_manager.cpp
index 0e3128e..428613b 100644
--- a/fw-update/update_manager.cpp
+++ b/fw-update/update_manager.cpp
@@ -40,8 +40,8 @@
             software::Activation::Activations::Activating)
         {
             error(
-                "Activation of PLDM FW update package already in progress, PACKAGE_VERSION={PKG_VERS}",
-                "PKG_VERS", parser->pkgVersion);
+                "Activation of PLDM fw update package for version '{VERSION}' already in progress.",
+                "VERSION", parser->pkgVersion);
             std::filesystem::remove(packageFilePath);
             return -1;
         }
@@ -56,8 +56,8 @@
     if (!package.good())
     {
         error(
-            "Opening the PLDM FW update package failed, ERR={ERR}, PACKAGEFILE={PKG_FILE}",
-            "ERR", unsigned(errno), "PKG_FILE", packageFilePath.c_str());
+            "Failed to open the PLDM fw update package file '{FILE}', error - {ERROR}.",
+            "ERROR", unsigned(errno), "FILE", packageFilePath.c_str());
         package.close();
         std::filesystem::remove(packageFilePath);
         return -1;
@@ -67,8 +67,9 @@
     if (packageSize < sizeof(pldm_package_header_information))
     {
         error(
-            "PLDM FW update package length less than the length of the package header information, PACKAGESIZE={PKG_SIZE}",
-            "PKG_SIZE", packageSize);
+            "PLDM fw update package length {SIZE} less than the length of the package header information '{PACKAGE_HEADER_INFO_SIZE}'.",
+            "SIZE", packageSize, "PACKAGE_HEADER_INFO_SIZE",
+            sizeof(pldm_package_header_information));
         package.close();
         std::filesystem::remove(packageFilePath);
         return -1;
@@ -113,7 +114,7 @@
     }
     catch (const std::exception& e)
     {
-        error("Invalid PLDM package header: {ERROR}", "ERROR", e);
+        error("Invalid PLDM package header, error - {ERROR}", "ERROR", e);
         activation = std::make_unique<Activation>(
             pldm::utils::DBusHandler::getBus(), objPath,
             software::Activation::Activations::Invalid, this);
@@ -207,7 +208,7 @@
         auto dur =
             std::chrono::duration<double, std::milli>(endTime - startTime)
                 .count();
-        error("Firmware update time: {DURATION}ms", "DURATION", dur);
+        info("Firmware update time: {DURATION}ms", "DURATION", dur);
         activation->activation(software::Activation::Activations::Active);
     }
     return;