diff --git a/Android.bp b/Android.bp index ee381a42d728..01a43b6a8de7 100644 --- a/Android.bp +++ b/Android.bp @@ -930,6 +930,8 @@ filegroup { srcs: [ "core/java/android/os/incremental/IIncrementalService.aidl", "core/java/android/os/incremental/IncrementalNewFileParams.aidl", + "core/java/android/os/incremental/IStorageHealthListener.aidl", + "core/java/android/os/incremental/StorageHealthCheckParams.aidl", ], path: "core/java", } diff --git a/core/java/android/os/incremental/IIncrementalService.aidl b/core/java/android/os/incremental/IIncrementalService.aidl index 25cb0400a38d..4c5757098025 100644 --- a/core/java/android/os/incremental/IIncrementalService.aidl +++ b/core/java/android/os/incremental/IIncrementalService.aidl @@ -19,6 +19,8 @@ package android.os.incremental; import android.content.pm.DataLoaderParamsParcel; import android.content.pm.IDataLoaderStatusListener; import android.os.incremental.IncrementalNewFileParams; +import android.os.incremental.IStorageHealthListener; +import android.os.incremental.StorageHealthCheckParams; /** @hide */ interface IIncrementalService { @@ -34,7 +36,10 @@ interface IIncrementalService { * Opens or creates a storage given a target path and data loader params. Returns the storage ID. */ int openStorage(in @utf8InCpp String path); - int createStorage(in @utf8InCpp String path, in DataLoaderParamsParcel params, in IDataLoaderStatusListener listener, int createMode); + int createStorage(in @utf8InCpp String path, in DataLoaderParamsParcel params, int createMode, + in IDataLoaderStatusListener statusListener, + in StorageHealthCheckParams healthCheckParams, + in IStorageHealthListener healthListener); int createLinkedStorage(in @utf8InCpp String path, int otherStorageId, int createMode); /** diff --git a/core/java/android/os/incremental/IStorageHealthListener.aidl b/core/java/android/os/incremental/IStorageHealthListener.aidl new file mode 100644 index 000000000000..9f93ede5c9fc --- /dev/null +++ b/core/java/android/os/incremental/IStorageHealthListener.aidl @@ -0,0 +1,35 @@ +/* + * 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. + */ + +package android.os.incremental; + +/** @hide */ +oneway interface IStorageHealthListener { + /** OK status, no pending reads. */ + const int HEALTH_STATUS_OK = 0; + /* Statuses depend on timeouts defined in StorageHealthCheckParams. */ + /** Pending reads detected, waiting for params.blockedTimeoutMs to confirm blocked state. */ + const int HEALTH_STATUS_READS_PENDING = 1; + /** There are reads pending for params.blockedTimeoutMs, waiting till + * params.unhealthyTimeoutMs to confirm unhealthy state. */ + const int HEALTH_STATUS_BLOCKED = 2; + /** There are reads pending for params.unhealthyTimeoutMs>, + * marking storage as unhealthy. */ + const int HEALTH_STATUS_UNHEALTHY = 3; + + /** Health status callback. */ + void onHealthStatus(in int storageId, in int status); +} diff --git a/core/java/android/os/incremental/IncrementalFileStorages.java b/core/java/android/os/incremental/IncrementalFileStorages.java index 958c7fb4fc8d..863d86ef88c9 100644 --- a/core/java/android/os/incremental/IncrementalFileStorages.java +++ b/core/java/android/os/incremental/IncrementalFileStorages.java @@ -65,7 +65,9 @@ public final class IncrementalFileStorages { public static IncrementalFileStorages initialize(Context context, @NonNull File stageDir, @NonNull DataLoaderParams dataLoaderParams, - @Nullable IDataLoaderStatusListener dataLoaderStatusListener, + @Nullable IDataLoaderStatusListener statusListener, + @Nullable StorageHealthCheckParams healthCheckParams, + @Nullable IStorageHealthListener healthListener, List addedFiles) throws IOException { // TODO(b/136132412): sanity check if session should not be incremental IncrementalManager incrementalManager = (IncrementalManager) context.getSystemService( @@ -75,9 +77,9 @@ public final class IncrementalFileStorages { throw new IOException("Failed to obtain incrementalManager."); } - final IncrementalFileStorages result = - new IncrementalFileStorages(stageDir, incrementalManager, dataLoaderParams, - dataLoaderStatusListener); + final IncrementalFileStorages result = new IncrementalFileStorages(stageDir, + incrementalManager, dataLoaderParams, statusListener, healthCheckParams, + healthListener); for (InstallationFileParcel file : addedFiles) { if (file.location == LOCATION_DATA_APP) { try { @@ -100,7 +102,9 @@ public final class IncrementalFileStorages { private IncrementalFileStorages(@NonNull File stageDir, @NonNull IncrementalManager incrementalManager, @NonNull DataLoaderParams dataLoaderParams, - @Nullable IDataLoaderStatusListener dataLoaderStatusListener) throws IOException { + @Nullable IDataLoaderStatusListener statusListener, + @Nullable StorageHealthCheckParams healthCheckParams, + @Nullable IStorageHealthListener healthListener) throws IOException { try { mStageDir = stageDir; mIncrementalManager = incrementalManager; @@ -117,10 +121,9 @@ public final class IncrementalFileStorages { mDefaultStorage.bind(stageDir.getAbsolutePath()); } else { mDefaultStorage = mIncrementalManager.createStorage(stageDir.getAbsolutePath(), - dataLoaderParams, - dataLoaderStatusListener, - IncrementalManager.CREATE_MODE_CREATE - | IncrementalManager.CREATE_MODE_TEMPORARY_BIND, false); + dataLoaderParams, IncrementalManager.CREATE_MODE_CREATE + | IncrementalManager.CREATE_MODE_TEMPORARY_BIND, false, + statusListener, healthCheckParams, healthListener); if (mDefaultStorage == null) { throw new IOException( "Couldn't create incremental storage at " + stageDir); diff --git a/core/java/android/os/incremental/IncrementalManager.java b/core/java/android/os/incremental/IncrementalManager.java index 916edfae679f..c7f50c951d70 100644 --- a/core/java/android/os/incremental/IncrementalManager.java +++ b/core/java/android/os/incremental/IncrementalManager.java @@ -110,11 +110,15 @@ public final class IncrementalManager { */ @Nullable public IncrementalStorage createStorage(@NonNull String path, - @NonNull DataLoaderParams params, @Nullable IDataLoaderStatusListener listener, + @NonNull DataLoaderParams params, @CreateMode int createMode, - boolean autoStartDataLoader) { + boolean autoStartDataLoader, + @Nullable IDataLoaderStatusListener statusListener, + @Nullable StorageHealthCheckParams healthCheckParams, + @Nullable IStorageHealthListener healthListener) { try { - final int id = mService.createStorage(path, params.getData(), listener, createMode); + final int id = mService.createStorage(path, params.getData(), createMode, + statusListener, healthCheckParams, healthListener); if (id < 0) { return null; } diff --git a/core/java/android/os/incremental/StorageHealthCheckParams.aidl b/core/java/android/os/incremental/StorageHealthCheckParams.aidl new file mode 100644 index 000000000000..6839317a9ad5 --- /dev/null +++ b/core/java/android/os/incremental/StorageHealthCheckParams.aidl @@ -0,0 +1,35 @@ +/* + * 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. + */ + +package android.os.incremental; + +/** + * @hide + */ +parcelable StorageHealthCheckParams { + /** Timeouts of the oldest pending read. + * Valid values 0ms < blockedTimeoutMs < unhealthyTimeoutMs < storage page read timeout. + * Invalid values will disable health checking. */ + + /** To consider storage "blocked". */ + int blockedTimeoutMs; + /** To consider storage "unhealthy". */ + int unhealthyTimeoutMs; + + /** After storage is marked "unhealthy", how often to check if it recovered. + * Valid value 1000ms < unhealthyMonitoringMs. */ + int unhealthyMonitoringMs; +} diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java index f8278de1531d..7ab05c4f762c 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerSession.java +++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java @@ -105,8 +105,10 @@ import android.os.RemoteException; import android.os.RevocableFileDescriptor; import android.os.SystemProperties; import android.os.UserHandle; +import android.os.incremental.IStorageHealthListener; import android.os.incremental.IncrementalFileStorages; import android.os.incremental.IncrementalManager; +import android.os.incremental.StorageHealthCheckParams; import android.os.storage.StorageManager; import android.provider.Settings.Secure; import android.stats.devicepolicy.DevicePolicyEnums; @@ -231,6 +233,10 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { private static final String SYSTEM_DATA_LOADER_PACKAGE = "android"; + private static final int INCREMENTAL_STORAGE_BLOCKED_TIMEOUT_MS = 2000; + private static final int INCREMENTAL_STORAGE_UNHEALTHY_TIMEOUT_MS = 7000; + private static final int INCREMENTAL_STORAGE_UNHEALTHY_MONITORING_MS = 60000; + // TODO: enforce INSTALL_ALLOW_TEST // TODO: enforce INSTALL_ALLOW_DOWNGRADE @@ -1568,7 +1574,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { dispatchSessionFinished(error, detailMessage, null); } - private void onDataLoaderUnrecoverable() { + private void onStorageUnhealthy() { if (TextUtils.isEmpty(mPackageName)) { // The package has not been installed. return; @@ -2745,7 +2751,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { final DataLoaderParams params = this.params.dataLoaderParams; final boolean manualStartAndDestroy = !isIncrementalInstallation(); - final IDataLoaderStatusListener listener = new IDataLoaderStatusListener.Stub() { + final IDataLoaderStatusListener statusListener = new IDataLoaderStatusListener.Stub() { @Override public void onStatusChanged(int dataLoaderId, int status) { switch (status) { @@ -2757,7 +2763,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { if (mDestroyed || mDataLoaderFinished) { switch (status) { case IDataLoaderStatusListener.DATA_LOADER_UNRECOVERABLE: - onDataLoaderUnrecoverable(); + onStorageUnhealthy(); return; } return; @@ -2840,9 +2846,49 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { }; if (!manualStartAndDestroy) { + final StorageHealthCheckParams healthCheckParams = new StorageHealthCheckParams(); + healthCheckParams.blockedTimeoutMs = INCREMENTAL_STORAGE_BLOCKED_TIMEOUT_MS; + healthCheckParams.unhealthyTimeoutMs = INCREMENTAL_STORAGE_UNHEALTHY_TIMEOUT_MS; + healthCheckParams.unhealthyMonitoringMs = INCREMENTAL_STORAGE_UNHEALTHY_MONITORING_MS; + + final boolean systemDataLoader = + params.getComponentName().getPackageName() == SYSTEM_DATA_LOADER_PACKAGE; + final IStorageHealthListener healthListener = new IStorageHealthListener.Stub() { + @Override + public void onHealthStatus(int storageId, int status) { + if (mDestroyed || mDataLoaderFinished) { + // App's installed. + switch (status) { + case IStorageHealthListener.HEALTH_STATUS_UNHEALTHY: + onStorageUnhealthy(); + return; + } + return; + } + + switch (status) { + case IStorageHealthListener.HEALTH_STATUS_OK: + break; + case IStorageHealthListener.HEALTH_STATUS_READS_PENDING: + case IStorageHealthListener.HEALTH_STATUS_BLOCKED: + if (systemDataLoader) { + // It's OK for ADB data loader to wait for pages. + break; + } + // fallthrough + case IStorageHealthListener.HEALTH_STATUS_UNHEALTHY: + // Even ADB installation can't wait for missing pages for too long. + mDataLoaderFinished = true; + dispatchSessionVerificationFailure(INSTALL_FAILED_MEDIA_UNAVAILABLE, + "Image is missing pages required for installation."); + break; + } + } + }; + try { mIncrementalFileStorages = IncrementalFileStorages.initialize(mContext, stageDir, - params, listener, addedFiles); + params, statusListener, healthCheckParams, healthListener, addedFiles); return false; } catch (IOException e) { throw new PackageManagerException(INSTALL_FAILED_MEDIA_UNAVAILABLE, e.getMessage(), @@ -2850,8 +2896,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } } - if (!dataLoaderManager.bindToDataLoader( - sessionId, params.getData(), listener)) { + if (!dataLoaderManager.bindToDataLoader(sessionId, params.getData(), statusListener)) { throw new PackageManagerException(INSTALL_FAILED_MEDIA_UNAVAILABLE, "Failed to initialize data loader"); } diff --git a/services/incremental/BinderIncrementalService.cpp b/services/incremental/BinderIncrementalService.cpp index 847667427593..6018b9e0b2e0 100644 --- a/services/incremental/BinderIncrementalService.cpp +++ b/services/incremental/BinderIncrementalService.cpp @@ -118,14 +118,18 @@ binder::Status BinderIncrementalService::openStorage(const std::string& path, } binder::Status BinderIncrementalService::createStorage( - const std::string& path, const content::pm::DataLoaderParamsParcel& params, - const ::android::sp<::android::content::pm::IDataLoaderStatusListener>& listener, - int32_t createMode, int32_t* _aidl_return) { + const ::std::string& path, const ::android::content::pm::DataLoaderParamsParcel& params, + int32_t createMode, + const ::android::sp<::android::content::pm::IDataLoaderStatusListener>& statusListener, + const ::android::os::incremental::StorageHealthCheckParams& healthCheckParams, + const ::android::sp<::android::os::incremental::IStorageHealthListener>& healthListener, + int32_t* _aidl_return) { *_aidl_return = mImpl.createStorage(path, const_cast(params), - listener, - android::incremental::IncrementalService::CreateOptions( - createMode)); + android::incremental::IncrementalService::CreateOptions(createMode), + statusListener, + const_cast(healthCheckParams), + healthListener); return ok(); } diff --git a/services/incremental/BinderIncrementalService.h b/services/incremental/BinderIncrementalService.h index 5a7d5da56f18..af1136344246 100644 --- a/services/incremental/BinderIncrementalService.h +++ b/services/incremental/BinderIncrementalService.h @@ -41,8 +41,11 @@ public: binder::Status openStorage(const std::string& path, int32_t* _aidl_return) final; binder::Status createStorage( const ::std::string& path, const ::android::content::pm::DataLoaderParamsParcel& params, - const ::android::sp<::android::content::pm::IDataLoaderStatusListener>& listener, - int32_t createMode, int32_t* _aidl_return) final; + int32_t createMode, + const ::android::sp<::android::content::pm::IDataLoaderStatusListener>& statusListener, + const ::android::os::incremental::StorageHealthCheckParams& healthCheckParams, + const ::android::sp& healthListener, + int32_t* _aidl_return) final; binder::Status createLinkedStorage(const std::string& path, int32_t otherStorageId, int32_t createMode, int32_t* _aidl_return) final; binder::Status makeBindMount(int32_t storageId, const std::string& sourcePath, @@ -55,8 +58,7 @@ public: binder::Status makeDirectories(int32_t storageId, const std::string& path, int32_t* _aidl_return) final; binder::Status makeFile(int32_t storageId, const std::string& path, - const ::android::os::incremental::IncrementalNewFileParams& params, - int32_t* _aidl_return) final; + const IncrementalNewFileParams& params, int32_t* _aidl_return) final; binder::Status makeFileFromRange(int32_t storageId, const std::string& targetPath, const std::string& sourcePath, int64_t start, int64_t end, int32_t* _aidl_return) final; diff --git a/services/incremental/IncrementalService.cpp b/services/incremental/IncrementalService.cpp index f0dca772adaa..b03d1eae8ca0 100644 --- a/services/incremental/IncrementalService.cpp +++ b/services/incremental/IncrementalService.cpp @@ -410,9 +410,12 @@ auto IncrementalService::getStorageSlotLocked() -> MountMap::iterator { } } -StorageId IncrementalService::createStorage( - std::string_view mountPoint, DataLoaderParamsParcel&& dataLoaderParams, - const DataLoaderStatusListener& dataLoaderStatusListener, CreateOptions options) { +StorageId IncrementalService::createStorage(std::string_view mountPoint, + content::pm::DataLoaderParamsParcel&& dataLoaderParams, + CreateOptions options, + const DataLoaderStatusListener& statusListener, + StorageHealthCheckParams&& healthCheckParams, + const StorageHealthListener& healthListener) { LOG(INFO) << "createStorage: " << mountPoint << " | " << int(options); if (!path::isAbsolute(mountPoint)) { LOG(ERROR) << "path is not absolute: " << mountPoint; @@ -545,8 +548,8 @@ StorageId IncrementalService::createStorage( // Done here as well, all data structures are in good state. secondCleanupOnFailure.release(); - auto dataLoaderStub = - prepareDataLoader(*ifs, std::move(dataLoaderParams), &dataLoaderStatusListener); + auto dataLoaderStub = prepareDataLoader(*ifs, std::move(dataLoaderParams), &statusListener, + std::move(healthCheckParams), &healthListener); CHECK(dataLoaderStub); mountIt->second = std::move(ifs); @@ -1254,7 +1257,7 @@ bool IncrementalService::mountExistingImage(std::string_view root) { dataLoaderParams.arguments = loader.arguments(); } - prepareDataLoader(*ifs, std::move(dataLoaderParams), nullptr); + prepareDataLoader(*ifs, std::move(dataLoaderParams)); CHECK(ifs->dataLoaderStub); std::vector> bindPoints; @@ -1338,14 +1341,18 @@ void IncrementalService::runCmdLooper() { IncrementalService::DataLoaderStubPtr IncrementalService::prepareDataLoader( IncFsMount& ifs, DataLoaderParamsParcel&& params, - const DataLoaderStatusListener* externalListener) { + const DataLoaderStatusListener* statusListener, + StorageHealthCheckParams&& healthCheckParams, const StorageHealthListener* healthListener) { std::unique_lock l(ifs.lock); - prepareDataLoaderLocked(ifs, std::move(params), externalListener); + prepareDataLoaderLocked(ifs, std::move(params), statusListener, std::move(healthCheckParams), + healthListener); return ifs.dataLoaderStub; } void IncrementalService::prepareDataLoaderLocked(IncFsMount& ifs, DataLoaderParamsParcel&& params, - const DataLoaderStatusListener* externalListener) { + const DataLoaderStatusListener* statusListener, + StorageHealthCheckParams&& healthCheckParams, + const StorageHealthListener* healthListener) { if (ifs.dataLoaderStub) { LOG(INFO) << "Skipped data loader preparation because it already exists"; return; @@ -1360,7 +1367,8 @@ void IncrementalService::prepareDataLoaderLocked(IncFsMount& ifs, DataLoaderPara ifs.dataLoaderStub = new DataLoaderStub(*this, ifs.mountId, std::move(params), std::move(fsControlParcel), - externalListener, path::join(ifs.root, constants().mount)); + statusListener, std::move(healthCheckParams), healthListener, + path::join(ifs.root, constants().mount)); } template @@ -1680,19 +1688,24 @@ void IncrementalService::onAppOpChanged(const std::string& packageName) { IncrementalService::DataLoaderStub::DataLoaderStub(IncrementalService& service, MountId id, DataLoaderParamsParcel&& params, FileSystemControlParcel&& control, - const DataLoaderStatusListener* externalListener, + const DataLoaderStatusListener* statusListener, + StorageHealthCheckParams&& healthCheckParams, + const StorageHealthListener* healthListener, std::string&& healthPath) : mService(service), mId(id), mParams(std::move(params)), mControl(std::move(control)), - mListener(externalListener ? *externalListener : DataLoaderStatusListener()), + mStatusListener(statusListener ? *statusListener : DataLoaderStatusListener()), + mHealthListener(healthListener ? *healthListener : StorageHealthListener()), mHealthPath(std::move(healthPath)) { + // TODO(b/153874006): enable external health listener. + mHealthListener = {}; healthStatusOk(); } IncrementalService::DataLoaderStub::~DataLoaderStub() { - if (mId != kInvalidStorageId) { + if (isValid()) { cleanupResources(); } } @@ -1710,13 +1723,14 @@ void IncrementalService::DataLoaderStub::cleanupResources() { mStatusCondition.wait_until(lock, now + 60s, [this] { return mCurrentStatus == IDataLoaderStatusListener::DATA_LOADER_DESTROYED; }); - mListener = {}; + mStatusListener = {}; + mHealthListener = {}; mId = kInvalidStorageId; } sp IncrementalService::DataLoaderStub::getDataLoader() { sp dataloader; - auto status = mService.mDataLoaderManager->getDataLoader(mId, &dataloader); + auto status = mService.mDataLoaderManager->getDataLoader(id(), &dataloader); if (!status.isOk()) { LOG(ERROR) << "Failed to get dataloader: " << status.toString8(); return {}; @@ -1752,15 +1766,15 @@ void IncrementalService::DataLoaderStub::setTargetStatusLocked(int status) { auto oldStatus = mTargetStatus; mTargetStatus = status; mTargetStatusTs = Clock::now(); - LOG(DEBUG) << "Target status update for DataLoader " << mId << ": " << oldStatus << " -> " + LOG(DEBUG) << "Target status update for DataLoader " << id() << ": " << oldStatus << " -> " << status << " (current " << mCurrentStatus << ")"; } bool IncrementalService::DataLoaderStub::bind() { bool result = false; - auto status = mService.mDataLoaderManager->bindToDataLoader(mId, mParams, this, &result); + auto status = mService.mDataLoaderManager->bindToDataLoader(id(), mParams, this, &result); if (!status.isOk() || !result) { - LOG(ERROR) << "Failed to bind a data loader for mount " << mId; + LOG(ERROR) << "Failed to bind a data loader for mount " << id(); return false; } return true; @@ -1771,9 +1785,9 @@ bool IncrementalService::DataLoaderStub::create() { if (!dataloader) { return false; } - auto status = dataloader->create(mId, mParams, mControl, this); + auto status = dataloader->create(id(), mParams, mControl, this); if (!status.isOk()) { - LOG(ERROR) << "Failed to start DataLoader: " << status.toString8(); + LOG(ERROR) << "Failed to create DataLoader: " << status.toString8(); return false; } return true; @@ -1784,7 +1798,7 @@ bool IncrementalService::DataLoaderStub::start() { if (!dataloader) { return false; } - auto status = dataloader->start(mId); + auto status = dataloader->start(id()); if (!status.isOk()) { LOG(ERROR) << "Failed to start DataLoader: " << status.toString8(); return false; @@ -1793,7 +1807,7 @@ bool IncrementalService::DataLoaderStub::start() { } bool IncrementalService::DataLoaderStub::destroy() { - return mService.mDataLoaderManager->unbindFromDataLoader(mId).isOk(); + return mService.mDataLoaderManager->unbindFromDataLoader(id()).isOk(); } bool IncrementalService::DataLoaderStub::fsmStep() { @@ -1852,8 +1866,8 @@ binder::Status IncrementalService::DataLoaderStub::onStatusChanged(MountId mount return binder::Status:: fromServiceSpecificError(-EINVAL, "onStatusChange came to invalid DataLoaderStub"); } - if (mId != mountId) { - LOG(ERROR) << "Mount ID mismatch: expected " << mId << ", but got: " << mountId; + if (id() != mountId) { + LOG(ERROR) << "Mount ID mismatch: expected " << id() << ", but got: " << mountId; return binder::Status::fromServiceSpecificError(-EPERM, "Mount ID mismatch."); } @@ -1869,7 +1883,7 @@ binder::Status IncrementalService::DataLoaderStub::onStatusChanged(MountId mount mCurrentStatus = newStatus; targetStatus = mTargetStatus; - listener = mListener; + listener = mStatusListener; if (mCurrentStatus == IDataLoaderStatusListener::DATA_LOADER_UNAVAILABLE) { // For unavailable, unbind from DataLoader to ensure proper re-commit. @@ -1877,7 +1891,7 @@ binder::Status IncrementalService::DataLoaderStub::onStatusChanged(MountId mount } } - LOG(DEBUG) << "Current status update for DataLoader " << mId << ": " << oldStatus << " -> " + LOG(DEBUG) << "Current status update for DataLoader " << id() << ": " << oldStatus << " -> " << newStatus << " (target " << targetStatus << ")"; if (listener) { diff --git a/services/incremental/IncrementalService.h b/services/incremental/IncrementalService.h index f3fde2a413e8..bde4ef68d2a7 100644 --- a/services/incremental/IncrementalService.h +++ b/services/incremental/IncrementalService.h @@ -21,6 +21,8 @@ #include #include #include +#include +#include #include #include #include @@ -56,10 +58,15 @@ using RawMetadata = incfs::RawMetadata; using Clock = std::chrono::steady_clock; using TimePoint = std::chrono::time_point; using Seconds = std::chrono::seconds; +using BootClockTsUs = uint64_t; using IDataLoaderStatusListener = ::android::content::pm::IDataLoaderStatusListener; using DataLoaderStatusListener = ::android::sp; +using StorageHealthCheckParams = ::android::os::incremental::StorageHealthCheckParams; +using IStorageHealthListener = ::android::os::incremental::IStorageHealthListener; +using StorageHealthListener = ::android::sp; + class IncrementalService final { public: explicit IncrementalService(ServiceManagerWrapper&& sm, std::string_view rootDir); @@ -72,6 +79,8 @@ public: static constexpr StorageId kInvalidStorageId = -1; static constexpr StorageId kMaxStorageId = std::numeric_limits::max(); + static constexpr BootClockTsUs kMaxBootClockTsUs = std::numeric_limits::max(); + enum CreateOptions { TemporaryBind = 1, PermanentBind = 2, @@ -97,8 +106,9 @@ public: StorageId createStorage(std::string_view mountPoint, content::pm::DataLoaderParamsParcel&& dataLoaderParams, - const DataLoaderStatusListener& dataLoaderStatusListener, - CreateOptions options = CreateOptions::Default); + CreateOptions options, const DataLoaderStatusListener& statusListener, + StorageHealthCheckParams&& healthCheckParams, + const StorageHealthListener& healthListener); StorageId createLinkedStorage(std::string_view mountPoint, StorageId linkedStorage, CreateOptions options = CreateOptions::Default); StorageId openStorage(std::string_view path); @@ -161,7 +171,9 @@ private: DataLoaderStub(IncrementalService& service, MountId id, content::pm::DataLoaderParamsParcel&& params, content::pm::FileSystemControlParcel&& control, - const DataLoaderStatusListener* externalListener, std::string&& healthPath); + const DataLoaderStatusListener* statusListener, + StorageHealthCheckParams&& healthCheckParams, + const StorageHealthListener* healthListener, std::string&& healthPath); ~DataLoaderStub(); // Cleans up the internal state and invalidates DataLoaderStub. Any subsequent calls will // result in an error. @@ -212,7 +224,8 @@ private: MountId mId = kInvalidStorageId; content::pm::DataLoaderParamsParcel mParams; content::pm::FileSystemControlParcel mControl; - DataLoaderStatusListener mListener; + DataLoaderStatusListener mStatusListener; + StorageHealthListener mHealthListener; std::condition_variable mStatusCondition; int mCurrentStatus = content::pm::IDataLoaderStatusListener::DATA_LOADER_DESTROYED; @@ -291,9 +304,13 @@ private: DataLoaderStubPtr prepareDataLoader(IncFsMount& ifs, content::pm::DataLoaderParamsParcel&& params, - const DataLoaderStatusListener* externalListener = nullptr); + const DataLoaderStatusListener* statusListener = nullptr, + StorageHealthCheckParams&& healthCheckParams = {}, + const StorageHealthListener* healthListener = nullptr); void prepareDataLoaderLocked(IncFsMount& ifs, content::pm::DataLoaderParamsParcel&& params, - const DataLoaderStatusListener* externalListener = nullptr); + const DataLoaderStatusListener* statusListener = nullptr, + StorageHealthCheckParams&& healthCheckParams = {}, + const StorageHealthListener* healthListener = nullptr); BindPathMap::const_iterator findStorageLocked(std::string_view path) const; StorageId findStorageId(std::string_view path) const; diff --git a/services/incremental/ServiceWrappers.cpp b/services/incremental/ServiceWrappers.cpp index 08fb486c8058..a76aa625ebc6 100644 --- a/services/incremental/ServiceWrappers.cpp +++ b/services/incremental/ServiceWrappers.cpp @@ -175,6 +175,10 @@ public: ErrorCode writeBlocks(std::span blocks) const final { return incfs::writeBlocks({blocks.data(), size_t(blocks.size())}); } + WaitResult waitForPendingReads(const Control& control, std::chrono::milliseconds timeout, + std::vector* pendingReadsBuffer) const final { + return incfs::waitForPendingReads(control, timeout, pendingReadsBuffer); + } }; RealServiceManager::RealServiceManager(sp serviceManager, JNIEnv* env) diff --git a/services/incremental/ServiceWrappers.h b/services/incremental/ServiceWrappers.h index abbf2f4c4424..a935ab99267e 100644 --- a/services/incremental/ServiceWrappers.h +++ b/services/incremental/ServiceWrappers.h @@ -69,6 +69,7 @@ public: using Control = incfs::Control; using FileId = incfs::FileId; using ErrorCode = incfs::ErrorCode; + using WaitResult = incfs::WaitResult; using ExistingMountCallback = std::function blocks) const = 0; + virtual WaitResult waitForPendingReads( + const Control& control, std::chrono::milliseconds timeout, + std::vector* pendingReadsBuffer) const = 0; }; class AppOpsManagerWrapper { diff --git a/services/incremental/test/IncrementalServiceTest.cpp b/services/incremental/test/IncrementalServiceTest.cpp index 2e4625cf85a1..2948b6a0f293 100644 --- a/services/incremental/test/IncrementalServiceTest.cpp +++ b/services/incremental/test/IncrementalServiceTest.cpp @@ -284,6 +284,9 @@ public: 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(std::span blocks)); + MOCK_CONST_METHOD3(waitForPendingReads, + WaitResult(const Control& control, std::chrono::milliseconds timeout, + std::vector* pendingReadsBuffer)); MockIncFs() { ON_CALL(*this, listExistingMounts(_)).WillByDefault(Return()); } @@ -292,12 +295,23 @@ public: void openMountSuccess() { ON_CALL(*this, openMount(_)).WillByDefault(Invoke(this, &MockIncFs::openMountForHealth)); } + void waitForPendingReadsSuccess() { + ON_CALL(*this, waitForPendingReads(_, _, _)) + .WillByDefault(Invoke(this, &MockIncFs::waitForPendingReadsForHealth)); + } static constexpr auto kPendingReadsFd = 42; Control openMountForHealth(std::string_view) { return UniqueControl(IncFs_CreateControl(-1, kPendingReadsFd, -1)); } + WaitResult waitForPendingReadsForHealth( + const Control& control, std::chrono::milliseconds timeout, + std::vector* pendingReadsBuffer) const { + pendingReadsBuffer->push_back({.bootClockTsUs = 0}); + return android::incfs::WaitResult::HaveData; + } + RawMetadata getMountInfoMetadata(const Control& control, std::string_view path) { metadata::Mount m; m.mutable_storage()->set_id(100); @@ -499,9 +513,9 @@ TEST_F(IncrementalServiceTest, testCreateStorageMountIncFsFails) { mVold->mountIncFsFails(); EXPECT_CALL(*mDataLoaderManager, bindToDataLoader(_, _, _, _)).Times(0); TemporaryDir tempDir; - int storageId = - mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), {}, - IncrementalService::CreateOptions::CreateNew); + int storageId = mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), + IncrementalService::CreateOptions::CreateNew, + {}, {}, {}); ASSERT_LT(storageId, 0); } @@ -510,9 +524,9 @@ TEST_F(IncrementalServiceTest, testCreateStorageMountIncFsInvalidControlParcel) EXPECT_CALL(*mDataLoaderManager, bindToDataLoader(_, _, _, _)).Times(0); EXPECT_CALL(*mDataLoaderManager, unbindFromDataLoader(_)).Times(0); TemporaryDir tempDir; - int storageId = - mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), {}, - IncrementalService::CreateOptions::CreateNew); + int storageId = mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), + IncrementalService::CreateOptions::CreateNew, + {}, {}, {}); ASSERT_LT(storageId, 0); } @@ -523,9 +537,9 @@ TEST_F(IncrementalServiceTest, testCreateStorageMakeFileFails) { EXPECT_CALL(*mDataLoaderManager, unbindFromDataLoader(_)).Times(0); EXPECT_CALL(*mVold, unmountIncFs(_)); TemporaryDir tempDir; - int storageId = - mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), {}, - IncrementalService::CreateOptions::CreateNew); + int storageId = mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), + IncrementalService::CreateOptions::CreateNew, + {}, {}, {}); ASSERT_LT(storageId, 0); } @@ -537,9 +551,9 @@ TEST_F(IncrementalServiceTest, testCreateStorageBindMountFails) { EXPECT_CALL(*mDataLoaderManager, unbindFromDataLoader(_)).Times(0); EXPECT_CALL(*mVold, unmountIncFs(_)); TemporaryDir tempDir; - int storageId = - mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), {}, - IncrementalService::CreateOptions::CreateNew); + int storageId = mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), + IncrementalService::CreateOptions::CreateNew, + {}, {}, {}); ASSERT_LT(storageId, 0); } @@ -555,9 +569,9 @@ TEST_F(IncrementalServiceTest, testCreateStoragePrepareDataLoaderFails) { EXPECT_CALL(*mDataLoader, destroy(_)).Times(0); EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2); TemporaryDir tempDir; - int storageId = - mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), {}, - IncrementalService::CreateOptions::CreateNew); + int storageId = mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), + IncrementalService::CreateOptions::CreateNew, + {}, {}, {}); ASSERT_LT(storageId, 0); } @@ -574,9 +588,9 @@ TEST_F(IncrementalServiceTest, testDeleteStorageSuccess) { EXPECT_CALL(*mDataLoader, destroy(_)).Times(1); EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2); TemporaryDir tempDir; - int storageId = - mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), {}, - IncrementalService::CreateOptions::CreateNew); + int storageId = mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), + IncrementalService::CreateOptions::CreateNew, + {}, {}, {}); ASSERT_GE(storageId, 0); mIncrementalService->deleteStorage(storageId); } @@ -594,9 +608,9 @@ TEST_F(IncrementalServiceTest, testDataLoaderDestroyed) { EXPECT_CALL(*mDataLoader, destroy(_)).Times(1); EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2); TemporaryDir tempDir; - int storageId = - mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), {}, - IncrementalService::CreateOptions::CreateNew); + int storageId = mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), + IncrementalService::CreateOptions::CreateNew, + {}, {}, {}); ASSERT_GE(storageId, 0); // Simulated crash/other connection breakage. mDataLoaderManager->setDataLoaderStatusDestroyed(); @@ -616,9 +630,9 @@ TEST_F(IncrementalServiceTest, testStartDataLoaderCreate) { EXPECT_CALL(*mDataLoader, destroy(_)).Times(1); EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2); TemporaryDir tempDir; - int storageId = - mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), {}, - IncrementalService::CreateOptions::CreateNew); + int storageId = mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), + IncrementalService::CreateOptions::CreateNew, + {}, {}, {}); ASSERT_GE(storageId, 0); mDataLoaderManager->setDataLoaderStatusCreated(); ASSERT_TRUE(mIncrementalService->startLoading(storageId)); @@ -639,9 +653,9 @@ TEST_F(IncrementalServiceTest, testStartDataLoaderPendingStart) { EXPECT_CALL(*mDataLoader, destroy(_)).Times(1); EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2); TemporaryDir tempDir; - int storageId = - mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), {}, - IncrementalService::CreateOptions::CreateNew); + int storageId = mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), + IncrementalService::CreateOptions::CreateNew, + {}, {}, {}); ASSERT_GE(storageId, 0); ASSERT_TRUE(mIncrementalService->startLoading(storageId)); mDataLoaderManager->setDataLoaderStatusCreated(); @@ -661,9 +675,9 @@ TEST_F(IncrementalServiceTest, testStartDataLoaderCreateUnavailable) { EXPECT_CALL(*mDataLoader, destroy(_)).Times(1); EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2); TemporaryDir tempDir; - int storageId = - mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), {}, - IncrementalService::CreateOptions::CreateNew); + int storageId = mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), + IncrementalService::CreateOptions::CreateNew, + {}, {}, {}); ASSERT_GE(storageId, 0); mDataLoaderManager->setDataLoaderStatusUnavailable(); } @@ -672,6 +686,7 @@ TEST_F(IncrementalServiceTest, testStartDataLoaderRecreateOnPendingReads) { mVold->mountIncFsSuccess(); mIncFs->makeFileSuccess(); mIncFs->openMountSuccess(); + mIncFs->waitForPendingReadsSuccess(); mVold->bindMountSuccess(); mDataLoader->initializeCreateOkNoStatus(); mDataLoaderManager->bindToDataLoaderSuccess(); @@ -685,9 +700,9 @@ TEST_F(IncrementalServiceTest, testStartDataLoaderRecreateOnPendingReads) { EXPECT_CALL(*mLooper, addFd(MockIncFs::kPendingReadsFd, _, _, _, _)).Times(1); EXPECT_CALL(*mLooper, removeFd(MockIncFs::kPendingReadsFd)).Times(1); TemporaryDir tempDir; - int storageId = - mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), {}, - IncrementalService::CreateOptions::CreateNew); + int storageId = mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), + IncrementalService::CreateOptions::CreateNew, + {}, {}, {}); ASSERT_GE(storageId, 0); mDataLoaderManager->setDataLoaderStatusUnavailable(); ASSERT_NE(nullptr, mLooper->mCallback); @@ -712,9 +727,9 @@ TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsSuccess) { // Not expecting callback removal. EXPECT_CALL(*mAppOpsManager, stopWatchingMode(_)).Times(0); TemporaryDir tempDir; - int storageId = - mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), {}, - IncrementalService::CreateOptions::CreateNew); + int storageId = mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), + IncrementalService::CreateOptions::CreateNew, + {}, {}, {}); ASSERT_GE(storageId, 0); ASSERT_GE(mDataLoader->setStorageParams(true), 0); } @@ -739,9 +754,9 @@ TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsSuccessAndPermissionChang // After callback is called, disable read logs and remove callback. EXPECT_CALL(*mAppOpsManager, stopWatchingMode(_)).Times(1); TemporaryDir tempDir; - int storageId = - mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), {}, - IncrementalService::CreateOptions::CreateNew); + int storageId = mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), + IncrementalService::CreateOptions::CreateNew, + {}, {}, {}); ASSERT_GE(storageId, 0); ASSERT_GE(mDataLoader->setStorageParams(true), 0); ASSERT_NE(nullptr, mAppOpsManager->mStoredCallback.get()); @@ -762,9 +777,9 @@ TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsCheckPermissionFails) { EXPECT_CALL(*mAppOpsManager, startWatchingMode(_, _, _)).Times(0); EXPECT_CALL(*mAppOpsManager, stopWatchingMode(_)).Times(0); TemporaryDir tempDir; - int storageId = - mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), {}, - IncrementalService::CreateOptions::CreateNew); + int storageId = mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), + IncrementalService::CreateOptions::CreateNew, + {}, {}, {}); ASSERT_GE(storageId, 0); ASSERT_LT(mDataLoader->setStorageParams(true), 0); } @@ -785,9 +800,9 @@ TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsFails) { EXPECT_CALL(*mAppOpsManager, startWatchingMode(_, _, _)).Times(0); EXPECT_CALL(*mAppOpsManager, stopWatchingMode(_)).Times(0); TemporaryDir tempDir; - int storageId = - mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), {}, - IncrementalService::CreateOptions::CreateNew); + int storageId = mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), + IncrementalService::CreateOptions::CreateNew, + {}, {}, {}); ASSERT_GE(storageId, 0); ASSERT_LT(mDataLoader->setStorageParams(true), 0); } @@ -799,9 +814,9 @@ TEST_F(IncrementalServiceTest, testMakeDirectory) { mDataLoaderManager->bindToDataLoaderSuccess(); mDataLoaderManager->getDataLoaderSuccess(); TemporaryDir tempDir; - int storageId = - mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), {}, - IncrementalService::CreateOptions::CreateNew); + int storageId = mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), + IncrementalService::CreateOptions::CreateNew, + {}, {}, {}); std::string dir_path("test"); // Expecting incfs to call makeDir on a path like: @@ -823,9 +838,9 @@ TEST_F(IncrementalServiceTest, testMakeDirectories) { mDataLoaderManager->bindToDataLoaderSuccess(); mDataLoaderManager->getDataLoaderSuccess(); TemporaryDir tempDir; - int storageId = - mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), {}, - IncrementalService::CreateOptions::CreateNew); + int storageId = mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), + IncrementalService::CreateOptions::CreateNew, + {}, {}, {}); auto first = "first"sv; auto second = "second"sv; auto third = "third"sv;