Change backup db logic 82/259282/6
authorIlho Kim <ilho159.kim@samsung.com>
Tue, 1 Jun 2021 04:43:13 +0000 (13:43 +0900)
committerilho kim <ilho159.kim@samsung.com>
Thu, 10 Jun 2021 07:42:31 +0000 (07:42 +0000)
 - Change to using sqlite3_backup api for making backup db
 - Try integrity check before restore backup db
   and if integrity check fail, the backup is cleared and continue to upgrade
 - Delete backup db when upgrade operation has failed

Change-Id: I0aacdbb7e756eb6ed2f48e683bea127057a09d13
Signed-off-by: Ilho Kim <ilho159.kim@samsung.com>
src/pkg_upgrade/CMakeLists.txt
src/pkg_upgrade/include/upgrader.hh
src/pkg_upgrade/src/upgrader.cc
tests/unit_tests/CMakeLists.txt
tests/unit_tests/data/db_bck/.pkgmgr_cert.db.bck-journal [moved from tests/unit_tests/data/db_bck/.pkgmgr_cert.db-journal.bck with 100% similarity]
tests/unit_tests/data/db_bck/.pkgmgr_parser.db.bck-journal [moved from tests/unit_tests/data/db_bck/.pkgmgr_parser.db-journal.bck with 100% similarity]

index 21fe1ac..581b7ae 100755 (executable)
@@ -12,6 +12,7 @@ APPLY_PKG_CONFIG(${TARGET_PKG_UPGRADE} PUBLIC
   TZPLATFORM_DEPS
   SMACK_DEPS
   DLOG_DEPS
+  SQLITE_DEPS
 )
 
 # Install
index f4eeb51..4d282bd 100644 (file)
@@ -46,15 +46,15 @@ class Upgrader {
   int CreateBackupFlag(const std::string& path);
   int CheckBackupFlag(const std::string& path);
   int RemoveBackupFlag(const std::string& path);
+  int IntegrityCheckBackupDbs();
+  int IntegrityCheck(const std::string& path);
 
  private:
   std::shared_ptr<utils::FileLogBackend> logger_;
   std::list<std::unique_ptr<PkgUpgrader>> success_list_;
   std::list<std::unique_ptr<PkgUpgrader>> failure_list_;
   std::string parser_db_;
-  std::string parser_db_journal_;
   std::string cert_db_;
-  std::string cert_db_journal_;
 };
 
 }  // common_fota
index 100e85f..c085bfe 100644 (file)
@@ -15,6 +15,7 @@
  */
 
 #include <fcntl.h>
+#include <sqlite3.h>
 #include <string.h>
 #include <stdlib.h>
 #include <stdio.h>
@@ -100,9 +101,7 @@ Upgrader::Upgrader() {
 
 void Upgrader::SetDbPath(const string& path) {
   parser_db_ = path + "/.pkgmgr_parser.db";
-  parser_db_journal_ = path + "/.pkgmgr_parser.db-journal";
   cert_db_ = path + "/.pkgmgr_cert.db";
-  cert_db_journal_ = path + "/.pkgmgr_cert.db-journal";
 }
 
 bool Upgrader::Process(PkgFinder* finder) {
@@ -145,16 +144,23 @@ const list<unique_ptr<PkgUpgrader>>& Upgrader::GetFailureList() const {
 }
 
 int Upgrader::CheckAndRestoreBackupDbs() {
-  if (CheckAndRestoreBackup(parser_db_))
-    return -1;
+  // if backup flag exists, it means the previous backup process aborted.
+  if (CheckBackupFlag(parser_db_) == 0 || CheckBackupFlag(cert_db_) == 0) {
+    RemoveBackupDbs();
+    RemoveBackupFlag(parser_db_);
+    RemoveBackupFlag(cert_db_);
+    return 0;
+  }
 
-  if (CheckAndRestoreBackup(parser_db_journal_))
-    return -1;
+  if (IntegrityCheckBackupDbs()) {
+    RemoveBackupDbs();
+    return 0;
+  }
 
-  if (CheckAndRestoreBackup(cert_db_))
+  if (CheckAndRestoreBackup(parser_db_))
     return -1;
 
-  if (CheckAndRestoreBackup(cert_db_journal_))
+  if (CheckAndRestoreBackup(cert_db_))
     return -1;
 
   return 0;
@@ -162,79 +168,139 @@ int Upgrader::CheckAndRestoreBackupDbs() {
 
 int Upgrader::CheckAndRestoreBackup(const string& origin_path) {
   string backup_path = origin_path + ".bck";
+  string journal_path = origin_path + "-journal";
+  string journal_backup_path = origin_path + ".bck-journal";
 
-  // if backup flag exists, it means the previous backup process aborted.
-  if (access(backup_path.c_str(), F_OK) && CheckBackupFlag(origin_path))
+  if (access(backup_path.c_str(), F_OK))
     return 0;
 
-  if (access(origin_path.c_str(), F_OK) == 0) {
-    if (remove(origin_path.c_str())) {
-      LOG(ERROR) << "cannot remove path(" << origin_path << " : " << errno;
+  if (access(journal_backup_path.c_str(), F_OK) == 0) {
+    if (rename(journal_backup_path.c_str(), journal_path.c_str())) {
+      LOG(ERROR) << "fail to rename " << journal_backup_path
+          << " to " << journal_path << " " << errno;
       return -1;
     }
   }
 
   if (rename(backup_path.c_str(), origin_path.c_str())) {
-    LOG(ERROR) << "fail to rename " << backup_path << " to " <<
-        origin_path << errno;
+    LOG(ERROR) << "fail to rename " << backup_path
+        << " to " << origin_path << " " << errno;
     return -1;
   }
 
   return 0;
 }
 
-int Upgrader::SetDbPermission(const string& path) {
-  struct stat sb;
-  struct passwd pwd;
-  struct passwd *result;
-  char buf[kBufSize];
+int Upgrader::IntegrityCheckBackupDbs() {
+  std::string parser_db_bck = parser_db_ + ".bck";
+  std::string cert_db_bck = cert_db_ + ".bck";
 
-  int ret = getpwnam_r(kAppfwUser, &pwd, buf, sizeof(buf), &result);
-  if (result == NULL) {
-    LOG(ERROR) << "getpwnam_r failed: " << errno;
+  if (IntegrityCheck(parser_db_bck) != 0) {
+    LOG(ERROR) << "Fail to integrity check db(" << parser_db_bck << "%s)";
     return -1;
   }
-  uid_t uid = pwd.pw_uid;
 
-  ret = getpwuid_r(uid, &pwd, buf, sizeof(buf), &result);
-  if (result == NULL) {
-    LOG(ERROR) << "getpwuid_r failed: " << errno;
+  if (IntegrityCheck(cert_db_bck) != 0) {
+    LOG(ERROR) << "Fail to integrity check db(" << cert_db_bck << "%s)";
     return -1;
   }
 
-  ::File file(path);
+  return 0;
+}
 
-  if (file.GetFd() == -1) {
-    LOG(ERROR) << "open(" << path << "failed: " << errno;
+int Upgrader::IntegrityCheck(const std::string& path) {
+  constexpr const char query[] = "PRAGMA integrity_check";
+  sqlite3* db;
+  sqlite3_stmt* stmt;
+
+  if (access(path.c_str(), F_OK))
+    return 0;
+
+  if (sqlite3_open_v2(path.c_str(), &db,
+      SQLITE_OPEN_READONLY, nullptr) != SQLITE_OK) {
+    LOG(ERROR) << "Fail to open (" << path << ") db handle";
     return -1;
   }
+  unique_ptr<sqlite3, decltype(sqlite3_close_v2)*>
+      db_auto(db, sqlite3_close_v2);
 
-  ret = file.FStat(&sb);
-  if (ret == -1) {
-    LOG(ERROR) << "stat " << path << "failed: " << errno;
+  if (sqlite3_prepare_v2(db, query, strlen(query),
+      &stmt, nullptr) != SQLITE_OK) {
+    LOG(ERROR) << "prepare failed : " << sqlite3_errmsg(db);
     return -1;
   }
-  if (S_ISLNK(sb.st_mode)) {
-    LOG(ERROR) << path << " is symlink!";
+  unique_ptr<sqlite3_stmt, decltype(sqlite3_finalize)*>
+      stmt_auto(stmt, sqlite3_finalize);
+
+  if (sqlite3_step(stmt) != SQLITE_ROW) {
+    LOG(ERROR) << "sqlite3_step fail";
     return -1;
   }
-  ret = file.FChOwn(uid, pwd.pw_gid);
-  if (ret == -1) {
-    LOG(ERROR) << "fchown " << path << " failed: " << errno;
+
+  std::string val = reinterpret_cast<const char*>(sqlite3_column_text(stmt, 0));
+  if (val != "ok") {
+    LOG(ERROR) << "Fail to integrity check, db("
+        << path << "), val(" << val << ")";
     return -1;
   }
 
-  mode_t mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH;
-  if (path.find(cert_db_) != string::npos)
-    mode |= S_IWOTH;
-  ret = file.FChMod(mode);
-  if (ret == -1) {
-    LOG(ERROR) << "fchmod " << path << " failed: " << errno;
+  return 0;
+}
+
+int Upgrader::SetDbPermission(const string& path) {
+  struct stat sb;
+  struct passwd pwd;
+  struct passwd* result;
+  char buf[kBufSize];
+
+  int ret = getpwnam_r(kAppfwUser, &pwd, buf, sizeof(buf), &result);
+  if (result == nullptr) {
+    LOG(ERROR) << "getpwnam_r failed: " << errno;
     return -1;
   }
-  file.FSync();
-  if (smack_setlabel(path.c_str(), kDbLabel, SMACK_LABEL_ACCESS))
-    LOG(ERROR) << "failed chsmack -a " << kDbLabel << " " << path;
+  uid_t uid = pwd.pw_uid;
+
+  ret = getpwuid_r(uid, &pwd, buf, sizeof(buf), &result);
+  if (result == nullptr) {
+    LOG(ERROR) << "getpwuid_r failed: " << errno;
+    return -1;
+  }
+
+  for (const auto& p : std::vector<std::string>{path, path + "-journal"}) {
+    ::File file(p);
+
+    if (file.GetFd() == -1) {
+      LOG(ERROR) << "open(" << p << "failed: " << errno;
+      return -1;
+    }
+
+    ret = file.FStat(&sb);
+    if (ret == -1) {
+      LOG(ERROR) << "stat " << p << "failed: " << errno;
+      return -1;
+    }
+    if (S_ISLNK(sb.st_mode)) {
+      LOG(ERROR) << p << " is symlink!";
+      return -1;
+    }
+    ret = file.FChOwn(uid, pwd.pw_gid);
+    if (ret == -1) {
+      LOG(ERROR) << "fchown " << p << " failed: " << errno;
+      return -1;
+    }
+
+    mode_t mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH;
+    if (p.find(cert_db_) != string::npos)
+      mode |= S_IWOTH;
+    ret = file.FChMod(mode);
+    if (ret == -1) {
+      LOG(ERROR) << "fchmod " << p << " failed: " << errno;
+      return -1;
+    }
+    file.FSync();
+    if (smack_setlabel(p.c_str(), kDbLabel, SMACK_LABEL_ACCESS))
+      LOG(ERROR) << "failed chsmack -a " << kDbLabel << " " << path;
+  }
 
   return 0;
 }
@@ -272,51 +338,57 @@ int Upgrader::RemoveBackupFlag(const string& path) {
 }
 
 int Upgrader::BackupFile(const string& src_path, const string& dest_path) {
-  char temp_buf[8192] = {'\0', };
-  size_t size_of_char = sizeof(char);
-  size_t size_of_temp_buf = sizeof(temp_buf);
+  int ret;
+  sqlite3* src_db;
+  sqlite3* dest_db;
+  sqlite3_backup* p_backup;
 
   if (access(src_path.c_str(), F_OK) != 0) {
     LOG(ERROR) << "File(" << src_path << ") is not exist";
     return -1;
   }
 
-  // if backup flag exists, it means the previous backup process aborted.
-  if (CheckBackupFlag(src_path)) {
-    if (access(dest_path.c_str(), F_OK) == 0) {
-      if (remove(dest_path.c_str())) {
-        LOG(ERROR) << "Failed to remove uncompleted backup file " <<
-            dest_path << " : " << errno;
-        return -1;
-      }
-    }
-  } else {
-    if (CreateBackupFlag(src_path)) {
-      LOG(ERROR) << "failed to create backup flag";
-      return -1;
-    }
+  if (CreateBackupFlag(src_path)) {
+    LOG(ERROR) << "failed to create backup flag";
+    return -1;
   }
 
-  FILE* src = fopen(src_path.c_str(), "r");
-  unique_ptr<FILE, decltype(fclose)*> src_auto(src, fclose);
-  if (src == nullptr) {
-    LOG(ERROR) << "Failed to open : " << src_path;
+  if (sqlite3_open_v2(src_path.c_str(), &src_db,
+      SQLITE_OPEN_READONLY, nullptr) != SQLITE_OK) {
+    LOG(ERROR) << "Failed to open ( "
+        << src_path << ") db handle : " << src_path;
     return -1;
   }
+  unique_ptr<sqlite3, decltype(sqlite3_close_v2)*>
+      src_db_auto(src_db, sqlite3_close_v2);
 
-  FILE* dest = fopen(dest_path.c_str(), "w");
-  unique_ptr<FILE, decltype(fclose)*> dest_auto(dest, fclose);
-  if (dest == nullptr) {
-    LOG(ERROR) << "Failed to open : " << dest_path;
+  if (sqlite3_open_v2(dest_path.c_str(), &dest_db,
+      SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE, nullptr) != SQLITE_OK) {
+    LOG(ERROR) << "Failed to open ( "
+        << dest_path << ") db handle : " << dest_path;
     return -1;
   }
+  unique_ptr<sqlite3, decltype(sqlite3_close_v2)*>
+      dest_db_auto(dest_db, sqlite3_close_v2);
+
+  p_backup = sqlite3_backup_init(dest_db, "main", src_db, "main");
+  if (p_backup) {
+    do {
+      ret = sqlite3_backup_step(p_backup, -1);
+      if (ret == SQLITE_BUSY || ret == SQLITE_LOCKED)
+        sqlite3_sleep(250);
+    } while (ret == SQLITE_OK || ret == SQLITE_BUSY
+        || ret == SQLITE_LOCKED);
+
+    sqlite3_backup_finish(p_backup);
+  }
 
-  while (!feof(src)) {
-    int rc = fread(temp_buf, size_of_char, size_of_temp_buf, src);
-    fwrite(temp_buf, size_of_char, rc, dest);
+  ret = sqlite3_errcode(dest_db);
+  if (ret != SQLITE_OK) {
+    LOG(ERROR) << "Fail to backup db val(" << ret << ")";
+    return -1;
   }
 
-  fsync(fileno(dest));
   RemoveBackupFlag(src_path);
 
   return 0;
@@ -334,72 +406,41 @@ int Upgrader::BackupDb(const string& src_path, const string& dest_path) {
 
 int Upgrader::MakeBackupDbs() {
   string parser_db_bck = parser_db_ + ".bck";
-  string parser_db_journal_bck = parser_db_journal_ + ".bck";
   string cert_db_bck = cert_db_ + ".bck";
-  string cert_db_journal_bck = cert_db_journal_ + ".bck";
 
   if (BackupDb(parser_db_, parser_db_bck) == -1) {
     LOG(ERROR) << "Fail to backup [" << parser_db_ <<
         "] to [" << parser_db_bck << "]";
-    goto CATCH;
-  }
-
-  if (BackupDb(parser_db_journal_, parser_db_journal_bck) == -1) {
-    LOG(ERROR) << "Fail to backup [" << parser_db_journal_ << "] to [" <<
-        parser_db_journal_bck << "]";
-    goto CATCH;
+    RemoveBackupDbs();
+    return -1;
   }
 
   if (BackupDb(cert_db_, cert_db_bck) == -1) {
     LOG(ERROR) << "Fail to backup [" << cert_db_ << "] to [" <<
         cert_db_bck << "]";
-    goto CATCH;
-  }
-
-  if (BackupDb(cert_db_journal_, cert_db_journal_bck) == -1) {
-    LOG(ERROR) << "Fail to backup [" << cert_db_journal_ << "] to [" <<
-        cert_db_journal_bck << "]";
-    goto CATCH;
+    RemoveBackupDbs();
+    return -1;
   }
 
   return 0;
-
-CATCH:
-  if (remove(parser_db_bck.c_str())) {
-    LOG(WARNING) << "cannot remove backup file("
-        << parser_db_bck << ") " << errno;
-  }
-
-  if (remove(parser_db_journal_bck.c_str())) {
-    LOG(WARNING) << "cannot remove backup file("
-        << parser_db_journal_bck << ") " << errno;
-  }
-
-  if (remove(cert_db_bck.c_str())) {
-    LOG(WARNING) << "cannot remove backup file("
-        << cert_db_bck << ") " << errno;
-  }
-
-  if (remove(cert_db_journal_bck.c_str())) {
-    LOG(WARNING) << "cannot remove backup file("
-        << cert_db_journal_bck << ") " << errno;
-  }
-
-  return -1;
 }
 
 void Upgrader::RemoveBackupPath(const string& origin_path) {
   string backup_path = origin_path + ".bck";
+  string journal_backup_path = origin_path + ".bck-journal";
 
-  if (remove(origin_path.c_str()))
+  if (access(backup_path.c_str(), F_OK) == 0 && remove(backup_path.c_str()))
     LOG(ERROR) << "cannot remove backup file(" << backup_path << ") " << errno;
+
+  if (access(journal_backup_path.c_str(), F_OK) == 0
+      && remove(journal_backup_path.c_str()))
+    LOG(ERROR) << "cannot remove backup file("
+        << journal_backup_path << ") " << errno;
 }
 
 void Upgrader::RemoveBackupDbs() {
   RemoveBackupPath(parser_db_);
-  RemoveBackupPath(parser_db_journal_);
   RemoveBackupPath(cert_db_);
-  RemoveBackupPath(cert_db_journal_);
 }
 
 }  // namespace common_fota
index b9cc613..02d8349 100644 (file)
@@ -1,45 +1,46 @@
-CMAKE_MINIMUM_REQUIRED(VERSION 2.8)\r
-PROJECT(pkgmgr-tool_unittests C CXX)\r
-\r
-INCLUDE(FindPkgConfig)\r
-PKG_CHECK_MODULES(pkgmgr-tool_unittests REQUIRED\r
-  dlog\r
-  gmock\r
-  pkgmgr-parser\r
-  pkgmgr-info\r
-  libtzplatform-config\r
-  libsmack\r
-)\r
-\r
-FOREACH(flag ${pkgmgr-tool_unittests_CFLAGS})\r
-    SET(EXTRA_CFLAGS "${EXTRA_CFLAGS} ${flag}")\r
-ENDFOREACH(flag)\r
-SET(EXTRA_CFLAGS "${EXTRA_CFLAGS} -fvisibility=hidden -Wall -Werror -fPIE")\r
-\r
-SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${EXTRA_CFLAGS} -std=c++14")\r
-SET(CMAKE_CXX_FLAGS_DEBUG "-O0 -g")\r
-SET(CMAKE_CXX_FLAGS_RELEASE "-O2")\r
-\r
-ADD_DEFINITIONS("-DDB_PATH=\"${DB_PATH}\"")\r
-\r
-INCLUDE_DIRECTORIES(${CMAKE_CURRENT_SOURCE_DIR}/../../src/pkg_upgrade/include)\r
-INCLUDE_DIRECTORIES(${CMAKE_CURRENT_SOURCE_DIR}/..)\r
-\r
-AUX_SOURCE_DIRECTORY(${CMAKE_CURRENT_SOURCE_DIR}/src SOURCES)\r
-AUX_SOURCE_DIRECTORY(${CMAKE_CURRENT_SOURCE_DIR}/../../src/pkg_upgrade/src LIB_SOURCES)\r
-LIST(REMOVE_ITEM LIB_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/../../src/pkg_upgrade/src/main.cc)\r
-AUX_SOURCE_DIRECTORY(${CMAKE_CURRENT_SOURCE_DIR}/../mock MOCK_SOURCES)\r
-\r
-ADD_EXECUTABLE(${PROJECT_NAME}\r
-       ${SOURCES}\r
-       ${MOCK_SOURCES}\r
-       ${LIB_SOURCES}\r
-)\r
-\r
-SET_TARGET_PROPERTIES(${PROJECT_NAME} PROPERTIES COMPILE_FLAGS "${EXTRA_CFLAGS}")\r
-TARGET_LINK_LIBRARIES(${PROJECT_NAME}\r
-    ${pkgmgr-tool_unittests_LDFLAGS}\r
-)\r
-\r
-INSTALL(TARGETS ${PROJECT_NAME} DESTINATION /usr/bin/)\r
-INSTALL(DIRECTORY data/ DESTINATION ${CMAKE_INSTALL_PREFIX}/share/${PROJECT_NAME}/data)
\ No newline at end of file
+CMAKE_MINIMUM_REQUIRED(VERSION 2.8)
+PROJECT(pkgmgr-tool_unittests C CXX)
+
+INCLUDE(FindPkgConfig)
+PKG_CHECK_MODULES(pkgmgr-tool_unittests REQUIRED
+  dlog
+  gmock
+  pkgmgr-parser
+  pkgmgr-info
+  libtzplatform-config
+  libsmack
+  sqlite3
+)
+
+FOREACH(flag ${pkgmgr-tool_unittests_CFLAGS})
+    SET(EXTRA_CFLAGS "${EXTRA_CFLAGS} ${flag}")
+ENDFOREACH(flag)
+SET(EXTRA_CFLAGS "${EXTRA_CFLAGS} -fvisibility=hidden -Wall -Werror -fPIE")
+
+SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${EXTRA_CFLAGS} -std=c++14")
+SET(CMAKE_CXX_FLAGS_DEBUG "-O0 -g")
+SET(CMAKE_CXX_FLAGS_RELEASE "-O2")
+
+ADD_DEFINITIONS("-DDB_PATH=\"${DB_PATH}\"")
+
+INCLUDE_DIRECTORIES(${CMAKE_CURRENT_SOURCE_DIR}/../../src/pkg_upgrade/include)
+INCLUDE_DIRECTORIES(${CMAKE_CURRENT_SOURCE_DIR}/..)
+
+AUX_SOURCE_DIRECTORY(${CMAKE_CURRENT_SOURCE_DIR}/src SOURCES)
+AUX_SOURCE_DIRECTORY(${CMAKE_CURRENT_SOURCE_DIR}/../../src/pkg_upgrade/src LIB_SOURCES)
+LIST(REMOVE_ITEM LIB_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/../../src/pkg_upgrade/src/main.cc)
+AUX_SOURCE_DIRECTORY(${CMAKE_CURRENT_SOURCE_DIR}/../mock MOCK_SOURCES)
+
+ADD_EXECUTABLE(${PROJECT_NAME}
+       ${SOURCES}
+       ${MOCK_SOURCES}
+       ${LIB_SOURCES}
+)
+
+SET_TARGET_PROPERTIES(${PROJECT_NAME} PROPERTIES COMPILE_FLAGS "${EXTRA_CFLAGS}")
+TARGET_LINK_LIBRARIES(${PROJECT_NAME}
+    ${pkgmgr-tool_unittests_LDFLAGS}
+)
+
+INSTALL(TARGETS ${PROJECT_NAME} DESTINATION /usr/bin/)
+INSTALL(DIRECTORY data/ DESTINATION ${CMAKE_INSTALL_PREFIX}/share/${PROJECT_NAME}/data)