commit 8bcf79d1ee0cec26358c5ba6f0868059213e48b3
author Kun Yi <kunyi731@gmail.com> Wed Jan 16 15:17:57 2019 -0800
committer Kun Yi <kunyi731@gmail.com> Tue Mar 05 15:00:04 2019 -0800
tree bdbce3e93b0bd9aa0e68b35d068b60faf9c4f0e3
parent 7005917ead758383bea4e44e68317ccc1f0d54db

binarystore: Add error logging across all the functions

Add error logging for all operations using phosphor-logging.

Signed-off-by: Kun Yi <kunyi@google.com>
Change-Id: Id6f9d2b9960f4aa1dda4d0ba1ace2fb192533424

binarystore.cpp

diff --git a/binarystore.cpp b/binarystore.cpp
index dff423e..9c37e47 100644
--- a/binarystore.cpp
+++ b/binarystore.cpp
@@ -7,6 +7,7 @@
 #include <boost/endian/arithmetic.hpp>
 #include <cstdint>
 #include <memory>
+#include <phosphor-logging/elog.hpp>
 #include <string>
 #include <vector>
 
@@ -21,6 +22,8 @@
 namespace binstore
 {
 
+using namespace phosphor::logging;
+
 std::unique_ptr<BinaryStoreInterface>
     BinaryStore::createFromConfig(const std::string& baseBlobId,
                                   std::unique_ptr<SysFile> file,
@@ -28,6 +31,8 @@
 {
     if (baseBlobId.empty() || !file)
     {
+        log<level::ERR>("Unable to create binarystore from invalid config",
+                        entry("BASE_ID=%s", baseBlobId.c_str()));
         return nullptr;
     }
 
@@ -61,12 +66,16 @@
 {
     if (!(flags & blobs::OpenFlags::read))
     {
+        log<level::ERR>("OpenFlags::read not specified when opening",
+                        entry("BLOB_ID=%s", blobId.c_str()));
         return false;
     }
 
     if (currentBlob_ && (currentBlob_->blob_id() != blobId))
     {
-        /* Already handling a different blob */
+        log<level::ERR>("Already handling a different blob",
+                        entry("EXPECTED=%s", currentBlob_->blob_id().c_str()),
+                        entry("RECEIVED=%s", blobId.c_str()));
         return false;
     }
 
@@ -76,6 +85,8 @@
      * Note it will overwrite existing unsaved data per design. */
     if (commitState_ != CommitState::Clean)
     {
+        log<level::NOTICE>("Try loading blob from persistent data",
+                           entry("BLOB_ID=%s", blobId.c_str()));
         try
         {
             /* Parse length-prefixed format to protobuf */
@@ -87,13 +98,16 @@
             {
                 /* Fail to parse the data, which might mean no preexsiting data
                  * and is a valid case to handle */
-                // TODO: logging
+                log<level::WARNING>(
+                    "Fail to parse. There might be no persisted blobs",
+                    entry("BLOB_ID=%s", blobId.c_str()));
             }
         }
         catch (const std::exception& e)
         {
             /* Read causes unexpected system-level failure */
-            // TODO: logging
+            log<level::ERR>("Reading from sysfile failed",
+                            entry("ERROR=%s", e.what()));
             return false;
         }
     }
@@ -115,6 +129,7 @@
     currentBlob_ = blob_.add_blobs();
     currentBlob_->set_blob_id(blobId);
 
+    log<level::NOTICE>("Created new blob", entry("BLOB_ID=%s", blobId.c_str()));
     return true;
 }
 
@@ -129,6 +144,7 @@
 
     if (!currentBlob_)
     {
+        log<level::ERR>("No open blob to read");
         return result;
     }
 
@@ -137,6 +153,9 @@
     /* If it is out of bound, return empty vector */
     if (offset >= dataPtr->size())
     {
+        log<level::ERR>("Read offset is beyond data size",
+                        entry("MAX_SIZE=0x%x", dataPtr->size()),
+                        entry("RECEIVED_OFFSET=0x%x", offset));
         return result;
     }
 
@@ -152,11 +171,13 @@
 {
     if (!currentBlob_)
     {
+        log<level::ERR>("No open blob to write");
         return false;
     }
 
     if (!writable_)
     {
+        log<level::ERR>("Open blob is not writable");
         return false;
     }
 
@@ -164,7 +185,7 @@
 
     if (offset > dataPtr->size())
     {
-        /* Will leave a gap with undefined data */
+        log<level::ERR>("Write would leave a gap with undefined data. Return.");
         return false;
     }
 
@@ -192,6 +213,8 @@
     catch (const std::exception& e)
     {
         commitState_ = CommitState::CommitError;
+        log<level::ERR>("Writing to sysfile failed",
+                        entry("ERROR=%s", e.what()));
         return false;
     };