Add mount callback

Mounting a FUSE path needs two steps:
1. Mounting the filesystem
2. Starting the FUSE session in the FUSE daemon

The second part requires retriving an fd from (1) and the mount paths
and passing it to the FUSE daemon.

Previously, we'd return from the Vold mount call and mark the volume
as mounted while we scramble to do (2). This means there's a time
period where the Volume is marked as MOUNTED but not actually ready
and any IO access on the paths will hang forever. This could also be
misleading when interpreting bug reports.

Now, we block the Vold mount call until the FUSE session is started

Test: atest AdoptableHostTest
Bug: 144275217

Change-Id: I45238a31df71286f67ef1c65c711d0085d72e97f

diff --git a/Android.bp b/Android.bp
index 323bc3c..5d93bfa 100644 (file)
--- a/Android.bp
+++ b/Android.bp
@@ -265,6 +265,7 @@ filegroup {
     srcs: [
         "binder/android/os/IVold.aidl",
         "binder/android/os/IVoldListener.aidl",
+        "binder/android/os/IVoldMountCallback.aidl",
         "binder/android/os/IVoldTaskListener.aidl",
     ],
     path: "binder",

diff --git a/VoldNativeService.cpp b/VoldNativeService.cpp
index e20d68e..1f7638f 100644 (file)
--- a/VoldNativeService.cpp
+++ b/VoldNativeService.cpp
@@ -325,9 +325,9 @@ binder::Status VoldNativeService::forgetPartition(const std::string& partGuid,
     return translate(VolumeManager::Instance()->forgetPartition(partGuid, fsUuid));
 }
 
-binder::Status VoldNativeService::mount(const std::string& volId, int32_t mountFlags,
-                                        int32_t mountUserId,
-                                        android::base::unique_fd* _aidl_return) {
+binder::Status VoldNativeService::mount(
+        const std::string& volId, int32_t mountFlags, int32_t mountUserId,
+        const android::sp<android::os::IVoldMountCallback>& callback) {
     ENFORCE_SYSTEM_OR_ROOT;
     CHECK_ARGUMENT_ID(volId);
     ACQUIRE_LOCK;
@@ -340,18 +340,14 @@ binder::Status VoldNativeService::mount(const std::string& volId, int32_t mountF
     vol->setMountFlags(mountFlags);
     vol->setMountUserId(mountUserId);
 
+    vol->setMountCallback(callback);
     int res = vol->mount();
+    vol->setMountCallback(nullptr);
+
     if (res != OK) {
         return translate(res);
     }
 
-    _aidl_return->reset(dup(vol->getFuseFd().get()));
-    if (_aidl_return->get() == -1) {
-        // Let's not return invalid fd since binder will not allow null fds. Instead give it a
-        // default value.
-        _aidl_return->reset(open("/dev/null", O_RDONLY | O_CLOEXEC));
-    }
-
     if ((mountFlags & MOUNT_FLAG_PRIMARY) != 0) {
         res = VolumeManager::Instance()->setPrimary(vol);
         if (res != OK) {

diff --git a/VoldNativeService.h b/VoldNativeService.h
index 744ff84..cc4dd1b 100644 (file)
--- a/VoldNativeService.h
+++ b/VoldNativeService.h
@@ -53,7 +53,7 @@ class VoldNativeService : public BinderService<VoldNativeService>, public os::Bn
     binder::Status forgetPartition(const std::string& partGuid, const std::string& fsUuid);
 
     binder::Status mount(const std::string& volId, int32_t mountFlags, int32_t mountUserId,
-                         android::base::unique_fd* _aidl_return);
+                         const android::sp<android::os::IVoldMountCallback>& callback);
     binder::Status unmount(const std::string& volId);
     binder::Status format(const std::string& volId, const std::string& fsType);
     binder::Status benchmark(const std::string& volId,

diff --git a/binder/android/os/IVold.aidl b/binder/android/os/IVold.aidl
index 91c0172..8412f9a 100644 (file)
--- a/binder/android/os/IVold.aidl
+++ b/binder/android/os/IVold.aidl
@@ -17,6 +17,7 @@
 package android.os;
 
 import android.os.IVoldListener;
+import android.os.IVoldMountCallback;
 import android.os.IVoldTaskListener;
 
 /** {@hide} */
@@ -40,7 +41,8 @@ interface IVold {
     void partition(@utf8InCpp String diskId, int partitionType, int ratio);
     void forgetPartition(@utf8InCpp String partGuid, @utf8InCpp String fsUuid);
 
-    FileDescriptor mount(@utf8InCpp String volId, int mountFlags, int mountUserId);
+    void mount(@utf8InCpp String volId, int mountFlags, int mountUserId,
+         IVoldMountCallback callback);
     void unmount(@utf8InCpp String volId);
     void format(@utf8InCpp String volId, @utf8InCpp String fsType);
     void benchmark(@utf8InCpp String volId, IVoldTaskListener listener);

diff --git a/binder/android/os/IVoldMountCallback.aidl b/binder/android/os/IVoldMountCallback.aidl
new file mode 100644 (file)
index 0000000..6bf46d7
--- /dev/null
+++ b/binder/android/os/IVoldMountCallback.aidl
@@ -0,0 +1,23 @@
+/*
+ * Copyright (C) 2019 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.
+ */
+
+package android.os;
+
+/** {@hide} */
+interface IVoldMountCallback {
+    boolean onVolumeChecking(FileDescriptor fuseFd, @utf8InCpp String path,
+        @utf8InCpp String internalPath);
+}

diff --git a/model/EmulatedVolume.cpp b/model/EmulatedVolume.cpp
index e0d0109..8224e61 100644 (file)
--- a/model/EmulatedVolume.cpp
+++ b/model/EmulatedVolume.cpp
@@ -97,8 +97,17 @@ status_t EmulatedVolume::doMount() {
             PLOG(ERROR) << "Failed to mount emulated fuse volume";
             return -result;
         }
-        setFuseFd(std::move(fd));
-        return 0;
+
+        auto callback = getMountCallback();
+        if (callback) {
+            bool is_ready = false;
+            callback->onVolumeChecking(std::move(fd), getPath(), getInternalPath(), &is_ready);
+            if (!is_ready) {
+                return -EIO;
+            }
+        }
+
+        return OK;
     } else if (getMountUserId() != 0) {
         // For sdcardfs, only mount for user 0, since user 0 will always be running
         // and the paths don't change for different users. Trying to double mount
@@ -177,7 +186,6 @@ status_t EmulatedVolume::doUnmount() {
 
         rmdir(fuse_path.c_str());
         rmdir(pass_through_path.c_str());
-        setFuseFd(android::base::unique_fd());
         return OK;
     } else if (getMountUserId() != 0) {
         // For sdcardfs, only unmount for user 0, since user 0 will always be running

diff --git a/model/PublicVolume.cpp b/model/PublicVolume.cpp
index 403e85c..d1b63a3 100644 (file)
--- a/model/PublicVolume.cpp
+++ b/model/PublicVolume.cpp
@@ -180,7 +180,16 @@ status_t PublicVolume::doMount() {
             LOG(ERROR) << "Failed to mount public fuse volume";
             return -result;
         }
-        setFuseFd(std::move(fd));
+
+        auto callback = getMountCallback();
+        if (callback) {
+            bool is_ready = false;
+            callback->onVolumeChecking(std::move(fd), getPath(), getInternalPath(), &is_ready);
+            if (!is_ready) {
+                return -EIO;
+            }
+        }
+
         return OK;
     }
 
@@ -269,9 +278,6 @@ status_t PublicVolume::doUnmount() {
         rmdir(pass_through_path.c_str());
         rmdir(mRawPath.c_str());
         mRawPath.clear();
-
-        setFuseFd(android::base::unique_fd());
-
         return OK;
     }
 

diff --git a/model/VolumeBase.cpp b/model/VolumeBase.cpp
index ae45f7e..636c065 100644 (file)
--- a/model/VolumeBase.cpp
+++ b/model/VolumeBase.cpp
@@ -143,16 +143,16 @@ status_t VolumeBase::setInternalPath(const std::string& internalPath) {
     return OK;
 }
 
-status_t VolumeBase::setFuseFd(android::base::unique_fd fuseFd) {
-    if ((mState != State::kChecking) && (mState != State::kEjecting)) {
-        LOG(WARNING) << getId() << " fuse fd change requires state checking or ejecting";
-        return -EBUSY;
-    }
-
-    mFuseFd = std::move(fuseFd);
+status_t VolumeBase::setMountCallback(
+        const android::sp<android::os::IVoldMountCallback>& callback) {
+    mMountCallback = callback;
     return OK;
 }
 
+sp<android::os::IVoldMountCallback> VolumeBase::getMountCallback() const {
+    return mMountCallback;
+}
+
 android::sp<android::os::IVoldListener> VolumeBase::getListener() const {
     if (mSilent) {
         return nullptr;

diff --git a/model/VolumeBase.h b/model/VolumeBase.h
index 82b0ae0..1d88d1b 100644 (file)
--- a/model/VolumeBase.h
+++ b/model/VolumeBase.h
@@ -19,6 +19,7 @@
 
 #include "Utils.h"
 #include "android/os/IVoldListener.h"
+#include "android/os/IVoldMountCallback.h"
 
 #include <cutils/multiuser.h>
 #include <utils/Errors.h>
@@ -87,13 +88,13 @@ class VolumeBase {
     State getState() const { return mState; }
     const std::string& getPath() const { return mPath; }
     const std::string& getInternalPath() const { return mInternalPath; }
-    const android::base::unique_fd& getFuseFd() const { return mFuseFd; }
     const std::list<std::shared_ptr<VolumeBase>>& getVolumes() const { return mVolumes; }
 
     status_t setDiskId(const std::string& diskId);
     status_t setPartGuid(const std::string& partGuid);
     status_t setMountFlags(int mountFlags);
     status_t setMountUserId(userid_t mountUserId);
+    status_t setMountCallback(const android::sp<android::os::IVoldMountCallback>& callback);
     status_t setSilent(bool silent);
 
     void addVolume(const std::shared_ptr<VolumeBase>& volume);
@@ -123,10 +124,9 @@ class VolumeBase {
     status_t setId(const std::string& id);
     status_t setPath(const std::string& path);
     status_t setInternalPath(const std::string& internalPath);
-    // Takes ownership of the fd passed in.
-    status_t setFuseFd(android::base::unique_fd fuseFd);
 
     android::sp<android::os::IVoldListener> getListener() const;
+    android::sp<android::os::IVoldMountCallback> getMountCallback() const;
 
   private:
     /* ID that uniquely references volume while alive */
@@ -151,8 +151,7 @@ class VolumeBase {
     std::string mInternalPath;
     /* Flag indicating that volume should emit no events */
     bool mSilent;
-    /* The filedescriptor for the fuse device, if the volume uses fuse, or -1 otherwise */
-    android::base::unique_fd mFuseFd;
+    android::sp<android::os::IVoldMountCallback> mMountCallback;
 
     /* Volumes stacked on top of this volume */
     std::list<std::shared_ptr<VolumeBase>> mVolumes;