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

Change-Id: I42f8cd86bd18dc08ba198c98243df67ccbbd7b7e

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();
-                IncrementalManager.CREATE_MODE_CREATE
+        final IncrementalStorage linkedApkStorage =
-                        | IncrementalManager.CREATE_MODE_PERMANENT_BIND);
+                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;
-                sourceStorage.makeLink(
-                        sourceEntryRelativePath, targetStorage, targetEntryRelativePath);
-            } else if (entry.isDirectory()) {
-                linkFiles(sourceStorage, entry, sourceEntryRelativePath, targetStorage,
-                        targetEntryRelativePath);
             }
-        }
+
+            @Override
+            public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)
+                    throws IOException {
+                final Path relativeFile = sourceBase.relativize(file);
+                sourceStorage.makeLink(
+                        file.toAbsolutePath().toString(), targetStorage,
+                        targetRelative.resolve(relativeFile).toString());
+                return FileVisitResult.CONTINUE;
+            }
+        });
     }
 
     /**

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) {
+    _aidl_return->assign(metadata.begin(), metadata.end());
-        auto metadata = mImpl.getMetadata(storageId, fid);
-        _aidl_return->assign(metadata.begin(), metadata.end());
-    }
     return ok();
 }
 

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,8 +265,14 @@ 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;
     StorageId findStorageId(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), _))
+    {
-            .WillOnce(Return(-ENOENT));
+        InSequence seq;
-    EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(normalized_parent_path), _))
+        EXPECT_CALL(*mIncFs,
-            .WillOnce(Return(-ENOENT));
+                    makeDir(_, Truly([&](auto arg) { return checkArgFor(dir_path, arg); }), _))
-    EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(normalized_first_path), _))
+                .WillOnce(Return(-ENOENT));
-            .WillOnce(Return(0));
+        EXPECT_CALL(*mIncFs,
-    EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(normalized_parent_path), _))
+                    makeDir(_, Truly([&](auto arg) { return checkArgFor(parent_path, arg); }), _))
-            .WillOnce(Return(0));
+                .WillOnce(Return(-ENOENT));
-    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(first, arg); }), _))
+                .WillOnce(Return(0));
+        EXPECT_CALL(*mIncFs,
+                    makeDir(_, Truly([&](auto arg) { return checkArgFor(parent_path, arg); }), _))
+                .WillOnce(Return(0));
+        EXPECT_CALL(*mIncFs,
+                    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