Lifecycle: detecting blocked and unhealthy.
Part 1: interfaces and PM implementation.
Bug: 153874006
Test: atest PackageManagerShellCommandTest PackageManagerShellCommandIncrementalTest IncrementalServiceTest
Change-Id: I312dd919d2bb552bea3d72fb49fd1579882da14b
diff --git a/Android.bp b/Android.bp
index ee381a4..01a43b6 100644
--- a/Android.bp
+++ b/Android.bp
@@ -930,6 +930,8 @@
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 25cb040..4c57570 100644
--- a/core/java/android/os/incremental/IIncrementalService.aidl
+++ b/core/java/android/os/incremental/IIncrementalService.aidl
@@ -19,6 +19,8 @@
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 @@
* 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 0000000..9f93ede
--- /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 958c7fb..863d86ef 100644
--- a/core/java/android/os/incremental/IncrementalFileStorages.java
+++ b/core/java/android/os/incremental/IncrementalFileStorages.java
@@ -65,7 +65,9 @@
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<InstallationFileParcel> addedFiles) throws IOException {
// TODO(b/136132412): sanity check if session should not be incremental
IncrementalManager incrementalManager = (IncrementalManager) context.getSystemService(
@@ -75,9 +77,9 @@
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 @@
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 @@
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 916edfa..c7f50c9 100644
--- a/core/java/android/os/incremental/IncrementalManager.java
+++ b/core/java/android/os/incremental/IncrementalManager.java
@@ -110,11 +110,15 @@
*/
@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 0000000..6839317
--- /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 f8278de..7ab05c4 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.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 @@
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 @@
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 @@
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 @@
if (mDestroyed || mDataLoaderFinished) {
switch (status) {
case IDataLoaderStatusListener.DATA_LOADER_UNRECOVERABLE:
- onDataLoaderUnrecoverable();
+ onStorageUnhealthy();
return;
}
return;
@@ -2840,9 +2846,49 @@
};
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 @@
}
}
- 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 8476674..6018b9e 100644
--- a/services/incremental/BinderIncrementalService.cpp
+++ b/services/incremental/BinderIncrementalService.cpp
@@ -118,14 +118,18 @@
}
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<content::pm::DataLoaderParamsParcel&&>(params),
- listener,
- android::incremental::IncrementalService::CreateOptions(
- createMode));
+ android::incremental::IncrementalService::CreateOptions(createMode),
+ statusListener,
+ const_cast<StorageHealthCheckParams&&>(healthCheckParams),
+ healthListener);
return ok();
}
diff --git a/services/incremental/BinderIncrementalService.h b/services/incremental/BinderIncrementalService.h
index 5a7d5da..af11363 100644
--- a/services/incremental/BinderIncrementalService.h
+++ b/services/incremental/BinderIncrementalService.h
@@ -41,8 +41,11 @@
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<IStorageHealthListener>& 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 @@
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 f0dca77..b03d1ea 100644
--- a/services/incremental/IncrementalService.cpp
+++ b/services/incremental/IncrementalService.cpp
@@ -410,9 +410,12 @@
}
}
-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 @@
// 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 @@
dataLoaderParams.arguments = loader.arguments();
}
- prepareDataLoader(*ifs, std::move(dataLoaderParams), nullptr);
+ prepareDataLoader(*ifs, std::move(dataLoaderParams));
CHECK(ifs->dataLoaderStub);
std::vector<std::pair<std::string, metadata::BindPoint>> bindPoints;
@@ -1338,14 +1341,18 @@
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 @@
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 <class Duration>
@@ -1680,19 +1688,24 @@
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 @@
mStatusCondition.wait_until(lock, now + 60s, [this] {
return mCurrentStatus == IDataLoaderStatusListener::DATA_LOADER_DESTROYED;
});
- mListener = {};
+ mStatusListener = {};
+ mHealthListener = {};
mId = kInvalidStorageId;
}
sp<content::pm::IDataLoader> IncrementalService::DataLoaderStub::getDataLoader() {
sp<IDataLoader> 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 @@
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 @@
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 @@
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::destroy() {
- return mService.mDataLoaderManager->unbindFromDataLoader(mId).isOk();
+ return mService.mDataLoaderManager->unbindFromDataLoader(id()).isOk();
}
bool IncrementalService::DataLoaderStub::fsmStep() {
@@ -1852,8 +1866,8 @@
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 @@
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 @@
}
}
- 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 f3fde2a..bde4ef6 100644
--- a/services/incremental/IncrementalService.h
+++ b/services/incremental/IncrementalService.h
@@ -21,6 +21,8 @@
#include <android/content/pm/FileSystemControlParcel.h>
#include <android/content/pm/IDataLoaderStatusListener.h>
#include <android/os/incremental/BnIncrementalServiceConnector.h>
+#include <android/os/incremental/BnStorageHealthListener.h>
+#include <android/os/incremental/StorageHealthCheckParams.h>
#include <binder/IAppOpsCallback.h>
#include <utils/String16.h>
#include <utils/StrongPointer.h>
@@ -56,10 +58,15 @@
using Clock = std::chrono::steady_clock;
using TimePoint = std::chrono::time_point<Clock>;
using Seconds = std::chrono::seconds;
+using BootClockTsUs = uint64_t;
using IDataLoaderStatusListener = ::android::content::pm::IDataLoaderStatusListener;
using DataLoaderStatusListener = ::android::sp<IDataLoaderStatusListener>;
+using StorageHealthCheckParams = ::android::os::incremental::StorageHealthCheckParams;
+using IStorageHealthListener = ::android::os::incremental::IStorageHealthListener;
+using StorageHealthListener = ::android::sp<IStorageHealthListener>;
+
class IncrementalService final {
public:
explicit IncrementalService(ServiceManagerWrapper&& sm, std::string_view rootDir);
@@ -72,6 +79,8 @@
static constexpr StorageId kInvalidStorageId = -1;
static constexpr StorageId kMaxStorageId = std::numeric_limits<int>::max();
+ static constexpr BootClockTsUs kMaxBootClockTsUs = std::numeric_limits<BootClockTsUs>::max();
+
enum CreateOptions {
TemporaryBind = 1,
PermanentBind = 2,
@@ -97,8 +106,9 @@
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 @@
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 @@
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 @@
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 08fb486..a76aa62 100644
--- a/services/incremental/ServiceWrappers.cpp
+++ b/services/incremental/ServiceWrappers.cpp
@@ -175,6 +175,10 @@
ErrorCode writeBlocks(std::span<const incfs::DataBlock> blocks) const final {
return incfs::writeBlocks({blocks.data(), size_t(blocks.size())});
}
+ WaitResult waitForPendingReads(const Control& control, std::chrono::milliseconds timeout,
+ std::vector<incfs::ReadInfo>* pendingReadsBuffer) const final {
+ return incfs::waitForPendingReads(control, timeout, pendingReadsBuffer);
+ }
};
RealServiceManager::RealServiceManager(sp<IServiceManager> serviceManager, JNIEnv* env)
diff --git a/services/incremental/ServiceWrappers.h b/services/incremental/ServiceWrappers.h
index abbf2f4..a935ab9 100644
--- a/services/incremental/ServiceWrappers.h
+++ b/services/incremental/ServiceWrappers.h
@@ -69,6 +69,7 @@
using Control = incfs::Control;
using FileId = incfs::FileId;
using ErrorCode = incfs::ErrorCode;
+ using WaitResult = incfs::WaitResult;
using ExistingMountCallback =
std::function<void(std::string_view root, std::string_view backingDir,
@@ -90,6 +91,9 @@
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(std::span<const incfs::DataBlock> blocks) const = 0;
+ virtual WaitResult waitForPendingReads(
+ const Control& control, std::chrono::milliseconds timeout,
+ std::vector<incfs::ReadInfo>* pendingReadsBuffer) const = 0;
};
class AppOpsManagerWrapper {
diff --git a/services/incremental/test/IncrementalServiceTest.cpp b/services/incremental/test/IncrementalServiceTest.cpp
index 2e4625c..2948b6a 100644
--- a/services/incremental/test/IncrementalServiceTest.cpp
+++ b/services/incremental/test/IncrementalServiceTest.cpp
@@ -284,6 +284,9 @@
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<const DataBlock> blocks));
+ MOCK_CONST_METHOD3(waitForPendingReads,
+ WaitResult(const Control& control, std::chrono::milliseconds timeout,
+ std::vector<incfs::ReadInfo>* pendingReadsBuffer));
MockIncFs() { ON_CALL(*this, listExistingMounts(_)).WillByDefault(Return()); }
@@ -292,12 +295,23 @@
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<incfs::ReadInfo>* 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
mVold->mountIncFsSuccess();
mIncFs->makeFileSuccess();
mIncFs->openMountSuccess();
+ mIncFs->waitForPendingReadsSuccess();
mVold->bindMountSuccess();
mDataLoader->initializeCreateOkNoStatus();
mDataLoaderManager->bindToDataLoaderSuccess();
@@ -685,9 +700,9 @@
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 @@
// 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 @@
// 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 @@
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 @@
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 @@
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 @@
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;