Merge "installd: Add destroyAppDataSnapshot and corresponding binder API"
am: 1b3315a657

Change-Id: I3342b12dcabb10795440e95d31f80c237d29e6a7
diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp
index 8146cc6..c7d9245 100644
--- a/cmds/installd/InstalldNativeService.cpp
+++ b/cmds/installd/InstalldNativeService.cpp
@@ -167,6 +167,15 @@
     }
 }
 
+binder::Status checkArgumentUuidTestOrNull(const std::unique_ptr<std::string>& uuid) {
+    if (!uuid || strcmp(uuid->c_str(), kTestUuid) == 0) {
+        return ok();
+    } else {
+        return exception(binder::Status::EX_ILLEGAL_ARGUMENT,
+                StringPrintf("UUID must be null or \"%s\", got: %s", kTestUuid, uuid->c_str()));
+    }
+}
+
 binder::Status checkArgumentPackageName(const std::string& packageName) {
     if (is_valid_package_name(packageName.c_str())) {
         return ok();
@@ -219,6 +228,13 @@
     }                                                       \
 }
 
+#define CHECK_ARGUMENT_UUID_IS_TEST_OR_NULL(uuid) {         \
+    auto status = checkArgumentUuidTestOrNull(uuid);        \
+    if (!status.isOk()) {                                   \
+        return status;                                      \
+    }                                                       \
+}                                                           \
+
 #define CHECK_ARGUMENT_PACKAGE_NAME(packageName) {          \
     binder::Status status =                                 \
             checkArgumentPackageName((packageName));        \
@@ -768,20 +784,21 @@
 
 binder::Status InstalldNativeService::snapshotAppData(
         const std::unique_ptr<std::string>& volumeUuid,
-        const std::string& packageName, int32_t user, int32_t storageFlags) {
+        const std::string& packageName, int32_t user, int32_t storageFlags,
+        int64_t* _aidl_return) {
     ENFORCE_UID(AID_SYSTEM);
+    CHECK_ARGUMENT_UUID_IS_TEST_OR_NULL(volumeUuid);
     CHECK_ARGUMENT_PACKAGE_NAME(packageName);
     std::lock_guard<std::recursive_mutex> lock(mLock);
 
     const char* volume_uuid = volumeUuid ? volumeUuid->c_str() : nullptr;
     const char* package_name = packageName.c_str();
 
-    if (volume_uuid && strcmp(volume_uuid, kTestUuid)) {
-        return exception(binder::Status::EX_ILLEGAL_ARGUMENT,
-                StringPrintf("volumeUuid must be null or \"%s\", got: %s", kTestUuid, volume_uuid));
-    }
-
     binder::Status res = ok();
+    // Default result to 0, it will be populated with inode of ce data snapshot
+    // if FLAG_STORAGE_CE has been passed.
+    if (_aidl_return != nullptr) *_aidl_return = 0;
+
     bool clear_ce_on_exit = false;
     bool clear_de_on_exit = false;
 
@@ -862,6 +879,16 @@
             clear_ce_on_exit = true;
             return res;
         }
+        if (_aidl_return != nullptr) {
+            auto ce_snapshot_path = create_data_misc_ce_rollback_package_path(volume_uuid, user,
+                    package_name);
+            rc = get_path_inode(ce_snapshot_path, reinterpret_cast<ino_t*>(_aidl_return));
+            if (rc != 0) {
+                res = error(rc, "Failed to get_path_inode for " + ce_snapshot_path);
+                clear_ce_on_exit = true;
+                return res;
+            }
+        }
     }
 
     return res;
@@ -872,17 +899,13 @@
         const int32_t appId, const int64_t ceDataInode, const std::string& seInfo,
         const int32_t user, int32_t storageFlags) {
     ENFORCE_UID(AID_SYSTEM);
+    CHECK_ARGUMENT_UUID_IS_TEST_OR_NULL(volumeUuid);
     CHECK_ARGUMENT_PACKAGE_NAME(packageName);
     std::lock_guard<std::recursive_mutex> lock(mLock);
 
     const char* volume_uuid = volumeUuid ? volumeUuid->c_str() : nullptr;
     const char* package_name = packageName.c_str();
 
-    if (volume_uuid && strcmp(volume_uuid, kTestUuid)) {
-        return exception(binder::Status::EX_ILLEGAL_ARGUMENT,
-                StringPrintf("volumeUuid must be null or \"%s\", got: %s", kTestUuid, volume_uuid));
-    }
-
     auto from_ce = create_data_misc_ce_rollback_package_path(volume_uuid,
             user, package_name);
     auto from_de = create_data_misc_de_rollback_package_path(volume_uuid,
@@ -933,6 +956,39 @@
     return restoreconAppData(volumeUuid, packageName, user, storageFlags, appId, seInfo);
 }
 
+binder::Status InstalldNativeService::destroyAppDataSnapshot(
+        const std::unique_ptr<std::string> &volumeUuid, const std::string& packageName,
+        const int32_t user, const int64_t ceSnapshotInode, int32_t storageFlags) {
+    ENFORCE_UID(AID_SYSTEM);
+    CHECK_ARGUMENT_UUID_IS_TEST_OR_NULL(volumeUuid);
+    CHECK_ARGUMENT_PACKAGE_NAME(packageName);
+    std::lock_guard<std::recursive_mutex> lock(mLock);
+
+    const char* volume_uuid = volumeUuid ? volumeUuid->c_str() : nullptr;
+    const char* package_name = packageName.c_str();
+
+    if (storageFlags & FLAG_STORAGE_DE) {
+        auto de_snapshot_path = create_data_misc_de_rollback_package_path(volume_uuid,
+                user, package_name);
+
+        int res = delete_dir_contents_and_dir(de_snapshot_path, true /* ignore_if_missing */);
+        if (res != 0) {
+            return error(res, "Failed clearing snapshot " + de_snapshot_path);
+        }
+    }
+
+    if (storageFlags & FLAG_STORAGE_CE) {
+        auto ce_snapshot_path = create_data_misc_ce_rollback_package_path(volume_uuid,
+                user, package_name, ceSnapshotInode);
+        int res = delete_dir_contents_and_dir(ce_snapshot_path, true /* ignore_if_missing */);
+        if (res != 0) {
+            return error(res, "Failed clearing snapshot " + ce_snapshot_path);
+        }
+    }
+    return ok();
+}
+
+
 binder::Status InstalldNativeService::moveCompleteApp(const std::unique_ptr<std::string>& fromUuid,
         const std::unique_ptr<std::string>& toUuid, const std::string& packageName,
         const std::string& dataAppName, int32_t appId, const std::string& seInfo,
diff --git a/cmds/installd/InstalldNativeService.h b/cmds/installd/InstalldNativeService.h
index 098a0c2..578132d 100644
--- a/cmds/installd/InstalldNativeService.h
+++ b/cmds/installd/InstalldNativeService.h
@@ -61,10 +61,14 @@
     binder::Status fixupAppData(const std::unique_ptr<std::string>& uuid, int32_t flags);
 
     binder::Status snapshotAppData(const std::unique_ptr<std::string>& volumeUuid,
-            const std::string& packageName, const int32_t user, int32_t storageFlags);
+            const std::string& packageName, const int32_t user, int32_t storageFlags,
+            int64_t* _aidl_return);
     binder::Status restoreAppDataSnapshot(const std::unique_ptr<std::string>& volumeUuid,
             const std::string& packageName, const int32_t appId, const int64_t ceDataInode,
             const std::string& seInfo, const int32_t user, int32_t storageFlags);
+    binder::Status destroyAppDataSnapshot(const std::unique_ptr<std::string> &volumeUuid,
+            const std::string& packageName, const int32_t user, const int64_t ceSnapshotInode,
+            int32_t storageFlags);
 
     binder::Status getAppSize(const std::unique_ptr<std::string>& uuid,
             const std::vector<std::string>& packageNames, int32_t userId, int32_t flags,
diff --git a/cmds/installd/binder/android/os/IInstalld.aidl b/cmds/installd/binder/android/os/IInstalld.aidl
index a70e9ff..377074a 100644
--- a/cmds/installd/binder/android/os/IInstalld.aidl
+++ b/cmds/installd/binder/android/os/IInstalld.aidl
@@ -105,14 +105,12 @@
         int userId, int appId, @utf8InCpp String profileName, @utf8InCpp String codePath,
         @nullable @utf8InCpp String dexMetadata);
 
-    void snapshotAppData(@nullable @utf8InCpp String uuid, in @utf8InCpp String packageName,
+    long snapshotAppData(@nullable @utf8InCpp String uuid, in @utf8InCpp String packageName,
             int userId, int storageFlags);
     void restoreAppDataSnapshot(@nullable @utf8InCpp String uuid, in @utf8InCpp String packageName,
             int appId, long ceDataInode, @utf8InCpp String seInfo, int user, int storageflags);
-
-    // TODO(narayan) we need an API to delete the app data snapshot as well.
-    // void destroyAppDataSnapshot(@nullable @utf8InCpp String uuid,
-    //        in @utf8InCpp String packageName, int userId, int storageFlags);
+    void destroyAppDataSnapshot(@nullable @utf8InCpp String uuid, @utf8InCpp String packageName,
+            int userId, long ceSnapshotInode, int storageFlags);
 
     const int FLAG_STORAGE_DE = 0x1;
     const int FLAG_STORAGE_CE = 0x2;
diff --git a/cmds/installd/tests/installd_service_test.cpp b/cmds/installd/tests/installd_service_test.cpp
index dae138a..cf7f1eb 100644
--- a/cmds/installd/tests/installd_service_test.cpp
+++ b/cmds/installd/tests/installd_service_test.cpp
@@ -292,8 +292,13 @@
           0700, 10000, 20000, false /* follow_symlinks */));
 
   // Request a snapshot of the CE content but not the DE content.
+  int64_t ce_snapshot_inode;
   ASSERT_BINDER_SUCCESS(service->snapshotAppData(std::make_unique<std::string>("TEST"),
-          "com.foo", 0, FLAG_STORAGE_CE));
+          "com.foo", 0, FLAG_STORAGE_CE, &ce_snapshot_inode));
+  struct stat buf;
+  memset(&buf, 0, sizeof(buf));
+  ASSERT_EQ(0, stat((rollback_ce_dir + "/com.foo").c_str(), &buf));
+  ASSERT_EQ(ce_snapshot_inode, (int64_t) buf.st_ino);
 
   std::string ce_content, de_content;
   // At this point, we should have the CE content but not the DE content.
@@ -311,7 +316,9 @@
 
   // Request a snapshot of the DE content but not the CE content.
   ASSERT_BINDER_SUCCESS(service->snapshotAppData(std::make_unique<std::string>("TEST"),
-          "com.foo", 0, FLAG_STORAGE_DE));
+          "com.foo", 0, FLAG_STORAGE_DE, &ce_snapshot_inode));
+  // Only DE content snapshot was requested.
+  ASSERT_EQ(ce_snapshot_inode, 0);
 
   // At this point, both the CE as well as the DE content should be fully
   // populated.
@@ -330,7 +337,7 @@
 
   // Request a snapshot of both the CE as well as the DE content.
   ASSERT_BINDER_SUCCESS(service->snapshotAppData(std::make_unique<std::string>("TEST"),
-          "com.foo", 0, FLAG_STORAGE_DE | FLAG_STORAGE_CE));
+          "com.foo", 0, FLAG_STORAGE_DE | FLAG_STORAGE_CE, nullptr));
 
   ASSERT_TRUE(android::base::ReadFileToString(
       rollback_ce_dir + "/com.foo/file1", &ce_content, false /* follow_symlinks */));
@@ -356,10 +363,13 @@
 
   auto scope_guard = android::base::make_scope_guard(deleter);
 
+  int64_t ce_snapshot_inode;
   ASSERT_BINDER_SUCCESS(service->snapshotAppData(std::make_unique<std::string>("TEST"),
-          "com.foo", 0, FLAG_STORAGE_CE));
+          "com.foo", 0, FLAG_STORAGE_CE, &ce_snapshot_inode));
   ASSERT_BINDER_SUCCESS(service->snapshotAppData(std::make_unique<std::string>("TEST"),
-          "com.foo", 0, FLAG_STORAGE_DE));
+          "com.foo", 0, FLAG_STORAGE_DE, nullptr));
+  // No CE content snapshot was performed.
+  ASSERT_EQ(ce_snapshot_inode, 0);
 
   // The snapshot calls must succeed but there should be no snapshot
   // created.
@@ -409,7 +419,7 @@
           0700, 10000, 20000, false /* follow_symlinks */));
 
   ASSERT_BINDER_SUCCESS(service->snapshotAppData(std::make_unique<std::string>("TEST"),
-          "com.foo", 0, FLAG_STORAGE_DE | FLAG_STORAGE_CE));
+          "com.foo", 0, FLAG_STORAGE_DE | FLAG_STORAGE_CE, nullptr));
 
   // Previous snapshot (with data for file1) must be cleared.
   struct stat sb;
@@ -435,7 +445,7 @@
   auto scope_guard = android::base::make_scope_guard(deleter);
 
   EXPECT_BINDER_FAIL(service->snapshotAppData(std::make_unique<std::string>("FOO"),
-          "com.foo", 0, FLAG_STORAGE_DE));
+          "com.foo", 0, FLAG_STORAGE_DE, nullptr));
 }
 
 TEST_F(ServiceTest, CreateAppDataSnapshot_ClearsCache) {
@@ -484,7 +494,7 @@
           "TEST_CONTENT_DE", fake_package_de_code_cache_path + "/file1",
           0700, 10000, 20000, false /* follow_symlinks */));
   ASSERT_BINDER_SUCCESS(service->snapshotAppData(std::make_unique<std::string>("TEST"),
-          "com.foo", 0, FLAG_STORAGE_CE | FLAG_STORAGE_DE));
+          "com.foo", 0, FLAG_STORAGE_CE | FLAG_STORAGE_DE, nullptr));
   // The snapshot call must clear cache.
   struct stat sb;
   ASSERT_EQ(-1, stat((fake_package_ce_cache_path + "/file1").c_str(), &sb));
@@ -546,6 +556,133 @@
   ASSERT_EQ("DE_RESTORE_CONTENT", de_content);
 }
 
+TEST_F(ServiceTest, CreateSnapshotThenDestroyIt) {
+  auto rollback_ce_dir = create_data_misc_ce_rollback_path("TEST", 0);
+  auto rollback_de_dir = create_data_misc_de_rollback_path("TEST", 0);
+
+  ASSERT_TRUE(mkdirs(rollback_ce_dir, 700));
+  ASSERT_TRUE(mkdirs(rollback_de_dir, 700));
+
+  auto fake_package_ce_path = create_data_user_ce_package_path("TEST", 0, "com.foo");
+  auto fake_package_de_path = create_data_user_de_package_path("TEST", 0, "com.foo");
+
+  ASSERT_TRUE(mkdirs(fake_package_ce_path, 700));
+  ASSERT_TRUE(mkdirs(fake_package_de_path, 700));
+
+  auto deleter = [&rollback_ce_dir, &rollback_de_dir,
+          &fake_package_ce_path, &fake_package_de_path]() {
+      delete_dir_contents(rollback_ce_dir, true);
+      delete_dir_contents(rollback_de_dir, true);
+      delete_dir_contents(fake_package_ce_path, true);
+      delete_dir_contents(fake_package_de_path, true);
+      rmdir(rollback_ce_dir.c_str());
+      rmdir(rollback_de_dir.c_str());
+  };
+  auto scope_guard = android::base::make_scope_guard(deleter);
+
+  // Prepare data for snapshot.
+  ASSERT_TRUE(android::base::WriteStringToFile(
+          "TEST_CONTENT_CE", fake_package_ce_path + "/file1",
+          0700, 10000, 20000, false /* follow_symlinks */));
+  ASSERT_TRUE(android::base::WriteStringToFile(
+          "TEST_CONTENT_DE", fake_package_de_path + "/file1",
+          0700, 10000, 20000, false /* follow_symlinks */));
+
+  int64_t ce_snapshot_inode;
+  // Request a snapshot of both the CE as well as the DE content.
+  ASSERT_TRUE(service->snapshotAppData(std::make_unique<std::string>("TEST"),
+          "com.foo", 0, FLAG_STORAGE_DE | FLAG_STORAGE_CE, &ce_snapshot_inode).isOk());
+  // Because CE data snapshot was requested, ce_snapshot_inode can't be null.
+  ASSERT_NE(0, ce_snapshot_inode);
+  // Check snapshot is there.
+  struct stat sb;
+  ASSERT_EQ(0, stat((rollback_ce_dir + "/com.foo").c_str(), &sb));
+  ASSERT_EQ(0, stat((rollback_de_dir + "/com.foo").c_str(), &sb));
+
+
+  ASSERT_TRUE(service->destroyAppDataSnapshot(std::make_unique<std::string>("TEST"),
+          "com.foo", 0, ce_snapshot_inode, FLAG_STORAGE_DE | FLAG_STORAGE_CE).isOk());
+  // Check snapshot is deleted.
+  ASSERT_EQ(-1, stat((rollback_ce_dir + "/com.foo").c_str(), &sb));
+  ASSERT_EQ(-1, stat((rollback_de_dir + "/com.foo").c_str(), &sb));
+}
+
+TEST_F(ServiceTest, DestroyAppDataSnapshot_CeSnapshotInodeIsZero) {
+  auto rollback_ce_dir = create_data_misc_ce_rollback_path("TEST", 0);
+  auto rollback_de_dir = create_data_misc_de_rollback_path("TEST", 0);
+
+  ASSERT_TRUE(mkdirs(rollback_ce_dir, 700));
+  ASSERT_TRUE(mkdirs(rollback_de_dir, 700));
+
+  auto fake_package_ce_path = create_data_user_ce_package_path("TEST", 0, "com.foo");
+  auto fake_package_de_path = create_data_user_de_package_path("TEST", 0, "com.foo");
+
+  ASSERT_TRUE(mkdirs(fake_package_ce_path, 700));
+  ASSERT_TRUE(mkdirs(fake_package_de_path, 700));
+
+  auto deleter = [&rollback_ce_dir, &rollback_de_dir,
+          &fake_package_ce_path, &fake_package_de_path]() {
+      delete_dir_contents(rollback_ce_dir, true);
+      delete_dir_contents(rollback_de_dir, true);
+      delete_dir_contents(fake_package_ce_path, true);
+      delete_dir_contents(fake_package_de_path, true);
+      rmdir(rollback_ce_dir.c_str());
+      rmdir(rollback_de_dir.c_str());
+  };
+  auto scope_guard = android::base::make_scope_guard(deleter);
+
+  // Create a snapshot
+  ASSERT_TRUE(mkdirs(rollback_ce_dir + "/com.foo/", 700));
+  ASSERT_TRUE(mkdirs(rollback_de_dir + "/com.foo/", 700));
+  ASSERT_TRUE(android::base::WriteStringToFile(
+          "CE_RESTORE_CONTENT", rollback_ce_dir + "/com.foo/file1",
+          0700, 10000, 20000, false /* follow_symlinks */));
+  ASSERT_TRUE(android::base::WriteStringToFile(
+          "DE_RESTORE_CONTENT", rollback_de_dir + "/com.foo/file1",
+          0700, 10000, 20000, false /* follow_symlinks */));
+
+  ASSERT_TRUE(service->destroyAppDataSnapshot(std::make_unique<std::string>("TEST"),
+          "com.foo", 0, 0, FLAG_STORAGE_DE | FLAG_STORAGE_CE).isOk());
+
+  // Check snapshot is deleted.
+  struct stat sb;
+  ASSERT_EQ(-1, stat((rollback_ce_dir + "/com.foo").c_str(), &sb));
+  ASSERT_EQ(-1, stat((rollback_de_dir + "/com.foo").c_str(), &sb));
+
+  // Check that deleting already deleted snapshot is no-op.
+  ASSERT_TRUE(service->destroyAppDataSnapshot(std::make_unique<std::string>("TEST"),
+          "com.foo", 0, 0, FLAG_STORAGE_DE | FLAG_STORAGE_CE).isOk());
+}
+
+TEST_F(ServiceTest, DestroyAppDataSnapshot_WrongVolumeUuid) {
+  // Setup rollback data to make sure that test fails due to wrong volumeUuid
+  // being passed, not because of some other reason.
+  auto rollback_ce_dir = create_data_misc_ce_rollback_path("TEST", 0);
+  auto rollback_de_dir = create_data_misc_de_rollback_path("TEST", 0);
+
+  ASSERT_TRUE(mkdirs(rollback_ce_dir, 700));
+  ASSERT_TRUE(mkdirs(rollback_de_dir, 700));
+
+  auto fake_package_ce_path = create_data_user_ce_package_path("TEST", 0, "com.foo");
+  auto fake_package_de_path = create_data_user_de_package_path("TEST", 0, "com.foo");
+
+  ASSERT_TRUE(mkdirs(fake_package_ce_path, 700));
+  ASSERT_TRUE(mkdirs(fake_package_de_path, 700));
+
+  auto deleter = [&rollback_ce_dir, &rollback_de_dir,
+          &fake_package_ce_path, &fake_package_de_path]() {
+      delete_dir_contents(rollback_ce_dir, true);
+      delete_dir_contents(rollback_de_dir, true);
+      delete_dir_contents(fake_package_ce_path, true);
+      delete_dir_contents(fake_package_de_path, true);
+      rmdir(rollback_ce_dir.c_str());
+      rmdir(rollback_de_dir.c_str());
+  };
+  auto scope_guard = android::base::make_scope_guard(deleter);
+
+  ASSERT_FALSE(service->destroyAppDataSnapshot(std::make_unique<std::string>("BAR"),
+          "com.foo", 0, 0, FLAG_STORAGE_DE).isOk());
+}
 
 TEST_F(ServiceTest, RestoreAppDataSnapshot_WrongVolumeUuid) {
   // Setup rollback data to make sure that fails due to wrong volumeUuid being
diff --git a/cmds/installd/tests/installd_utils_test.cpp b/cmds/installd/tests/installd_utils_test.cpp
index ce99fff..1782aa2 100644
--- a/cmds/installd/tests/installd_utils_test.cpp
+++ b/cmds/installd/tests/installd_utils_test.cpp
@@ -18,6 +18,7 @@
 #include <string.h>
 
 #include <android-base/logging.h>
+#include <android-base/scopeguard.h>
 #include <gtest/gtest.h>
 
 #include "InstalldNativeService.h"
@@ -565,6 +566,29 @@
     EXPECT_EQ("/data/misc_de/10/rollback",
             create_data_misc_de_rollback_path(nullptr, 10));
 
+    EXPECT_EQ("/data/misc_ce/0/rollback/com.foo",
+            create_data_misc_ce_rollback_package_path(nullptr, 0, "com.foo", 0));
+    EXPECT_EQ("/data/misc_ce/0/rollback/com.foo",
+            create_data_misc_ce_rollback_package_path(nullptr, 0, "com.foo", 239));
+
+    auto rollback_ce_package_path = create_data_misc_ce_rollback_package_path(nullptr, 0, "com.foo");
+    auto deleter = [&rollback_ce_package_path]() {
+        delete_dir_contents_and_dir(rollback_ce_package_path, true /* ignore_if_missing */);
+    };
+    auto scope_guard = android::base::make_scope_guard(deleter);
+
+    ASSERT_NE(-1, mkdir(rollback_ce_package_path.c_str(), 700));
+
+    ino_t ce_data_inode;
+    ASSERT_EQ(0, get_path_inode(rollback_ce_package_path, &ce_data_inode));
+
+    EXPECT_EQ("/data/misc_ce/0/rollback/com.foo",
+            create_data_misc_ce_rollback_package_path(nullptr, 0, "com.foo", ce_data_inode));
+    // Check that path defined by inode is picked even if it's not the same as
+    // the fallback one.
+    EXPECT_EQ("/data/misc_ce/0/rollback/com.foo",
+            create_data_misc_ce_rollback_package_path(nullptr, 0, "com.bar", ce_data_inode));
+
     // These last couple of cases are never exercised in production because we
     // only snapshot apps in the primary data partition. Exercise them here for
     // the sake of completeness.
diff --git a/cmds/installd/utils.cpp b/cmds/installd/utils.cpp
index 24f5eab..5b487bb 100644
--- a/cmds/installd/utils.cpp
+++ b/cmds/installd/utils.cpp
@@ -70,6 +70,35 @@
     CHECK(is_valid_package_name(package_name));
 }
 
+static std::string resolve_ce_path_by_inode_or_fallback(const std::string& root_path,
+        ino_t ce_data_inode, const std::string& fallback) {
+    if (ce_data_inode != 0) {
+        DIR* dir = opendir(root_path.c_str());
+        if (dir == nullptr) {
+            PLOG(ERROR) << "Failed to opendir " << root_path;
+            return fallback;
+        }
+
+        struct dirent* ent;
+        while ((ent = readdir(dir))) {
+            if (ent->d_ino == ce_data_inode) {
+                auto resolved = StringPrintf("%s/%s", root_path.c_str(), ent->d_name);
+                if (resolved != fallback) {
+                    LOG(DEBUG) << "Resolved path " << resolved << " for inode " << ce_data_inode
+                            << " instead of " << fallback;
+                }
+                closedir(dir);
+                return resolved;
+            }
+        }
+        LOG(WARNING) << "Failed to resolve inode " << ce_data_inode << "; using " << fallback;
+        closedir(dir);
+        return fallback;
+    } else {
+        return fallback;
+    }
+}
+
 /**
  * Create the path name where package app contents should be stored for
  * the given volume UUID and package name.  An empty UUID is assumed to
@@ -113,34 +142,8 @@
     // For testing purposes, rely on the inode when defined; this could be
     // optimized to use access() in the future.
     auto fallback = create_data_user_ce_package_path(volume_uuid, user, package_name);
-    if (ce_data_inode != 0) {
-        auto user_path = create_data_user_ce_path(volume_uuid, user);
-        DIR* dir = opendir(user_path.c_str());
-        if (dir == nullptr) {
-            PLOG(ERROR) << "Failed to opendir " << user_path;
-            return fallback;
-        }
-
-        struct dirent* ent;
-        while ((ent = readdir(dir))) {
-            if (ent->d_ino == ce_data_inode) {
-                auto resolved = StringPrintf("%s/%s", user_path.c_str(), ent->d_name);
-#if DEBUG_XATTRS
-                if (resolved != fallback) {
-                    LOG(DEBUG) << "Resolved path " << resolved << " for inode " << ce_data_inode
-                            << " instead of " << fallback;
-                }
-#endif
-                closedir(dir);
-                return resolved;
-            }
-        }
-        LOG(WARNING) << "Failed to resolve inode " << ce_data_inode << "; using " << fallback;
-        closedir(dir);
-        return fallback;
-    } else {
-        return fallback;
-    }
+    auto user_path = create_data_user_ce_path(volume_uuid, user);
+    return resolve_ce_path_by_inode_or_fallback(user_path, ce_data_inode, fallback);
 }
 
 std::string create_data_user_de_package_path(const char* volume_uuid,
@@ -209,6 +212,13 @@
            create_data_misc_ce_rollback_path(volume_uuid, user).c_str(), package_name);
 }
 
+std::string create_data_misc_ce_rollback_package_path(const char* volume_uuid,
+        userid_t user, const char* package_name, ino_t ce_rollback_inode) {
+    auto fallback = create_data_misc_ce_rollback_package_path(volume_uuid, user, package_name);
+    auto user_path = create_data_misc_ce_rollback_path(volume_uuid, user);
+    return resolve_ce_path_by_inode_or_fallback(user_path, ce_rollback_inode, fallback);
+}
+
 std::string create_data_misc_de_rollback_package_path(const char* volume_uuid,
         userid_t user, const char* package_name) {
     return StringPrintf("%s/%s",
diff --git a/cmds/installd/utils.h b/cmds/installd/utils.h
index 430f515..5d3a97d 100644
--- a/cmds/installd/utils.h
+++ b/cmds/installd/utils.h
@@ -65,6 +65,8 @@
 std::string create_data_misc_de_rollback_path(const char* volume_uuid, userid_t user);
 std::string create_data_misc_ce_rollback_package_path(const char* volume_uuid,
         userid_t user, const char* package_name);
+std::string create_data_misc_ce_rollback_package_path(const char* volume_uuid,
+        userid_t user, const char* package_name, ino_t ce_rollback_inode);
 std::string create_data_misc_de_rollback_package_path(const char* volume_uuid,
         userid_t user, const char* package_name);