Merge changes I227d8e0c,Ided4415a into rvc-dev am: f3d4495df9 am: c95191f0a1 am: ebd109b081

Change-Id: I11490e03b647509af9870f79b57fc52d44cc209a

core/java/android/os/incremental/IncrementalManager.java

@@ -32,8 +32,12 @@ import java.io.File;
 import java.io.IOException;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
+import java.nio.file.FileVisitResult;
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.nio.file.SimpleFileVisitor;
+import java.nio.file.attribute.BasicFileAttributes;
 
 /**
  * Provides operations to open or create an IncrementalStorage, using IIncrementalService
@@ -175,25 +179,6 @@ public final class IncrementalManager {
         }
     }
 
-    /**
-     * Iterates through path parents to find the base dir of an Incremental Storage.
-     *
-     * @param file Target file to search storage for.
-     * @return Absolute path which is a bind-mount point of Incremental File System.
-     */
-    @Nullable
-    private Path getStoragePathForFile(File file) {
-        File currentPath = new File(file.getParent());
-        while (currentPath.getParent() != null) {
-            IncrementalStorage storage = openStorage(currentPath.getAbsolutePath());
-            if (storage != null) {
-                return currentPath.toPath();
-            }
-            currentPath = new File(currentPath.getParent());
-        }
-        return null;
-    }
-
     /**
      * Set up an app's code path. The expected outcome of this method is:
      * 1) The actual apk directory under /data/incremental is bind-mounted to the parent directory
@@ -212,29 +197,27 @@ public final class IncrementalManager {
      */
     public void renameCodePath(File beforeCodeFile, File afterCodeFile)
             throws IllegalArgumentException, IOException {
-        final String beforeCodePath = beforeCodeFile.getAbsolutePath();
+        final File beforeCodeAbsolute = beforeCodeFile.getAbsoluteFile();
-        final String afterCodePathParent = afterCodeFile.getParentFile().getAbsolutePath();
+        final IncrementalStorage apkStorage = openStorage(beforeCodeAbsolute.toString());
-        if (!isIncrementalPath(beforeCodePath)) {
-            throw new IllegalArgumentException("Not an Incremental path: " + beforeCodePath);
-        }
-        final String afterCodePathName = afterCodeFile.getName();
-        final Path apkStoragePath = Paths.get(beforeCodePath);
-        if (apkStoragePath == null || apkStoragePath.toAbsolutePath() == null) {
-            throw new IOException("Invalid source storage path for: " + beforeCodePath);
-        }
-        final IncrementalStorage apkStorage =
-                openStorage(apkStoragePath.toAbsolutePath().toString());
         if (apkStorage == null) {
-            throw new IOException("Failed to retrieve storage from Incremental Service.");
+            throw new IllegalArgumentException("Not an Incremental path: " + beforeCodeAbsolute);
         }
-        final IncrementalStorage linkedApkStorage = createStorage(afterCodePathParent, apkStorage,
+        final String targetStorageDir = afterCodeFile.getAbsoluteFile().getParent();
+        final IncrementalStorage linkedApkStorage =
+                createStorage(targetStorageDir, apkStorage,
                         IncrementalManager.CREATE_MODE_CREATE
                                 | IncrementalManager.CREATE_MODE_PERMANENT_BIND);
         if (linkedApkStorage == null) {
-            throw new IOException("Failed to create linked storage at dir: " + afterCodePathParent);
+            throw new IOException("Failed to create linked storage at dir: " + targetStorageDir);
+        }
+        try {
+            final String afterCodePathName = afterCodeFile.getName();
+            linkFiles(apkStorage, beforeCodeAbsolute, "", linkedApkStorage, afterCodePathName);
+            apkStorage.unBind(beforeCodeAbsolute.toString());
+        } catch (Exception e) {
+            linkedApkStorage.unBind(targetStorageDir);
+            throw e;
         }
-        linkFiles(apkStorage, beforeCodeFile, "", linkedApkStorage, afterCodePathName);
-        apkStorage.unBind(beforeCodePath);
     }
 
     /**
@@ -252,22 +235,27 @@ public final class IncrementalManager {
     private void linkFiles(IncrementalStorage sourceStorage, File sourceAbsolutePath,
             String sourceRelativePath, IncrementalStorage targetStorage,
             String targetRelativePath) throws IOException {
-        targetStorage.makeDirectory(targetRelativePath);
+        final Path sourceBase = sourceAbsolutePath.toPath().resolve(sourceRelativePath);
-        final File[] entryList = sourceAbsolutePath.listFiles();
+        final Path targetRelative = Paths.get(targetRelativePath);
-        for (int i = 0; i < entryList.length; i++) {
+        Files.walkFileTree(sourceAbsolutePath.toPath(), new SimpleFileVisitor<Path>() {
-            final File entry = entryList[i];
+            @Override
-            final String entryName = entryList[i].getName();
+            public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs)
-            final String sourceEntryRelativePath =
+                    throws IOException {
-                    sourceRelativePath.isEmpty() ? entryName : sourceRelativePath + "/" + entryName;
+                final Path relativeDir = sourceBase.relativize(dir);
-            final String targetEntryRelativePath = targetRelativePath + "/" + entryName;
+                targetStorage.makeDirectory(targetRelative.resolve(relativeDir).toString());
-            if (entry.isFile()) {
+                return FileVisitResult.CONTINUE;
+            }
+
+            @Override
+            public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)
+                    throws IOException {
+                final Path relativeFile = sourceBase.relativize(file);
                 sourceStorage.makeLink(
-                        sourceEntryRelativePath, targetStorage, targetEntryRelativePath);
+                        file.toAbsolutePath().toString(), targetStorage,
-            } else if (entry.isDirectory()) {
+                        targetRelative.resolve(relativeFile).toString());
-                linkFiles(sourceStorage, entry, sourceEntryRelativePath, targetStorage,
+                return FileVisitResult.CONTINUE;
-                        targetEntryRelativePath);
-            }
             }
+        });
     }
 
     /**

services/incremental/Android.bp

@@ -62,6 +62,7 @@ filegroup {
     srcs: [
         "incremental_service.c",
         "IncrementalService.cpp",
+        "IncrementalServiceValidation.cpp",
         "BinderIncrementalService.cpp",
         "path.cpp",
         "ServiceWrappers.cpp",

services/incremental/BinderIncrementalService.cpp

@@ -18,6 +18,7 @@
 
 #include <android-base/logging.h>
 #include <android-base/no_destructor.h>
+#include <android/os/IVold.h>
 #include <binder/IResultReceiver.h>
 #include <binder/PermissionCache.h>
 #include <incfs.h>
@@ -117,11 +118,12 @@ binder::Status BinderIncrementalService::openStorage(const std::string& path,
 }
 
 binder::Status BinderIncrementalService::createStorage(
-        const std::string& path, const DataLoaderParamsParcel& params,
+        const std::string& path, const content::pm::DataLoaderParamsParcel& params,
         const ::android::sp<::android::content::pm::IDataLoaderStatusListener>& listener,
         int32_t createMode, int32_t* _aidl_return) {
     *_aidl_return =
-            mImpl.createStorage(path, const_cast<DataLoaderParamsParcel&&>(params), listener,
+            mImpl.createStorage(path, const_cast<content::pm::DataLoaderParamsParcel&&>(params),
+                                listener,
                                 android::incremental::IncrementalService::CreateOptions(
                                         createMode));
     return ok();
@@ -238,11 +240,8 @@ binder::Status BinderIncrementalService::isFileRangeLoaded(int32_t storageId,
 binder::Status BinderIncrementalService::getMetadataByPath(int32_t storageId,
                                                            const std::string& path,
                                                            std::vector<uint8_t>* _aidl_return) {
-    auto fid = mImpl.nodeFor(storageId, path);
+    auto metadata = mImpl.getMetadata(storageId, path);
-    if (fid != kIncFsInvalidFileId) {
-        auto metadata = mImpl.getMetadata(storageId, fid);
     _aidl_return->assign(metadata.begin(), metadata.end());
-    }
     return ok();
 }
 

services/incremental/IncrementalService.cpp

@@ -18,32 +18,26 @@
 
 #include "IncrementalService.h"
 
-#include <android-base/file.h>
 #include <android-base/logging.h>
 #include <android-base/no_destructor.h>
 #include <android-base/properties.h>
 #include <android-base/stringprintf.h>
-#include <android-base/strings.h>
+#include <binder/AppOpsManager.h>
-#include <android/content/pm/IDataLoaderStatusListener.h>
-#include <android/os/IVold.h>
-#include <binder/BinderService.h>
 #include <binder/Nullable.h>
-#include <binder/ParcelFileDescriptor.h>
 #include <binder/Status.h>
 #include <sys/stat.h>
 #include <uuid/uuid.h>
 
 #include <charconv>
 #include <ctime>
-#include <filesystem>
 #include <iterator>
 #include <span>
 #include <type_traits>
 
+#include "IncrementalServiceValidation.h"
 #include "Metadata.pb.h"
 
 using namespace std::literals;
-using namespace android::content::pm;
 namespace fs = std::filesystem;
 
 constexpr const char* kDataUsageStats = "android.permission.LOADER_USAGE_STATS";
@@ -51,10 +45,13 @@ constexpr const char* kOpUsage = "android:loader_usage_stats";
 
 namespace android::incremental {
 
+using content::pm::DataLoaderParamsParcel;
+using content::pm::FileSystemControlParcel;
+using content::pm::IDataLoader;
+
 namespace {
 
-using IncrementalFileSystemControlParcel =
+using IncrementalFileSystemControlParcel = os::incremental::IncrementalFileSystemControlParcel;
-        ::android::os::incremental::IncrementalFileSystemControlParcel;
 
 struct Constants {
     static constexpr auto backing = "backing_store"sv;
@@ -105,10 +102,13 @@ static std::string toMountKey(std::string_view path) {
     if (path::isAbsolute(path)) {
         path.remove_prefix(1);
     }
+    if (path.size() > 16) {
+        path = path.substr(0, 16);
+    }
     std::string res(path);
-    std::replace(res.begin(), res.end(), '/', '_');
+    std::replace_if(
-    std::replace(res.begin(), res.end(), '@', '_');
+            res.begin(), res.end(), [](char c) { return c == '/' || c == '@'; }, '_');
-    return std::string(constants().mountKeyPrefix) + res;
+    return std::string(constants().mountKeyPrefix) += res;
 }
 
 static std::pair<std::string, std::string> makeMountDir(std::string_view incrementalDir,
@@ -125,8 +125,26 @@ static std::pair<std::string, std::string> makeMountDir(std::string_view increme
     return {};
 }
 
+template <class Map>
+typename Map::const_iterator findParentPath(const Map& map, std::string_view path) {
+    const auto nextIt = map.upper_bound(path);
+    if (nextIt == map.begin()) {
+        return map.end();
+    }
+    const auto suspectIt = std::prev(nextIt);
+    if (!path::startsWith(path, suspectIt->first)) {
+        return map.end();
+    }
+    return suspectIt;
+}
+
+static base::unique_fd dup(base::borrowed_fd fd) {
+    const auto res = fcntl(fd.get(), F_DUPFD_CLOEXEC, 0);
+    return base::unique_fd(res);
+}
+
 template <class ProtoMessage, class Control>
-static ProtoMessage parseFromIncfs(const IncFsWrapper* incfs, Control&& control,
+static ProtoMessage parseFromIncfs(const IncFsWrapper* incfs, const Control& control,
                                    std::string_view path) {
     auto md = incfs->getMetadata(control, path);
     ProtoMessage message;
@@ -155,20 +173,18 @@ std::string makeBindMdName() {
 }
 } // namespace
 
-const bool IncrementalService::sEnablePerfLogging =
-        android::base::GetBoolProperty("incremental.perflogging", false);
-
 IncrementalService::IncFsMount::~IncFsMount() {
     if (dataLoaderStub) {
         dataLoaderStub->cleanupResources();
         dataLoaderStub = {};
     }
+    control.close();
     LOG(INFO) << "Unmounting and cleaning up mount " << mountId << " with root '" << root << '\'';
     for (auto&& [target, _] : bindPoints) {
-        LOG(INFO) << "\tbind: " << target;
+        LOG(INFO) << "  bind: " << target;
         incrementalService.mVold->unmountIncFs(target);
     }
-    LOG(INFO) << "\troot: " << root;
+    LOG(INFO) << "  root: " << root;
     incrementalService.mVold->unmountIncFs(path::join(root, constants().mount));
     cleanupFilesystem(root);
 }
@@ -193,8 +209,18 @@ auto IncrementalService::IncFsMount::makeStorage(StorageId id) -> StorageMap::it
     return storages.end();
 }
 
-static std::unique_ptr<DIR, decltype(&::closedir)> openDir(const char* path) {
+template <class Func>
-    return {::opendir(path), ::closedir};
+static auto makeCleanup(Func&& f) {
+    auto deleter = [f = std::move(f)](auto) { f(); };
+    return std::unique_ptr<Func, decltype(deleter)>(&f, std::move(deleter));
+}
+
+static std::unique_ptr<DIR, decltype(&::closedir)> openDir(const char* dir) {
+    return {::opendir(dir), ::closedir};
+}
+
+static auto openDir(std::string_view dir) {
+    return openDir(path::c_str(dir));
 }
 
 static int rmDirContent(const char* path) {
@@ -206,7 +232,7 @@ static int rmDirContent(const char* path) {
         if (entry->d_name == "."sv || entry->d_name == ".."sv) {
             continue;
         }
-        auto fullPath = android::base::StringPrintf("%s/%s", path, entry->d_name);
+        auto fullPath = base::StringPrintf("%s/%s", path, entry->d_name);
         if (entry->d_type == DT_DIR) {
             if (const auto err = rmDirContent(fullPath.c_str()); err != 0) {
                 PLOG(WARNING) << "Failed to delete " << fullPath << " content";
@@ -256,7 +282,8 @@ IncrementalService::IncrementalService(ServiceManagerWrapper&& sm, std::string_v
         runJobProcessing();
     });
 
-    mountExistingImages();
+    const auto mountedRootNames = adoptMountedInstances();
+    mountExistingImages(mountedRootNames);
 }
 
 IncrementalService::~IncrementalService() {
@@ -268,15 +295,7 @@ IncrementalService::~IncrementalService() {
     mJobProcessor.join();
 }
 
-inline const char* toString(TimePoint t) {
+static const char* toString(IncrementalService::BindKind kind) {
-    using SystemClock = std::chrono::system_clock;
-    time_t time = SystemClock::to_time_t(
-            SystemClock::now() +
-            std::chrono::duration_cast<SystemClock::duration>(t - Clock::now()));
-    return std::ctime(&time);
-}
-
-inline const char* toString(IncrementalService::BindKind kind) {
     switch (kind) {
         case IncrementalService::BindKind::Temporary:
             return "Temporary";
@@ -291,38 +310,48 @@ void IncrementalService::onDump(int fd) {
 
     std::unique_lock l(mLock);
 
-    dprintf(fd, "Mounts (%d):\n", int(mMounts.size()));
+    dprintf(fd, "Mounts (%d): {\n", int(mMounts.size()));
     for (auto&& [id, ifs] : mMounts) {
-        const IncFsMount& mnt = *ifs.get();
+        const IncFsMount& mnt = *ifs;
-        dprintf(fd, "\t[%d]:\n", id);
+        dprintf(fd, "  [%d]: {\n", id);
-        dprintf(fd, "\t\tmountId: %d\n", mnt.mountId);
+        if (id != mnt.mountId) {
-        dprintf(fd, "\t\troot: %s\n", mnt.root.c_str());
+            dprintf(fd, "    reference to mountId: %d\n", mnt.mountId);
-        dprintf(fd, "\t\tnextStorageDirNo: %d\n", mnt.nextStorageDirNo.load());
+        } else {
+            dprintf(fd, "    mountId: %d\n", mnt.mountId);
+            dprintf(fd, "    root: %s\n", mnt.root.c_str());
+            dprintf(fd, "    nextStorageDirNo: %d\n", mnt.nextStorageDirNo.load());
             if (mnt.dataLoaderStub) {
                 mnt.dataLoaderStub->onDump(fd);
+            } else {
+                dprintf(fd, "    dataLoader: null\n");
             }
-        dprintf(fd, "\t\tstorages (%d):\n", int(mnt.storages.size()));
+            dprintf(fd, "    storages (%d): {\n", int(mnt.storages.size()));
             for (auto&& [storageId, storage] : mnt.storages) {
-            dprintf(fd, "\t\t\t[%d] -> [%s]\n", storageId, storage.name.c_str());
+                dprintf(fd, "      [%d] -> [%s]\n", storageId, storage.name.c_str());
             }
+            dprintf(fd, "    }\n");
 
-        dprintf(fd, "\t\tbindPoints (%d):\n", int(mnt.bindPoints.size()));
+            dprintf(fd, "    bindPoints (%d): {\n", int(mnt.bindPoints.size()));
             for (auto&& [target, bind] : mnt.bindPoints) {
-            dprintf(fd, "\t\t\t[%s]->[%d]:\n", target.c_str(), bind.storage);
+                dprintf(fd, "      [%s]->[%d]:\n", target.c_str(), bind.storage);
-            dprintf(fd, "\t\t\t\tsavedFilename: %s\n", bind.savedFilename.c_str());
+                dprintf(fd, "        savedFilename: %s\n", bind.savedFilename.c_str());
-            dprintf(fd, "\t\t\t\tsourceDir: %s\n", bind.sourceDir.c_str());
+                dprintf(fd, "        sourceDir: %s\n", bind.sourceDir.c_str());
-            dprintf(fd, "\t\t\t\tkind: %s\n", toString(bind.kind));
+                dprintf(fd, "        kind: %s\n", toString(bind.kind));
             }
+            dprintf(fd, "    }\n");
         }
-
+        dprintf(fd, "  }\n");
-    dprintf(fd, "Sorted binds (%d):\n", int(mBindsByPath.size()));
+    }
+    dprintf(fd, "}\n");
+    dprintf(fd, "Sorted binds (%d): {\n", int(mBindsByPath.size()));
     for (auto&& [target, mountPairIt] : mBindsByPath) {
         const auto& bind = mountPairIt->second;
-        dprintf(fd, "\t\t[%s]->[%d]:\n", target.c_str(), bind.storage);
+        dprintf(fd, "    [%s]->[%d]:\n", target.c_str(), bind.storage);
-        dprintf(fd, "\t\t\tsavedFilename: %s\n", bind.savedFilename.c_str());
+        dprintf(fd, "      savedFilename: %s\n", bind.savedFilename.c_str());
-        dprintf(fd, "\t\t\tsourceDir: %s\n", bind.sourceDir.c_str());
+        dprintf(fd, "      sourceDir: %s\n", bind.sourceDir.c_str());
-        dprintf(fd, "\t\t\tkind: %s\n", toString(bind.kind));
+        dprintf(fd, "      kind: %s\n", toString(bind.kind));
     }
+    dprintf(fd, "}\n");
 }
 
 void IncrementalService::onSystemReady() {
@@ -471,9 +500,9 @@ StorageId IncrementalService::createStorage(
         metadata::Mount m;
         m.mutable_storage()->set_id(ifs->mountId);
         m.mutable_loader()->set_type((int)dataLoaderParams.type);
-        m.mutable_loader()->set_package_name(dataLoaderParams.packageName);
+        m.mutable_loader()->set_allocated_package_name(&dataLoaderParams.packageName);
-        m.mutable_loader()->set_class_name(dataLoaderParams.className);
+        m.mutable_loader()->set_allocated_class_name(&dataLoaderParams.className);
-        m.mutable_loader()->set_arguments(dataLoaderParams.arguments);
+        m.mutable_loader()->set_allocated_arguments(&dataLoaderParams.arguments);
         const auto metadata = m.SerializeAsString();
         m.mutable_loader()->release_arguments();
         m.mutable_loader()->release_class_name();
@@ -528,7 +557,7 @@ StorageId IncrementalService::createLinkedStorage(std::string_view mountPoint,
     }
 
     std::unique_lock l(mLock);
-    const auto& ifs = getIfsLocked(linkedStorage);
+    auto ifs = getIfsLocked(linkedStorage);
     if (!ifs) {
         LOG(ERROR) << "Ifs unavailable";
         return kInvalidStorageId;
@@ -552,6 +581,8 @@ StorageId IncrementalService::createLinkedStorage(std::string_view mountPoint,
                                 bk, l);
         err < 0) {
         LOG(ERROR) << "bindMount failed with error: " << err;
+        (void)mIncFs->unlink(ifs->control, storageIt->second.name);
+        ifs->storages.erase(storageIt);
         return kInvalidStorageId;
     }
 
@@ -561,15 +592,7 @@ StorageId IncrementalService::createLinkedStorage(std::string_view mountPoint,
 
 IncrementalService::BindPathMap::const_iterator IncrementalService::findStorageLocked(
         std::string_view path) const {
-    auto bindPointIt = mBindsByPath.upper_bound(path);
+    return findParentPath(mBindsByPath, path);
-    if (bindPointIt == mBindsByPath.begin()) {
-        return mBindsByPath.end();
-    }
-    --bindPointIt;
-    if (!path::startsWith(path, bindPointIt->first)) {
-        return mBindsByPath.end();
-    }
-    return bindPointIt;
 }
 
 StorageId IncrementalService::findStorageId(std::string_view path) const {
@@ -611,13 +634,12 @@ int IncrementalService::setStorageParams(StorageId storageId, bool enableReadLog
 }
 
 binder::Status IncrementalService::applyStorageParams(IncFsMount& ifs, bool enableReadLogs) {
-    using unique_fd = ::android::base::unique_fd;
+    os::incremental::IncrementalFileSystemControlParcel control;
-    ::android::os::incremental::IncrementalFileSystemControlParcel control;
+    control.cmd.reset(dup(ifs.control.cmd()));
-    control.cmd.reset(unique_fd(dup(ifs.control.cmd())));
+    control.pendingReads.reset(dup(ifs.control.pendingReads()));
-    control.pendingReads.reset(unique_fd(dup(ifs.control.pendingReads())));
     auto logsFd = ifs.control.logs();
     if (logsFd >= 0) {
-        control.log.reset(unique_fd(dup(logsFd)));
+        control.log.reset(dup(logsFd));
     }
 
     std::lock_guard l(mMountOperationLock);
@@ -664,38 +686,6 @@ StorageId IncrementalService::openStorage(std::string_view pathInMount) {
     return findStorageId(path::normalize(pathInMount));
 }
 
-FileId IncrementalService::nodeFor(StorageId storage, std::string_view subpath) const {
-    const auto ifs = getIfs(storage);
-    if (!ifs) {
-        return kIncFsInvalidFileId;
-    }
-    std::unique_lock l(ifs->lock);
-    auto storageIt = ifs->storages.find(storage);
-    if (storageIt == ifs->storages.end()) {
-        return kIncFsInvalidFileId;
-    }
-    if (subpath.empty() || subpath == "."sv) {
-        return kIncFsInvalidFileId;
-    }
-    auto path = path::join(ifs->root, constants().mount, storageIt->second.name, subpath);
-    l.unlock();
-    return mIncFs->getFileId(ifs->control, path);
-}
-
-std::pair<FileId, std::string_view> IncrementalService::parentAndNameFor(
-        StorageId storage, std::string_view subpath) const {
-    auto name = path::basename(subpath);
-    if (name.empty()) {
-        return {kIncFsInvalidFileId, {}};
-    }
-    auto dir = path::dirname(subpath);
-    if (dir.empty() || dir == "/"sv) {
-        return {kIncFsInvalidFileId, {}};
-    }
-    auto id = nodeFor(storage, dir);
-    return {id, name};
-}
-
 IncrementalService::IfsMountPtr IncrementalService::getIfs(StorageId storage) const {
     std::lock_guard l(mLock);
     return getIfsLocked(storage);
@@ -704,7 +694,7 @@ IncrementalService::IfsMountPtr IncrementalService::getIfs(StorageId storage) co
 const IncrementalService::IfsMountPtr& IncrementalService::getIfsLocked(StorageId storage) const {
     auto it = mMounts.find(storage);
     if (it == mMounts.end()) {
-        static const android::base::NoDestructor<IfsMountPtr> kEmpty{};
+        static const base::NoDestructor<IfsMountPtr> kEmpty{};
         return *kEmpty;
     }
     return it->second;
@@ -713,21 +703,25 @@ const IncrementalService::IfsMountPtr& IncrementalService::getIfsLocked(StorageI
 int IncrementalService::bind(StorageId storage, std::string_view source, std::string_view target,
                              BindKind kind) {
     if (!isValidMountTarget(target)) {
+        LOG(ERROR) << __func__ << ": not a valid bind target " << target;
         return -EINVAL;
     }
 
     const auto ifs = getIfs(storage);
     if (!ifs) {
+        LOG(ERROR) << __func__ << ": no ifs object for storage " << storage;
         return -EINVAL;
     }
 
     std::unique_lock l(ifs->lock);
     const auto storageInfo = ifs->storages.find(storage);
     if (storageInfo == ifs->storages.end()) {
+        LOG(ERROR) << "no storage";
         return -EINVAL;
     }
-    std::string normSource = normalizePathToStorageLocked(storageInfo, source);
+    std::string normSource = normalizePathToStorageLocked(ifs, storageInfo, source);
     if (normSource.empty()) {
+        LOG(ERROR) << "invalid source path";
         return -EINVAL;
     }
     l.unlock();
@@ -779,27 +773,32 @@ int IncrementalService::unbind(StorageId storage, std::string_view target) {
 }
 
 std::string IncrementalService::normalizePathToStorageLocked(
-        IncFsMount::StorageMap::iterator storageIt, std::string_view path) {
+        const IfsMountPtr& incfs, IncFsMount::StorageMap::iterator storageIt,
-    std::string normPath;
+        std::string_view path) const {
-    if (path::isAbsolute(path)) {
+    if (!path::isAbsolute(path)) {
-        normPath = path::normalize(path);
+        return path::normalize(path::join(storageIt->second.name, path));
-        if (!path::startsWith(normPath, storageIt->second.name)) {
+    }
+    auto normPath = path::normalize(path);
+    if (path::startsWith(normPath, storageIt->second.name)) {
+        return normPath;
+    }
+    // not that easy: need to find if any of the bind points match
+    const auto bindIt = findParentPath(incfs->bindPoints, normPath);
+    if (bindIt == incfs->bindPoints.end()) {
         return {};
     }
-    } else {
+    return path::join(bindIt->second.sourceDir, path::relativize(bindIt->first, normPath));
-        normPath = path::normalize(path::join(storageIt->second.name, path));
-    }
-    return normPath;
 }
 
 std::string IncrementalService::normalizePathToStorage(const IncrementalService::IfsMountPtr& ifs,
-                                                       StorageId storage, std::string_view path) {
+                                                       StorageId storage,
+                                                       std::string_view path) const {
     std::unique_lock l(ifs->lock);
     const auto storageInfo = ifs->storages.find(storage);
     if (storageInfo == ifs->storages.end()) {
         return {};
     }
-    return normalizePathToStorageLocked(storageInfo, path);
+    return normalizePathToStorageLocked(ifs, storageInfo, path);
 }
 
 int IncrementalService::makeFile(StorageId storage, std::string_view path, int mode, FileId id,
@@ -844,7 +843,8 @@ int IncrementalService::makeDirs(StorageId storageId, std::string_view path, int
     auto err = mIncFs->makeDir(ifs->control, normPath, mode);
     if (err == -EEXIST) {
         return 0;
-    } else if (err != -ENOENT) {
+    }
+    if (err != -ENOENT) {
         return err;
     }
     if (auto err = makeDirs(storageId, path::dirname(normPath), mode)) {
@@ -855,18 +855,23 @@ int IncrementalService::makeDirs(StorageId storageId, std::string_view path, int
 
 int IncrementalService::link(StorageId sourceStorageId, std::string_view oldPath,
                              StorageId destStorageId, std::string_view newPath) {
-    auto ifsSrc = getIfs(sourceStorageId);
+    std::unique_lock l(mLock);
-    auto ifsDest = sourceStorageId == destStorageId ? ifsSrc : getIfs(destStorageId);
+    auto ifsSrc = getIfsLocked(sourceStorageId);
-    if (ifsSrc && ifsSrc == ifsDest) {
+    if (!ifsSrc) {
+        return -EINVAL;
+    }
+    if (sourceStorageId != destStorageId && getIfsLocked(destStorageId) != ifsSrc) {
+        return -EINVAL;
+    }
+    l.unlock();
     std::string normOldPath = normalizePathToStorage(ifsSrc, sourceStorageId, oldPath);
-        std::string normNewPath = normalizePathToStorage(ifsDest, destStorageId, newPath);
+    std::string normNewPath = normalizePathToStorage(ifsSrc, destStorageId, newPath);
     if (normOldPath.empty() || normNewPath.empty()) {
+        LOG(ERROR) << "Invalid paths in link(): " << normOldPath << " | " << normNewPath;
         return -EINVAL;
     }
     return mIncFs->link(ifsSrc->control, normOldPath, normNewPath);
 }
-    return -EINVAL;
-}
 
 int IncrementalService::unlink(StorageId storage, std::string_view path) {
     if (auto ifs = getIfs(storage)) {
@@ -881,10 +886,12 @@ int IncrementalService::addBindMount(IncFsMount& ifs, StorageId storage,
                                      std::string&& target, BindKind kind,
                                      std::unique_lock<std::mutex>& mainLock) {
     if (!isValidMountTarget(target)) {
+        LOG(ERROR) << __func__ << ": invalid mount target " << target;
         return -EINVAL;
     }
 
     std::string mdFileName;
+    std::string metadataFullPath;
     if (kind != BindKind::Temporary) {
         metadata::BindPoint bp;
         bp.set_storage_id(storage);
@@ -894,17 +901,21 @@ int IncrementalService::addBindMount(IncFsMount& ifs, StorageId storage,
         bp.release_dest_path();
         bp.release_source_subdir();
         mdFileName = makeBindMdName();
-        auto node =
+        metadataFullPath = path::join(ifs.root, constants().mount, mdFileName);
-                mIncFs->makeFile(ifs.control, path::join(ifs.root, constants().mount, mdFileName),
+        auto node = mIncFs->makeFile(ifs.control, metadataFullPath, 0444, idFromMetadata(metadata),
-                                 0444, idFromMetadata(metadata),
                                      {.metadata = {metadata.data(), (IncFsSize)metadata.size()}});
         if (node) {
+            LOG(ERROR) << __func__ << ": couldn't create a mount node " << mdFileName;
             return int(node);
         }
     }
 
-    return addBindMountWithMd(ifs, storage, std::move(mdFileName), std::move(source),
+    const auto res = addBindMountWithMd(ifs, storage, std::move(mdFileName), std::move(source),
                                         std::move(target), kind, mainLock);
+    if (res) {
+        mIncFs->unlink(ifs.control, metadataFullPath);
+    }
+    return res;
 }
 
 int IncrementalService::addBindMountWithMd(IncrementalService::IncFsMount& ifs, StorageId storage,
@@ -929,12 +940,31 @@ int IncrementalService::addBindMountWithMd(IncrementalService::IncFsMount& ifs,
         mainLock.lock();
     }
     std::lock_guard l(ifs.lock);
+    addBindMountRecordLocked(ifs, storage, std::move(metadataName), std::move(source),
+                             std::move(target), kind);
+    return 0;
+}
+
+void IncrementalService::addBindMountRecordLocked(IncFsMount& ifs, StorageId storage,
+                                                  std::string&& metadataName, std::string&& source,
+                                                  std::string&& target, BindKind kind) {
     const auto [it, _] =
             ifs.bindPoints.insert_or_assign(target,
                                             IncFsMount::Bind{storage, std::move(metadataName),
                                                              std::move(source), kind});
     mBindsByPath[std::move(target)] = it;
-    return 0;
+}
+
+RawMetadata IncrementalService::getMetadata(StorageId storage, std::string_view path) const {
+    const auto ifs = getIfs(storage);
+    if (!ifs) {
+        return {};
+    }
+    const auto normPath = normalizePathToStorage(ifs, storage, path);
+    if (normPath.empty()) {
+        return {};
+    }
+    return mIncFs->getMetadata(ifs->control, normPath);
 }
 
 RawMetadata IncrementalService::getMetadata(StorageId storage, FileId node) const {
@@ -945,47 +975,6 @@ RawMetadata IncrementalService::getMetadata(StorageId storage, FileId node) cons
     return mIncFs->getMetadata(ifs->control, node);
 }
 
-std::vector<std::string> IncrementalService::listFiles(StorageId storage) const {
-    const auto ifs = getIfs(storage);
-    if (!ifs) {
-        return {};
-    }
-
-    std::unique_lock l(ifs->lock);
-    auto subdirIt = ifs->storages.find(storage);
-    if (subdirIt == ifs->storages.end()) {
-        return {};
-    }
-    auto dir = path::join(ifs->root, constants().mount, subdirIt->second.name);
-    l.unlock();
-
-    const auto prefixSize = dir.size() + 1;
-    std::vector<std::string> todoDirs{std::move(dir)};
-    std::vector<std::string> result;
-    do {
-        auto currDir = std::move(todoDirs.back());
-        todoDirs.pop_back();
-
-        auto d =
-                std::unique_ptr<DIR, decltype(&::closedir)>(::opendir(currDir.c_str()), ::closedir);
-        while (auto e = ::readdir(d.get())) {
-            if (e->d_type == DT_REG) {
-                result.emplace_back(
-                        path::join(std::string_view(currDir).substr(prefixSize), e->d_name));
-                continue;
-            }
-            if (e->d_type == DT_DIR) {
-                if (e->d_name == "."sv || e->d_name == ".."sv) {
-                    continue;
-                }
-                todoDirs.emplace_back(path::join(currDir, e->d_name));
-                continue;
-            }
-        }
-    } while (!todoDirs.empty());
-    return result;
-}
-
 bool IncrementalService::startLoading(StorageId storage) const {
     DataLoaderStubPtr dataLoaderStub;
     {
@@ -1003,16 +992,216 @@ bool IncrementalService::startLoading(StorageId storage) const {
     return true;
 }
 
-void IncrementalService::mountExistingImages() {
+std::unordered_set<std::string_view> IncrementalService::adoptMountedInstances() {
-    for (const auto& entry : fs::directory_iterator(mIncrementalDir)) {
+    std::unordered_set<std::string_view> mountedRootNames;
-        const auto path = entry.path().u8string();
+    mIncFs->listExistingMounts([this, &mountedRootNames](auto root, auto backingDir, auto binds) {
-        const auto name = entry.path().filename().u8string();
+        LOG(INFO) << "Existing mount: " << backingDir << "->" << root;
-        if (!base::StartsWith(name, constants().mountKeyPrefix)) {
+        for (auto [source, target] : binds) {
+            LOG(INFO) << "  bind: '" << source << "'->'" << target << "'";
+            LOG(INFO) << "         " << path::join(root, source);
+        }
+
+        // Ensure it's a kind of a mount that's managed by IncrementalService
+        if (path::basename(root) != constants().mount ||
+            path::basename(backingDir) != constants().backing) {
+            return;
+        }
+        const auto expectedRoot = path::dirname(root);
+        if (path::dirname(backingDir) != expectedRoot) {
+            return;
+        }
+        if (path::dirname(expectedRoot) != mIncrementalDir) {
+            return;
+        }
+        if (!path::basename(expectedRoot).starts_with(constants().mountKeyPrefix)) {
+            return;
+        }
+
+        LOG(INFO) << "Looks like an IncrementalService-owned: " << expectedRoot;
+
+        // make sure we clean up the mount if it happens to be a bad one.
+        // Note: unmounting needs to run first, so the cleanup object is created _last_.
+        auto cleanupFiles = makeCleanup([&]() {
+            LOG(INFO) << "Failed to adopt existing mount, deleting files: " << expectedRoot;
+            IncFsMount::cleanupFilesystem(expectedRoot);
+        });
+        auto cleanupMounts = makeCleanup([&]() {
+            LOG(INFO) << "Failed to adopt existing mount, cleaning up: " << expectedRoot;
+            for (auto&& [_, target] : binds) {
+                mVold->unmountIncFs(std::string(target));
+            }
+            mVold->unmountIncFs(std::string(root));
+        });
+
+        auto control = mIncFs->openMount(root);
+        if (!control) {
+            LOG(INFO) << "failed to open mount " << root;
+            return;
+        }
+
+        auto mountRecord =
+                parseFromIncfs<metadata::Mount>(mIncFs.get(), control,
+                                                path::join(root, constants().infoMdName));
+        if (!mountRecord.has_loader() || !mountRecord.has_storage()) {
+            LOG(ERROR) << "Bad mount metadata in mount at " << expectedRoot;
+            return;
+        }
+
+        auto mountId = mountRecord.storage().id();
+        mNextId = std::max(mNextId, mountId + 1);
+
+        DataLoaderParamsParcel dataLoaderParams;
+        {
+            const auto& loader = mountRecord.loader();
+            dataLoaderParams.type = (content::pm::DataLoaderType)loader.type();
+            dataLoaderParams.packageName = loader.package_name();
+            dataLoaderParams.className = loader.class_name();
+            dataLoaderParams.arguments = loader.arguments();
+        }
+
+        auto ifs = std::make_shared<IncFsMount>(std::string(expectedRoot), mountId,
+                                                std::move(control), *this);
+        cleanupFiles.release(); // ifs will take care of that now
+
+        std::vector<std::pair<std::string, metadata::BindPoint>> permanentBindPoints;
+        auto d = openDir(root);
+        while (auto e = ::readdir(d.get())) {
+            if (e->d_type == DT_REG) {
+                auto name = std::string_view(e->d_name);
+                if (name.starts_with(constants().mountpointMdPrefix)) {
+                    permanentBindPoints
+                            .emplace_back(name,
+                                          parseFromIncfs<metadata::BindPoint>(mIncFs.get(),
+                                                                              ifs->control,
+                                                                              path::join(root,
+                                                                                         name)));
+                    if (permanentBindPoints.back().second.dest_path().empty() ||
+                        permanentBindPoints.back().second.source_subdir().empty()) {
+                        permanentBindPoints.pop_back();
+                        mIncFs->unlink(ifs->control, path::join(root, name));
+                    } else {
+                        LOG(INFO) << "Permanent bind record: '"
+                                  << permanentBindPoints.back().second.source_subdir() << "'->'"
+                                  << permanentBindPoints.back().second.dest_path() << "'";
+                    }
+                }
+            } else if (e->d_type == DT_DIR) {
+                if (e->d_name == "."sv || e->d_name == ".."sv) {
+                    continue;
+                }
+                auto name = std::string_view(e->d_name);
+                if (name.starts_with(constants().storagePrefix)) {
+                    int storageId;
+                    const auto res =
+                            std::from_chars(name.data() + constants().storagePrefix.size() + 1,
+                                            name.data() + name.size(), storageId);
+                    if (res.ec != std::errc{} || *res.ptr != '_') {
+                        LOG(WARNING) << "Ignoring storage with invalid name '" << name
+                                     << "' for mount " << expectedRoot;
+                        continue;
+                    }
+                    auto [_, inserted] = mMounts.try_emplace(storageId, ifs);
+                    if (!inserted) {
+                        LOG(WARNING) << "Ignoring storage with duplicate id " << storageId
+                                     << " for mount " << expectedRoot;
+                        continue;
+                    }
+                    ifs->storages.insert_or_assign(storageId,
+                                                   IncFsMount::Storage{path::join(root, name)});
+                    mNextId = std::max(mNextId, storageId + 1);
+                }
+            }
+        }
+
+        if (ifs->storages.empty()) {
+            LOG(WARNING) << "No valid storages in mount " << root;
+            return;
+        }
+
+        // now match the mounted directories with what we expect to have in the metadata
+        {
+            std::unique_lock l(mLock, std::defer_lock);
+            for (auto&& [metadataFile, bindRecord] : permanentBindPoints) {
+                auto mountedIt = std::find_if(binds.begin(), binds.end(),
+                                              [&, bindRecord = bindRecord](auto&& bind) {
+                                                  return bind.second == bindRecord.dest_path() &&
+                                                          path::join(root, bind.first) ==
+                                                          bindRecord.source_subdir();
+                                              });
+                if (mountedIt != binds.end()) {
+                    LOG(INFO) << "Matched permanent bound " << bindRecord.source_subdir()
+                              << " to mount " << mountedIt->first;
+                    addBindMountRecordLocked(*ifs, bindRecord.storage_id(), std::move(metadataFile),
+                                             std::move(*bindRecord.mutable_source_subdir()),
+                                             std::move(*bindRecord.mutable_dest_path()),
+                                             BindKind::Permanent);
+                    if (mountedIt != binds.end() - 1) {
+                        std::iter_swap(mountedIt, binds.end() - 1);
+                    }
+                    binds = binds.first(binds.size() - 1);
+                } else {
+                    LOG(INFO) << "Didn't match permanent bound " << bindRecord.source_subdir()
+                              << ", mounting";
+                    // doesn't exist - try mounting back
+                    if (addBindMountWithMd(*ifs, bindRecord.storage_id(), std::move(metadataFile),
+                                           std::move(*bindRecord.mutable_source_subdir()),
+                                           std::move(*bindRecord.mutable_dest_path()),
+                                           BindKind::Permanent, l)) {
+                        mIncFs->unlink(ifs->control, metadataFile);
+                    }
+                }
+            }
+        }
+
+        // if anything stays in |binds| those are probably temporary binds; system restarted since
+        // they were mounted - so let's unmount them all.
+        for (auto&& [source, target] : binds) {
+            if (source.empty()) {
+                continue;
+            }
+            mVold->unmountIncFs(std::string(target));
+        }
+        cleanupMounts.release(); // ifs now manages everything
+
+        if (ifs->bindPoints.empty()) {
+            LOG(WARNING) << "No valid bind points for mount " << expectedRoot;
+            deleteStorage(*ifs);
+            return;
+        }
+
+        prepareDataLoaderLocked(*ifs, std::move(dataLoaderParams));
+        CHECK(ifs->dataLoaderStub);
+
+        mountedRootNames.insert(path::basename(ifs->root));
+
+        // not locking here at all: we're still in the constructor, no other calls can happen
+        mMounts[ifs->mountId] = std::move(ifs);
+    });
+
+    return mountedRootNames;
+}
+
+void IncrementalService::mountExistingImages(
+        const std::unordered_set<std::string_view>& mountedRootNames) {
+    auto dir = openDir(mIncrementalDir);
+    if (!dir) {
+        PLOG(WARNING) << "Couldn't open the root incremental dir " << mIncrementalDir;
+        return;
+    }
+    while (auto entry = ::readdir(dir.get())) {
+        if (entry->d_type != DT_DIR) {
+            continue;
+        }
+        std::string_view name = entry->d_name;
+        if (!name.starts_with(constants().mountKeyPrefix)) {
+            continue;
+        }
+        if (mountedRootNames.find(name) != mountedRootNames.end()) {
             continue;
         }
         const auto root = path::join(mIncrementalDir, name);
         if (!mountExistingImage(root)) {
-            IncFsMount::cleanupFilesystem(path);
+            IncFsMount::cleanupFilesystem(root);
         }
     }
 }
@@ -1049,7 +1238,7 @@ bool IncrementalService::mountExistingImage(std::string_view root) {
     DataLoaderParamsParcel dataLoaderParams;
     {
         const auto& loader = mount.loader();
-        dataLoaderParams.type = (android::content::pm::DataLoaderType)loader.type();
+        dataLoaderParams.type = (content::pm::DataLoaderType)loader.type();
         dataLoaderParams.packageName = loader.package_name();
         dataLoaderParams.className = loader.class_name();
         dataLoaderParams.arguments = loader.arguments();
@@ -1059,7 +1248,7 @@ bool IncrementalService::mountExistingImage(std::string_view root) {
     CHECK(ifs->dataLoaderStub);
 
     std::vector<std::pair<std::string, metadata::BindPoint>> bindPoints;
-    auto d = openDir(path::c_str(mountTarget));
+    auto d = openDir(mountTarget);
     while (auto e = ::readdir(d.get())) {
         if (e->d_type == DT_REG) {
             auto name = std::string_view(e->d_name);
@@ -1109,13 +1298,15 @@ bool IncrementalService::mountExistingImage(std::string_view root) {
     }
 
     int bindCount = 0;
-    for (auto&& bp : bindPoints) {
+    {
         std::unique_lock l(mLock, std::defer_lock);
+        for (auto&& bp : bindPoints) {
             bindCount += !addBindMountWithMd(*ifs, bp.second.storage_id(), std::move(bp.first),
                                              std::move(*bp.second.mutable_source_subdir()),
                                              std::move(*bp.second.mutable_dest_path()),
                                              BindKind::Permanent, l);
         }
+    }
 
     if (bindCount == 0) {
         LOG(WARNING) << "No valid bind points for mount " << root;
@@ -1123,30 +1314,35 @@ bool IncrementalService::mountExistingImage(std::string_view root) {
         return false;
     }
 
+    // not locking here at all: we're still in the constructor, no other calls can happen
     mMounts[ifs->mountId] = std::move(ifs);
     return true;
 }
 
 IncrementalService::DataLoaderStubPtr IncrementalService::prepareDataLoader(
-        IncrementalService::IncFsMount& ifs, DataLoaderParamsParcel&& params,
+        IncFsMount& ifs, DataLoaderParamsParcel&& params,
         const DataLoaderStatusListener* externalListener) {
     std::unique_lock l(ifs.lock);
+    prepareDataLoaderLocked(ifs, std::move(params), externalListener);
+    return ifs.dataLoaderStub;
+}
+
+void IncrementalService::prepareDataLoaderLocked(IncFsMount& ifs, DataLoaderParamsParcel&& params,
+                                                 const DataLoaderStatusListener* externalListener) {
     if (ifs.dataLoaderStub) {
         LOG(INFO) << "Skipped data loader preparation because it already exists";
-        return ifs.dataLoaderStub;
+        return;
     }
 
     FileSystemControlParcel fsControlParcel;
     fsControlParcel.incremental = aidl::make_nullable<IncrementalFileSystemControlParcel>();
-    fsControlParcel.incremental->cmd.reset(base::unique_fd(::dup(ifs.control.cmd())));
+    fsControlParcel.incremental->cmd.reset(dup(ifs.control.cmd()));
-    fsControlParcel.incremental->pendingReads.reset(
+    fsControlParcel.incremental->pendingReads.reset(dup(ifs.control.pendingReads()));
-            base::unique_fd(::dup(ifs.control.pendingReads())));
+    fsControlParcel.incremental->log.reset(dup(ifs.control.logs()));
-    fsControlParcel.incremental->log.reset(base::unique_fd(::dup(ifs.control.logs())));
     fsControlParcel.service = new IncrementalServiceConnector(*this, ifs.mountId);
 
     ifs.dataLoaderStub = new DataLoaderStub(*this, ifs.mountId, std::move(params),
                                             std::move(fsControlParcel), externalListener);
-    return ifs.dataLoaderStub;
 }
 
 template <class Duration>
@@ -1208,7 +1404,7 @@ bool IncrementalService::configureNativeBinaries(StorageId storage, std::string_
         const auto targetLibPathAbsolute = normalizePathToStorage(ifs, storage, targetLibPath);
         // If the extract file already exists, skip
         if (access(targetLibPathAbsolute.c_str(), F_OK) == 0) {
-            if (sEnablePerfLogging) {
+            if (perfLoggingEnabled()) {
                 LOG(INFO) << "incfs: Native lib file already exists: " << targetLibPath
                           << "; skipping extraction, spent "
                           << elapsedMcs(startFileTs, Clock::now()) << "mcs";
@@ -1235,7 +1431,7 @@ bool IncrementalService::configureNativeBinaries(StorageId storage, std::string_
 
         // If it is a zero-byte file, skip data writing
         if (entry.uncompressed_length == 0) {
-            if (sEnablePerfLogging) {
+            if (perfLoggingEnabled()) {
                 LOG(INFO) << "incfs: Extracted " << libName
                           << "(0 bytes): " << elapsedMcs(startFileTs, makeFileTs) << "mcs";
             }
@@ -1248,7 +1444,7 @@ bool IncrementalService::configureNativeBinaries(StorageId storage, std::string_
             extractZipFile(ifs.lock(), zipFile.get(), entry, libFileId, libPath, makeFileTs);
         });
 
-        if (sEnablePerfLogging) {
+        if (perfLoggingEnabled()) {
             auto prepareJobTs = Clock::now();
             LOG(INFO) << "incfs: Processed " << libName << ": "
                       << elapsedMcs(startFileTs, prepareJobTs)
@@ -1275,7 +1471,7 @@ bool IncrementalService::configureNativeBinaries(StorageId storage, std::string_
         mJobCondition.notify_all();
     }
 
-    if (sEnablePerfLogging) {
+    if (perfLoggingEnabled()) {
         auto end = Clock::now();
         LOG(INFO) << "incfs: configureNativeBinaries complete in " << elapsedMcs(start, end)
                   << "mcs, make dirs: " << elapsedMcs(start, mkDirsTs)
@@ -1340,7 +1536,7 @@ void IncrementalService::extractZipFile(const IfsMountPtr& ifs, ZipArchiveHandle
         return;
     }
 
-    if (sEnablePerfLogging) {
+    if (perfLoggingEnabled()) {
         auto endFileTs = Clock::now();
         LOG(INFO) << "incfs: Extracted " << libName << "(" << entry.compressed_length << " -> "
                   << entry.uncompressed_length << " bytes): " << elapsedMcs(startedTs, endFileTs)
@@ -1356,7 +1552,7 @@ bool IncrementalService::waitForNativeBinariesExtraction(StorageId storage) {
     struct WaitPrinter {
         const Clock::time_point startTs = Clock::now();
         ~WaitPrinter() noexcept {
-            if (sEnablePerfLogging) {
+            if (perfLoggingEnabled()) {
                 const auto endTs = Clock::now();
                 LOG(INFO) << "incfs: waitForNativeBinariesExtraction() complete in "
                           << elapsedMcs(startTs, endTs) << "mcs";
@@ -1381,6 +1577,11 @@ bool IncrementalService::waitForNativeBinariesExtraction(StorageId storage) {
     return mRunning;
 }
 
+bool IncrementalService::perfLoggingEnabled() {
+    static const bool enabled = base::GetBoolProperty("incremental.perflogging", false);
+    return enabled;
+}
+
 void IncrementalService::runJobProcessing() {
     for (;;) {
         std::unique_lock lock(mJobMutex);
@@ -1492,12 +1693,17 @@ bool IncrementalService::DataLoaderStub::requestDestroy() {
     return setTargetStatus(IDataLoaderStatusListener::DATA_LOADER_DESTROYED);
 }
 
-bool IncrementalService::DataLoaderStub::setTargetStatus(int status) {
+bool IncrementalService::DataLoaderStub::setTargetStatus(int newStatus) {
+    int oldStatus, curStatus;
     {
         std::unique_lock lock(mStatusMutex);
-        mTargetStatus = status;
+        oldStatus = mTargetStatus;
+        mTargetStatus = newStatus;
         mTargetStatusTs = Clock::now();
+        curStatus = mCurrentStatus;
     }
+    LOG(DEBUG) << "Target status update for DataLoader " << mId << ": " << oldStatus << " -> "
+               << newStatus << " (current " << curStatus << ")";
     return fsmStep();
 }
 
@@ -1596,14 +1802,20 @@ binder::Status IncrementalService::DataLoaderStub::onStatusChanged(MountId mount
         return binder::Status::fromServiceSpecificError(-EPERM, "Mount ID mismatch.");
     }
 
+    int targetStatus, oldStatus;
     {
         std::unique_lock lock(mStatusMutex);
         if (mCurrentStatus == newStatus) {
             return binder::Status::ok();
         }
         mCurrentStatus = newStatus;
+        oldStatus = mCurrentStatus;
+        targetStatus = mTargetStatus;
     }
 
+    LOG(DEBUG) << "Current status update for DataLoader " << mId << ": " << oldStatus << " -> "
+               << newStatus << " (target " << targetStatus << ")";
+
     if (mListener) {
         mListener->onStatusChanged(mountId, newStatus);
     }
@@ -1616,17 +1828,19 @@ binder::Status IncrementalService::DataLoaderStub::onStatusChanged(MountId mount
 }
 
 void IncrementalService::DataLoaderStub::onDump(int fd) {
-    dprintf(fd, "\t\tdataLoader:");
+    dprintf(fd, "    dataLoader: {\n");
-    dprintf(fd, "\t\t\tcurrentStatus: %d\n", mCurrentStatus);
+    dprintf(fd, "      currentStatus: %d\n", mCurrentStatus);
-    dprintf(fd, "\t\t\ttargetStatus: %d\n", mTargetStatus);
+    dprintf(fd, "      targetStatus: %d\n", mTargetStatus);
-    dprintf(fd, "\t\t\ttargetStatusTs: %lldmcs\n",
+    dprintf(fd, "      targetStatusTs: %lldmcs\n",
             (long long)(elapsedMcs(mTargetStatusTs, Clock::now())));
     const auto& params = mParams;
-    dprintf(fd, "\t\t\tdataLoaderParams:\n");
+    dprintf(fd, "      dataLoaderParams: {\n");
-    dprintf(fd, "\t\t\t\ttype: %s\n", toString(params.type).c_str());
+    dprintf(fd, "        type: %s\n", toString(params.type).c_str());
-    dprintf(fd, "\t\t\t\tpackageName: %s\n", params.packageName.c_str());
+    dprintf(fd, "        packageName: %s\n", params.packageName.c_str());
-    dprintf(fd, "\t\t\t\tclassName: %s\n", params.className.c_str());
+    dprintf(fd, "        className: %s\n", params.className.c_str());
-    dprintf(fd, "\t\t\t\targuments: %s\n", params.arguments.c_str());
+    dprintf(fd, "        arguments: %s\n", params.arguments.c_str());
+    dprintf(fd, "      }\n");
+    dprintf(fd, "    }\n");
 }
 
 void IncrementalService::AppOpsListener::opChanged(int32_t, const String16&) {

services/incremental/IncrementalService.h

@@ -16,13 +16,13 @@
 
 #pragma once
 
-#include <android-base/strings.h>
+#include <android/content/pm/BnDataLoaderStatusListener.h>
-#include <android-base/unique_fd.h>
 #include <android/content/pm/DataLoaderParamsParcel.h>
-#include <binder/IServiceManager.h>
+#include <android/content/pm/IDataLoaderStatusListener.h>
+#include <android/os/incremental/BnIncrementalServiceConnector.h>
+#include <binder/IAppOpsCallback.h>
 #include <utils/String16.h>
 #include <utils/StrongPointer.h>
-#include <utils/Vector.h>
 #include <ziparchive/zip_archive.h>
 
 #include <atomic>
@@ -37,21 +37,14 @@
 #include <string_view>
 #include <thread>
 #include <unordered_map>
+#include <unordered_set>
 #include <utility>
 #include <vector>
 
 #include "ServiceWrappers.h"
-#include "android/content/pm/BnDataLoaderStatusListener.h"
-#include "android/os/incremental/BnIncrementalServiceConnector.h"
 #include "incfs.h"
 #include "path.h"
 
-using namespace android::os::incremental;
-
-namespace android::os {
-class IVold;
-}
-
 namespace android::incremental {
 
 using MountId = int;
@@ -101,17 +94,14 @@ public:
 
     void onSystemReady();
 
-    StorageId createStorage(std::string_view mountPoint, DataLoaderParamsParcel&& dataLoaderParams,
+    StorageId createStorage(std::string_view mountPoint,
+                            content::pm::DataLoaderParamsParcel&& dataLoaderParams,
                             const DataLoaderStatusListener& dataLoaderStatusListener,
                             CreateOptions options = CreateOptions::Default);
     StorageId createLinkedStorage(std::string_view mountPoint, StorageId linkedStorage,
                                   CreateOptions options = CreateOptions::Default);
     StorageId openStorage(std::string_view path);
 
-    FileId nodeFor(StorageId storage, std::string_view path) const;
-    std::pair<FileId, std::string_view> parentAndNameFor(StorageId storage,
-                                                         std::string_view path) const;
-
     int bind(StorageId storage, std::string_view source, std::string_view target, BindKind kind);
     int unbind(StorageId storage, std::string_view target);
     void deleteStorage(StorageId storage);
@@ -131,9 +121,9 @@ public:
         return false;
     }
 
+    RawMetadata getMetadata(StorageId storage, std::string_view path) const;
     RawMetadata getMetadata(StorageId storage, FileId node) const;
 
-    std::vector<std::string> listFiles(StorageId storage) const;
     bool startLoading(StorageId storage) const;
 
     bool configureNativeBinaries(StorageId storage, std::string_view apkFullPath,
@@ -151,7 +141,7 @@ public:
         const std::string packageName;
     };
 
-    class IncrementalServiceConnector : public BnIncrementalServiceConnector {
+    class IncrementalServiceConnector : public os::incremental::BnIncrementalServiceConnector {
     public:
         IncrementalServiceConnector(IncrementalService& incrementalService, int32_t storage)
               : incrementalService(incrementalService), storage(storage) {}
@@ -163,14 +153,13 @@ public:
     };
 
 private:
-    static const bool sEnablePerfLogging;
-
     struct IncFsMount;
 
-    class DataLoaderStub : public android::content::pm::BnDataLoaderStatusListener {
+    class DataLoaderStub : public content::pm::BnDataLoaderStatusListener {
     public:
-        DataLoaderStub(IncrementalService& service, MountId id, DataLoaderParamsParcel&& params,
+        DataLoaderStub(IncrementalService& service, MountId id,
-                       FileSystemControlParcel&& control,
+                       content::pm::DataLoaderParamsParcel&& params,
+                       content::pm::FileSystemControlParcel&& control,
                        const DataLoaderStatusListener* externalListener);
         ~DataLoaderStub();
         // Cleans up the internal state and invalidates DataLoaderStub. Any subsequent calls will
@@ -184,7 +173,7 @@ private:
         void onDump(int fd);
 
         MountId id() const { return mId; }
-        const DataLoaderParamsParcel& params() const { return mParams; }
+        const content::pm::DataLoaderParamsParcel& params() const { return mParams; }
 
     private:
         binder::Status onStatusChanged(MountId mount, int newStatus) final;
@@ -202,14 +191,14 @@ private:
 
         IncrementalService& mService;
         MountId mId = kInvalidStorageId;
-        DataLoaderParamsParcel mParams;
+        content::pm::DataLoaderParamsParcel mParams;
-        FileSystemControlParcel mControl;
+        content::pm::FileSystemControlParcel mControl;
         DataLoaderStatusListener mListener;
 
         std::mutex mStatusMutex;
         std::condition_variable mStatusCondition;
-        int mCurrentStatus = IDataLoaderStatusListener::DATA_LOADER_DESTROYED;
+        int mCurrentStatus = content::pm::IDataLoaderStatusListener::DATA_LOADER_DESTROYED;
-        int mTargetStatus = IDataLoaderStatusListener::DATA_LOADER_DESTROYED;
+        int mTargetStatus = content::pm::IDataLoaderStatusListener::DATA_LOADER_DESTROYED;
         TimePoint mTargetStatusTs = {};
     };
     using DataLoaderStubPtr = sp<DataLoaderStub>;
@@ -228,7 +217,7 @@ private:
 
         using Control = incfs::UniqueControl;
 
-        using BindMap = std::map<std::string, Bind>;
+        using BindMap = std::map<std::string, Bind, path::PathLess>;
         using StorageMap = std::unordered_map<StorageId, Storage>;
 
         mutable std::mutex lock;
@@ -260,7 +249,10 @@ private:
     using MountMap = std::unordered_map<MountId, IfsMountPtr>;
     using BindPathMap = std::map<std::string, IncFsMount::BindMap::iterator, path::PathLess>;
 
-    void mountExistingImages();
+    static bool perfLoggingEnabled();
+
+    std::unordered_set<std::string_view> adoptMountedInstances();
+    void mountExistingImages(const std::unordered_set<std::string_view>& mountedRootNames);
     bool mountExistingImage(std::string_view root);
 
     IfsMountPtr getIfs(StorageId storage) const;
@@ -273,7 +265,13 @@ private:
                            std::string&& source, std::string&& target, BindKind kind,
                            std::unique_lock<std::mutex>& mainLock);
 
-    DataLoaderStubPtr prepareDataLoader(IncFsMount& ifs, DataLoaderParamsParcel&& params,
+    void addBindMountRecordLocked(IncFsMount& ifs, StorageId storage, std::string&& metadataName,
+                                  std::string&& source, std::string&& target, BindKind kind);
+
+    DataLoaderStubPtr prepareDataLoader(IncFsMount& ifs,
+                                        content::pm::DataLoaderParamsParcel&& params,
+                                        const DataLoaderStatusListener* externalListener = nullptr);
+    void prepareDataLoaderLocked(IncFsMount& ifs, content::pm::DataLoaderParamsParcel&& params,
                                  const DataLoaderStatusListener* externalListener = nullptr);
 
     BindPathMap::const_iterator findStorageLocked(std::string_view path) const;
@@ -283,9 +281,10 @@ private:
     void deleteStorageLocked(IncFsMount& ifs, std::unique_lock<std::mutex>&& ifsLock);
     MountMap::iterator getStorageSlotLocked();
     std::string normalizePathToStorage(const IfsMountPtr& incfs, StorageId storage,
-                                       std::string_view path);
+                                       std::string_view path) const;
-    std::string normalizePathToStorageLocked(IncFsMount::StorageMap::iterator storageIt,
+    std::string normalizePathToStorageLocked(const IfsMountPtr& incfs,
-                                             std::string_view path);
+                                             IncFsMount::StorageMap::iterator storageIt,
+                                             std::string_view path) const;
 
     binder::Status applyStorageParams(IncFsMount& ifs, bool enableReadLogs);
 

services/incremental/IncrementalServiceValidation.cpp

@@ -0,0 +1,77 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "IncrementalServiceValidation.h"
+
+#include <android-base/stringprintf.h>
+#include <binder/IPCThreadState.h>
+#include <binder/PermissionCache.h>
+#include <binder/PermissionController.h>
+#include <errno.h>
+#include <utils/String16.h>
+
+namespace android::incremental {
+
+binder::Status Ok() {
+    return binder::Status::ok();
+}
+
+binder::Status Exception(uint32_t code, const std::string& msg) {
+    return binder::Status::fromExceptionCode(code, String8(msg.c_str()));
+}
+
+int fromBinderStatus(const binder::Status& status) {
+    return status.exceptionCode() == binder::Status::EX_SERVICE_SPECIFIC
+            ? status.serviceSpecificErrorCode() > 0
+                    ? -status.serviceSpecificErrorCode()
+                    : status.serviceSpecificErrorCode() == 0 ? -EFAULT
+                                                             : status.serviceSpecificErrorCode()
+            : -EIO;
+}
+
+binder::Status CheckPermissionForDataDelivery(const char* permission, const char* operation,
+                                              const char* package) {
+    using android::base::StringPrintf;
+
+    int32_t pid;
+    int32_t uid;
+
+    if (!PermissionCache::checkCallingPermission(String16(permission), &pid, &uid)) {
+        return Exception(binder::Status::EX_SECURITY,
+                         StringPrintf("UID %d / PID %d lacks permission %s", uid, pid, permission));
+    }
+
+    String16 packageName{package};
+
+    // Caller must also have op granted.
+    PermissionController pc;
+    if (auto packageUid = pc.getPackageUid(packageName, 0); packageUid != uid) {
+        return Exception(binder::Status::EX_SECURITY,
+                         StringPrintf("UID %d / PID %d does not own package %s", uid, pid,
+                                      package));
+    }
+    switch (auto result = pc.noteOp(String16(operation), uid, packageName); result) {
+        case PermissionController::MODE_ALLOWED:
+        case PermissionController::MODE_DEFAULT:
+            return binder::Status::ok();
+        default:
+            return Exception(binder::Status::EX_SECURITY,
+                             StringPrintf("UID %d / PID %d / package %s lacks app-op %s, error %d",
+                                          uid, pid, package, operation, result));
+    }
+}
+
+} // namespace android::incremental

services/incremental/IncrementalServiceValidation.h

@@ -16,61 +16,17 @@
 
 #pragma once
 
-#include <android-base/stringprintf.h>
-#include <binder/IPCThreadState.h>
-#include <binder/PermissionCache.h>
-#include <binder/PermissionController.h>
 #include <binder/Status.h>
+#include <stdint.h>
+
+#include <string>
 
 namespace android::incremental {
 
-inline binder::Status Ok() {
+binder::Status Ok();
-    return binder::Status::ok();
+binder::Status Exception(uint32_t code, const std::string& msg);
-}
+int fromBinderStatus(const binder::Status& status);
-
+binder::Status CheckPermissionForDataDelivery(const char* permission, const char* operation,
-inline binder::Status Exception(uint32_t code, const std::string& msg) {
+                                              const char* package);
-    return binder::Status::fromExceptionCode(code, String8(msg.c_str()));
-}
-
-inline int fromBinderStatus(const binder::Status& status) {
-    return status.exceptionCode() == binder::Status::EX_SERVICE_SPECIFIC
-            ? status.serviceSpecificErrorCode() > 0 ? -status.serviceSpecificErrorCode()
-                                                    : status.serviceSpecificErrorCode() == 0
-                            ? -EFAULT
-                            : status.serviceSpecificErrorCode()
-            : -EIO;
-}
-
-inline binder::Status CheckPermissionForDataDelivery(const char* permission, const char* operation,
-                                                     const char* package) {
-    using android::base::StringPrintf;
-
-    int32_t pid;
-    int32_t uid;
-
-    if (!PermissionCache::checkCallingPermission(String16(permission), &pid, &uid)) {
-        return Exception(binder::Status::EX_SECURITY,
-                         StringPrintf("UID %d / PID %d lacks permission %s", uid, pid, permission));
-    }
-
-    String16 packageName{package};
-
-    // Caller must also have op granted.
-    PermissionController pc;
-    if (auto packageUid = pc.getPackageUid(packageName, 0); packageUid != uid) {
-        return Exception(binder::Status::EX_SECURITY,
-                         StringPrintf("UID %d / PID %d does not own package %s", uid, pid,
-                                      package));
-    }
-    switch (auto result = pc.noteOp(String16(operation), uid, packageName); result) {
-        case PermissionController::MODE_ALLOWED:
-        case PermissionController::MODE_DEFAULT:
-            return binder::Status::ok();
-        default:
-            return Exception(binder::Status::EX_SECURITY,
-                             StringPrintf("UID %d / PID %d / package %s lacks app-op %s, error %d",
-                                          uid, pid, package, operation, result));
-    }
-}
 
 } // namespace android::incremental

services/incremental/ServiceWrappers.cpp

@@ -18,12 +18,18 @@
 
 #include "ServiceWrappers.h"
 
+#include <MountRegistry.h>
 #include <android-base/logging.h>
+#include <android/content/pm/IDataLoaderManager.h>
+#include <android/os/IVold.h>
+#include <binder/AppOpsManager.h>
 #include <utils/String16.h>
 
+#include "IncrementalServiceValidation.h"
+
 using namespace std::literals;
 
-namespace android::os::incremental {
+namespace android::incremental {
 
 static constexpr auto kVoldServiceName = "vold"sv;
 static constexpr auto kDataLoaderManagerName = "dataloader_manager"sv;
@@ -32,9 +38,9 @@ class RealVoldService : public VoldServiceWrapper {
 public:
     RealVoldService(const sp<os::IVold> vold) : mInterface(std::move(vold)) {}
     ~RealVoldService() = default;
-    binder::Status mountIncFs(const std::string& backingPath, const std::string& targetDir,
+    binder::Status mountIncFs(
-                              int32_t flags,
+            const std::string& backingPath, const std::string& targetDir, int32_t flags,
-                              IncrementalFileSystemControlParcel* _aidl_return) const final {
+            os::incremental::IncrementalFileSystemControlParcel* _aidl_return) const final {
         return mInterface->mountIncFs(backingPath, targetDir, flags, _aidl_return);
     }
     binder::Status unmountIncFs(const std::string& dir) const final {
@@ -56,16 +62,18 @@ private:
 
 class RealDataLoaderManager : public DataLoaderManagerWrapper {
 public:
-    RealDataLoaderManager(const sp<content::pm::IDataLoaderManager> manager)
+    RealDataLoaderManager(sp<content::pm::IDataLoaderManager> manager)
-          : mInterface(manager) {}
+          : mInterface(std::move(manager)) {}
     ~RealDataLoaderManager() = default;
-    binder::Status initializeDataLoader(MountId mountId, const DataLoaderParamsParcel& params,
+    binder::Status initializeDataLoader(MountId mountId,
-                                        const FileSystemControlParcel& control,
+                                        const content::pm::DataLoaderParamsParcel& params,
-                                        const sp<IDataLoaderStatusListener>& listener,
+                                        const content::pm::FileSystemControlParcel& control,
+                                        const sp<content::pm::IDataLoaderStatusListener>& listener,
                                         bool* _aidl_return) const final {
         return mInterface->initializeDataLoader(mountId, params, control, listener, _aidl_return);
     }
-    binder::Status getDataLoader(MountId mountId, sp<IDataLoader>* _aidl_return) const final {
+    binder::Status getDataLoader(MountId mountId,
+                                 sp<content::pm::IDataLoader>* _aidl_return) const final {
         return mInterface->getDataLoader(mountId, _aidl_return);
     }
     binder::Status destroyDataLoader(MountId mountId) const final {
@@ -109,21 +117,28 @@ private:
 class RealIncFs : public IncFsWrapper {
 public:
     RealIncFs() = default;
-    ~RealIncFs() = default;
+    ~RealIncFs() final = default;
+    void listExistingMounts(const ExistingMountCallback& cb) const final {
+        for (auto mount : incfs::defaultMountRegistry().copyMounts()) {
+            auto binds = mount.binds(); // span() doesn't like rvalue containers, needs to save it.
+            cb(mount.root(), mount.backingDir(), binds);
+        }
+    }
+    Control openMount(std::string_view path) const final { return incfs::open(path); }
     Control createControl(IncFsFd cmd, IncFsFd pendingReads, IncFsFd logs) const final {
         return incfs::createControl(cmd, pendingReads, logs);
     }
     ErrorCode makeFile(const Control& control, std::string_view path, int mode, FileId id,
-                       NewFileParams params) const final {
+                       incfs::NewFileParams params) const final {
         return incfs::makeFile(control, path, mode, id, params);
     }
     ErrorCode makeDir(const Control& control, std::string_view path, int mode) const final {
         return incfs::makeDir(control, path, mode);
     }
-    RawMetadata getMetadata(const Control& control, FileId fileid) const final {
+    incfs::RawMetadata getMetadata(const Control& control, FileId fileid) const final {
         return incfs::getMetadata(control, fileid);
     }
-    RawMetadata getMetadata(const Control& control, std::string_view path) const final {
+    incfs::RawMetadata getMetadata(const Control& control, std::string_view path) const final {
         return incfs::getMetadata(control, path);
     }
     FileId getFileId(const Control& control, std::string_view path) const final {
@@ -138,8 +153,8 @@ public:
     base::unique_fd openForSpecialOps(const Control& control, FileId id) const final {
         return base::unique_fd{incfs::openForSpecialOps(control, id).release()};
     }
-    ErrorCode writeBlocks(Span<const DataBlock> blocks) const final {
+    ErrorCode writeBlocks(std::span<const incfs::DataBlock> blocks) const final {
-        return incfs::writeBlocks(blocks);
+        return incfs::writeBlocks({blocks.data(), size_t(blocks.size())});
     }
 };
 
@@ -165,8 +180,9 @@ std::unique_ptr<VoldServiceWrapper> RealServiceManager::getVoldService() {
 }
 
 std::unique_ptr<DataLoaderManagerWrapper> RealServiceManager::getDataLoaderManager() {
-    sp<IDataLoaderManager> manager =
+    sp<content::pm::IDataLoaderManager> manager =
-            RealServiceManager::getRealService<IDataLoaderManager>(kDataLoaderManagerName);
+            RealServiceManager::getRealService<content::pm::IDataLoaderManager>(
+                    kDataLoaderManagerName);
     if (manager) {
         return std::make_unique<RealDataLoaderManager>(manager);
     }
@@ -239,4 +255,4 @@ JavaVM* RealJniWrapper::getJvm(JNIEnv* env) {
     return getJavaVm(env);
 }
 
-} // namespace android::os::incremental
+} // namespace android::incremental

services/incremental/ServiceWrappers.h

@@ -16,29 +16,23 @@
 
 #pragma once
 
-#include "IncrementalServiceValidation.h"
-
-#include <android-base/strings.h>
 #include <android-base/unique_fd.h>
 #include <android/content/pm/DataLoaderParamsParcel.h>
 #include <android/content/pm/FileSystemControlParcel.h>
 #include <android/content/pm/IDataLoader.h>
-#include <android/content/pm/IDataLoaderManager.h>
 #include <android/content/pm/IDataLoaderStatusListener.h>
-#include <android/os/IVold.h>
+#include <binder/IAppOpsCallback.h>
-#include <binder/AppOpsManager.h>
 #include <binder/IServiceManager.h>
+#include <binder/Status.h>
 #include <incfs.h>
 #include <jni.h>
 
 #include <memory>
+#include <span>
 #include <string>
 #include <string_view>
 
-using namespace android::incfs;
+namespace android::incremental {
-using namespace android::content::pm;
-
-namespace android::os::incremental {
 
 // --- Wrapper interfaces ---
 
@@ -47,42 +41,54 @@ using MountId = int32_t;
 class VoldServiceWrapper {
 public:
     virtual ~VoldServiceWrapper() = default;
-    virtual binder::Status mountIncFs(const std::string& backingPath, const std::string& targetDir,
+    virtual binder::Status mountIncFs(
-                                      int32_t flags,
+            const std::string& backingPath, const std::string& targetDir, int32_t flags,
-                                      IncrementalFileSystemControlParcel* _aidl_return) const = 0;
+            os::incremental::IncrementalFileSystemControlParcel* result) const = 0;
     virtual binder::Status unmountIncFs(const std::string& dir) const = 0;
     virtual binder::Status bindMount(const std::string& sourceDir,
                                      const std::string& targetDir) const = 0;
-    virtual binder::Status setIncFsMountOptions(const ::android::os::incremental::IncrementalFileSystemControlParcel& control, bool enableReadLogs) const = 0;
+    virtual binder::Status setIncFsMountOptions(
+            const os::incremental::IncrementalFileSystemControlParcel& control,
+            bool enableReadLogs) const = 0;
 };
 
 class DataLoaderManagerWrapper {
 public:
     virtual ~DataLoaderManagerWrapper() = default;
-    virtual binder::Status initializeDataLoader(MountId mountId,
+    virtual binder::Status initializeDataLoader(
-                                                const DataLoaderParamsParcel& params,
+            MountId mountId, const content::pm::DataLoaderParamsParcel& params,
-                                                const FileSystemControlParcel& control,
+            const content::pm::FileSystemControlParcel& control,
-                                                const sp<IDataLoaderStatusListener>& listener,
+            const sp<content::pm::IDataLoaderStatusListener>& listener, bool* result) const = 0;
-                                                bool* _aidl_return) const = 0;
+    virtual binder::Status getDataLoader(MountId mountId,
-    virtual binder::Status getDataLoader(MountId mountId, sp<IDataLoader>* _aidl_return) const = 0;
+                                         sp<content::pm::IDataLoader>* result) const = 0;
     virtual binder::Status destroyDataLoader(MountId mountId) const = 0;
 };
 
 class IncFsWrapper {
 public:
+    using Control = incfs::Control;
+    using FileId = incfs::FileId;
+    using ErrorCode = incfs::ErrorCode;
+
+    using ExistingMountCallback =
+            std::function<void(std::string_view root, std::string_view backingDir,
+                               std::span<std::pair<std::string_view, std::string_view>> binds)>;
+
     virtual ~IncFsWrapper() = default;
+    virtual void listExistingMounts(const ExistingMountCallback& cb) const = 0;
+    virtual Control openMount(std::string_view path) const = 0;
     virtual Control createControl(IncFsFd cmd, IncFsFd pendingReads, IncFsFd logs) const = 0;
     virtual ErrorCode makeFile(const Control& control, std::string_view path, int mode, FileId id,
-                               NewFileParams params) const = 0;
+                               incfs::NewFileParams params) const = 0;
     virtual ErrorCode makeDir(const Control& control, std::string_view path, int mode) const = 0;
-    virtual RawMetadata getMetadata(const Control& control, FileId fileid) const = 0;
+    virtual incfs::RawMetadata getMetadata(const Control& control, FileId fileid) const = 0;
-    virtual RawMetadata getMetadata(const Control& control, std::string_view path) const = 0;
+    virtual incfs::RawMetadata getMetadata(const Control& control, std::string_view path) const = 0;
     virtual FileId getFileId(const Control& control, std::string_view path) const = 0;
     virtual ErrorCode link(const Control& control, std::string_view from,
                            std::string_view to) const = 0;
     virtual ErrorCode unlink(const Control& control, std::string_view path) const = 0;
     virtual base::unique_fd openForSpecialOps(const Control& control, FileId id) const = 0;
-    virtual ErrorCode writeBlocks(Span<const DataBlock> blocks) const = 0;
+    virtual ErrorCode writeBlocks(std::span<const incfs::DataBlock> blocks) const = 0;
 };
 
 class AppOpsManagerWrapper {
@@ -129,4 +135,4 @@ private:
     JavaVM* const mJvm;
 };
 
-} // namespace android::os::incremental
+} // namespace android::incremental

services/incremental/test/IncrementalServiceTest.cpp

@@ -25,6 +25,7 @@
 #include <future>
 
 #include "IncrementalService.h"
+#include "IncrementalServiceValidation.h"
 #include "Metadata.pb.h"
 #include "ServiceWrappers.h"
 
@@ -262,6 +263,8 @@ private:
 
 class MockIncFs : public IncFsWrapper {
 public:
+    MOCK_CONST_METHOD1(listExistingMounts, void(const ExistingMountCallback& cb));
+    MOCK_CONST_METHOD1(openMount, Control(std::string_view path));
     MOCK_CONST_METHOD3(createControl, Control(IncFsFd cmd, IncFsFd pendingReads, IncFsFd logs));
     MOCK_CONST_METHOD5(makeFile,
                        ErrorCode(const Control& control, std::string_view path, int mode, FileId id,
@@ -274,10 +277,13 @@ public:
                        ErrorCode(const Control& control, std::string_view from, std::string_view to));
     MOCK_CONST_METHOD2(unlink, ErrorCode(const Control& control, std::string_view path));
     MOCK_CONST_METHOD2(openForSpecialOps, base::unique_fd(const Control& control, FileId id));
-    MOCK_CONST_METHOD1(writeBlocks, ErrorCode(Span<const DataBlock> blocks));
+    MOCK_CONST_METHOD1(writeBlocks, ErrorCode(std::span<const DataBlock> blocks));
+
+    MockIncFs() { ON_CALL(*this, listExistingMounts(_)).WillByDefault(Return()); }
 
     void makeFileFails() { ON_CALL(*this, makeFile(_, _, _, _, _)).WillByDefault(Return(-1)); }
     void makeFileSuccess() { ON_CALL(*this, makeFile(_, _, _, _, _)).WillByDefault(Return(0)); }
+
     RawMetadata getMountInfoMetadata(const Control& control, std::string_view path) {
         metadata::Mount m;
         m.mutable_storage()->set_id(100);
@@ -692,14 +698,14 @@ TEST_F(IncrementalServiceTest, testMakeDirectory) {
                                                IncrementalService::CreateOptions::CreateNew);
     std::string dir_path("test");
 
-    std::string tempPath(tempDir.path);
-    std::replace(tempPath.begin(), tempPath.end(), '/', '_');
-    std::string mount_dir = std::string(mRootDir.path) + "/MT_" + tempPath.substr(1);
-    std::string normalized_dir_path = mount_dir + "/mount/st_1_0/" + dir_path;
-
     // Expecting incfs to call makeDir on a path like:
-    // /data/local/tmp/TemporaryDir-06yixG/data_local_tmp_TemporaryDir-xwdFhT/mount/st_1_0/test
+    // <root>/*/mount/<storage>/test
-    EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(normalized_dir_path), _));
+    EXPECT_CALL(*mIncFs,
+                makeDir(_, Truly([&](std::string_view arg) {
+                            return arg.starts_with(mRootDir.path) &&
+                                    arg.ends_with("/mount/st_1_0/" + dir_path);
+                        }),
+                        _));
     auto res = mIncrementalService->makeDir(storageId, dir_path, 0555);
     ASSERT_EQ(res, 0);
 }
@@ -717,29 +723,32 @@ TEST_F(IncrementalServiceTest, testMakeDirectories) {
     auto first = "first"sv;
     auto second = "second"sv;
     auto third = "third"sv;
-
-    std::string tempPath(tempDir.path);
-    std::replace(tempPath.begin(), tempPath.end(), '/', '_');
-    std::string mount_dir = std::string(mRootDir.path) + "/MT_" + tempPath.substr(1);
-
-    InSequence seq;
     auto parent_path = std::string(first) + "/" + std::string(second);
     auto dir_path = parent_path + "/" + std::string(third);
 
-    std::string normalized_first_path = mount_dir + "/mount/st_1_0/" + std::string(first);
+    auto checkArgFor = [&](std::string_view expected, std::string_view arg) {
-    std::string normalized_parent_path = mount_dir + "/mount/st_1_0/" + parent_path;
+        return arg.starts_with(mRootDir.path) && arg.ends_with("/mount/st_1_0/"s += expected);
-    std::string normalized_dir_path = mount_dir + "/mount/st_1_0/" + dir_path;
+    };
 
-    EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(normalized_dir_path), _))
+    {
+        InSequence seq;
+        EXPECT_CALL(*mIncFs,
+                    makeDir(_, Truly([&](auto arg) { return checkArgFor(dir_path, arg); }), _))
                 .WillOnce(Return(-ENOENT));
-    EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(normalized_parent_path), _))
+        EXPECT_CALL(*mIncFs,
+                    makeDir(_, Truly([&](auto arg) { return checkArgFor(parent_path, arg); }), _))
                 .WillOnce(Return(-ENOENT));
-    EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(normalized_first_path), _))
+        EXPECT_CALL(*mIncFs,
+                    makeDir(_, Truly([&](auto arg) { return checkArgFor(first, arg); }), _))
                 .WillOnce(Return(0));
-    EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(normalized_parent_path), _))
+        EXPECT_CALL(*mIncFs,
+                    makeDir(_, Truly([&](auto arg) { return checkArgFor(parent_path, arg); }), _))
                 .WillOnce(Return(0));
-    EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(normalized_dir_path), _)).WillOnce(Return(0));
+        EXPECT_CALL(*mIncFs,
-    auto res = mIncrementalService->makeDirs(storageId, normalized_dir_path, 0555);
+                    makeDir(_, Truly([&](auto arg) { return checkArgFor(dir_path, arg); }), _))
+                .WillOnce(Return(0));
+    }
+    auto res = mIncrementalService->makeDirs(storageId, dir_path, 0555);
     ASSERT_EQ(res, 0);
 }
 } // namespace android::os::incremental