commit 4546cd6c65cf75452fc43cebf60261c27c56e96e
author Mohammad Samiul Islam <samiul@google.com> Mon Jun 28 16:43:43 2021 +0100
committer Mohammad Samiul Islam <samiul@google.com> Tue Jun 29 15:25:43 2021 +0100
tree 5e96c4dde9bbaf79f2f2b536c051910d06b074c7
parent 804ac629bc2e564ac1593f99a63f9e97baafce58

Clean up ota-apex whenever we reserve space

Whenever we reserve space for OTA, it means there is a new OTA update
incoming. As such, any existing '.ota.apex' files in
/data/apex/decompressed belong to a previous OTA update.

We should clean up these old '.ota.apex' files so that there is enough
space for reservation.

Bug: 172911822
Test: atest ApexTestCases
Change-Id: Ief90c2df6cf1f6b488cf0b1793445a53c1ab9130
Merged-In: Ief90c2df6cf1f6b488cf0b1793445a53c1ab9130
(cherry picked from commit 6c6a6e00014eb0042a13a67533790edb51298f25)

apexd/apexd.cpp

diff --git a/apexd/apexd.cpp b/apexd/apexd.cpp
index 0987a4b..a744fd7 100644
--- a/apexd/apexd.cpp
+++ b/apexd/apexd.cpp
@@ -3136,30 +3136,31 @@
   com::android::apex::write(os, apex_info_list);
 }
 
-// Reserve |size| bytes in |dest_dir| by creating a zero-filled file
-// If |size| passed is 0, then we cleanup reserved space and any
-// ota_apex that has been processed as part of pre-reboot decompression.
+// Reserve |size| bytes in |dest_dir| by creating a zero-filled file.
+// Also, we always clean up ota_apex that has been processed as
+// part of pre-reboot decompression whenever we reserve space.
 Result<void> ReserveSpaceForCompressedApex(int64_t size,
                                            const std::string& dest_dir) {
   if (size < 0) {
     return Error() << "Cannot reserve negative byte of space";
   }
+
+  // Since we are reserving space, then we must be preparing for a new OTA.
+  // Clean up any processed ota_apex from previous OTA.
+  auto ota_apex_files =
+      FindFilesBySuffix(gConfig->decompression_dir, {kOtaApexPackageSuffix});
+  if (!ota_apex_files.ok()) {
+    return Error() << "Failed to clean up ota_apex: " << ota_apex_files.error();
+  }
+  for (const std::string& ota_apex : *ota_apex_files) {
+    RemoveFileIfExists(ota_apex);
+  }
+
   auto file_path = StringPrintf("%s/full.tmp", dest_dir.c_str());
   if (size == 0) {
     LOG(INFO) << "Cleaning up reserved space for compressed APEX";
     // Ota is being cancelled. Clean up reserved space
     RemoveFileIfExists(file_path);
-
-    // Clean up any processed ota_apex
-    auto ota_apex_files =
-        FindFilesBySuffix(gConfig->decompression_dir, {kOtaApexPackageSuffix});
-    if (!ota_apex_files.ok()) {
-      return Error() << "Failed to clean up ota_apex: "
-                     << ota_apex_files.error();
-    }
-    for (const std::string& ota_apex : *ota_apex_files) {
-      RemoveFileIfExists(ota_apex);
-    }
     return {};
   }
 

apexd/apexd_test.cpp

diff --git a/apexd/apexd_test.cpp b/apexd/apexd_test.cpp
index 0d47172..d2873f7 100644
--- a/apexd/apexd_test.cpp
+++ b/apexd/apexd_test.cpp
@@ -644,14 +644,22 @@
   ASSERT_EQ(files->size(), 0u);
 }
 
-TEST_F(ApexdUnitTest, ReserveSpaceForCapexCleansOtaApexIfPassedZero) {
+TEST_F(ApexdUnitTest, ReserveSpaceForCapexCleansOtaApex) {
   TemporaryDir dest_dir;
 
-  // Create an ota_apex first
   auto ota_apex_path = StringPrintf(
       "%s/ota_apex%s", GetDecompressionDir().c_str(), kOtaApexPackageSuffix);
-  fs::copy(GetTestFile("com.android.apex.compressed.v1_original.apex"),
-           ota_apex_path);
+  auto create_ota_apex = [&]() {
+    // Create an ota_apex first
+    fs::copy(GetTestFile("com.android.apex.compressed.v1_original.apex"),
+             ota_apex_path);
+    auto path_exists = PathExists(ota_apex_path);
+    ASSERT_TRUE(*path_exists);
+  };
+  create_ota_apex();
+
+  // Should not delete the reserved file if size passed is negative
+  ASSERT_FALSE(IsOk(ReserveSpaceForCompressedApex(-1, dest_dir.path)));
   auto path_exists = PathExists(ota_apex_path);
   ASSERT_TRUE(*path_exists);
 
@@ -659,6 +667,12 @@
   ASSERT_TRUE(IsOk(ReserveSpaceForCompressedApex(0, dest_dir.path)));
   path_exists = PathExists(ota_apex_path);
   ASSERT_FALSE(*path_exists);
+
+  create_ota_apex();
+  // Should delete the reserved file if size passed is positive
+  ASSERT_TRUE(IsOk(ReserveSpaceForCompressedApex(10, dest_dir.path)));
+  path_exists = PathExists(ota_apex_path);
+  ASSERT_FALSE(*path_exists);
 }
 
 TEST_F(ApexdUnitTest, ReserveSpaceForCompressedApexErrorForNegativeValue) {