[incfs] Call the new isFullyLoaded api where possible
Replace the remaining calls to getFilledRanges() with
isFullyLoaded() where we don't care about the progress
Bug: 183067554
Test: atest IncrementalService
Change-Id: Ic8dc2e3a0ef078353883feef7969b29e11dfa2d0
diff --git a/services/incremental/BinderIncrementalService.cpp b/services/incremental/BinderIncrementalService.cpp
index 5e5be25..052b4dd 100644
--- a/services/incremental/BinderIncrementalService.cpp
+++ b/services/incremental/BinderIncrementalService.cpp
@@ -266,8 +266,7 @@
binder::Status BinderIncrementalService::getLoadingProgress(int32_t storageId,
float* _aidl_return) {
- *_aidl_return =
- mImpl.getLoadingProgress(storageId, /*stopOnFirstIncomplete=*/false).getProgress();
+ *_aidl_return = mImpl.getLoadingProgress(storageId).getProgress();
return ok();
}
diff --git a/services/incremental/IncrementalService.cpp b/services/incremental/IncrementalService.cpp
index 34253f9..958eb15 100644
--- a/services/incremental/IncrementalService.cpp
+++ b/services/incremental/IncrementalService.cpp
@@ -247,14 +247,17 @@
}
template <class Func>
-static auto makeCleanup(Func&& f) {
+static auto makeCleanup(Func&& f) requires(!std::is_lvalue_reference_v<Func>) {
auto deleter = [f = std::move(f)](auto) { f(); };
// &f is a dangling pointer here, but we actually never use it as deleter moves it in.
return std::unique_ptr<Func, decltype(deleter)>(&f, std::move(deleter));
}
-static std::unique_ptr<DIR, decltype(&::closedir)> openDir(const char* dir) {
- return {::opendir(dir), ::closedir};
+static auto openDir(const char* dir) {
+ struct DirCloser {
+ void operator()(DIR* d) const noexcept { ::closedir(d); }
+ };
+ return std::unique_ptr<DIR, DirCloser>(::opendir(dir));
}
static auto openDir(std::string_view dir) {
@@ -390,9 +393,7 @@
dprintf(fd, " storages (%d): {\n", int(mnt.storages.size()));
for (auto&& [storageId, storage] : mnt.storages) {
dprintf(fd, " [%d] -> [%s] (%d %% loaded) \n", storageId, storage.name.c_str(),
- (int)(getLoadingProgressFromPath(mnt, storage.name.c_str(),
- /*stopOnFirstIncomplete=*/false)
- .getProgress() *
+ (int)(getLoadingProgressFromPath(mnt, storage.name.c_str()).getProgress() *
100));
}
dprintf(fd, " }\n");
@@ -425,21 +426,7 @@
return true;
}
- // Check all permanent binds.
- for (auto&& [_, bindPoint] : ifs.bindPoints) {
- if (bindPoint.kind != BindKind::Permanent) {
- continue;
- }
- const auto progress = getLoadingProgressFromPath(ifs, bindPoint.sourceDir,
- /*stopOnFirstIncomplete=*/true);
- if (!progress.isError() && !progress.fullyLoaded()) {
- LOG(INFO) << "Non system mount: [" << bindPoint.sourceDir
- << "], partial progress: " << progress.getProgress() * 100 << "%";
- return true;
- }
- }
-
- return false;
+ return mIncFs->isEverythingFullyLoaded(ifs.control) == incfs::LoadingState::MissingBlocks;
}
void IncrementalService::onSystemReady() {
@@ -576,7 +563,7 @@
std::make_shared<IncFsMount>(std::move(mountRoot), mountId, std::move(control), *this);
// Now it's the |ifs|'s responsibility to clean up after itself, and the only cleanup we need
// is the removal of the |ifs|.
- firstCleanupOnFailure.release();
+ (void)firstCleanupOnFailure.release();
auto secondCleanup = [this, &l](auto itPtr) {
if (!l.owns_lock()) {
@@ -622,7 +609,7 @@
}
// Done here as well, all data structures are in good state.
- secondCleanupOnFailure.release();
+ (void)secondCleanupOnFailure.release();
mountIt->second = std::move(ifs);
l.unlock();
@@ -840,7 +827,7 @@
}
std::lock_guard l(mMountOperationLock);
- const auto status = mVold->setIncFsMountOptions(control, enableReadLogs);
+ auto status = mVold->setIncFsMountOptions(control, enableReadLogs);
if (status.isOk()) {
// Store enabled state.
ifs.setReadLogsEnabled(enableReadLogs);
@@ -1257,13 +1244,13 @@
}
// Still loading?
- const auto progress = getLoadingProgress(storage, /*stopOnFirstIncomplete=*/true);
- if (progress.isError()) {
+ const auto state = isMountFullyLoaded(storage);
+ if (int(state) < 0) {
// Something is wrong, abort.
return clearUidReadTimeouts(storage);
}
- if (progress.started() && progress.fullyLoaded()) {
+ if (state == incfs::LoadingState::Full) {
// Fully loaded, check readLogs collection.
const auto ifs = getIfs(storage);
if (!ifs->readLogsEnabled()) {
@@ -1350,7 +1337,7 @@
auto ifs = std::make_shared<IncFsMount>(std::string(expectedRoot), mountId,
std::move(control), *this);
- cleanupFiles.release(); // ifs will take care of that now
+ (void)cleanupFiles.release(); // ifs will take care of that now
// Check if marker file present.
if (checkReadLogsDisabledMarker(root)) {
@@ -1455,7 +1442,7 @@
}
mVold->unmountIncFs(std::string(target));
}
- cleanupMounts.release(); // ifs now manages everything
+ (void)cleanupMounts.release(); // ifs now manages everything
if (ifs->bindPoints.empty()) {
LOG(WARNING) << "No valid bind points for mount " << expectedRoot;
@@ -2007,7 +1994,7 @@
}
IncrementalService::LoadingProgress IncrementalService::getLoadingProgress(
- StorageId storage, bool stopOnFirstIncomplete) const {
+ StorageId storage) const {
std::unique_lock l(mLock);
const auto ifs = getIfsLocked(storage);
if (!ifs) {
@@ -2020,12 +2007,11 @@
return {-EINVAL, -EINVAL};
}
l.unlock();
- return getLoadingProgressFromPath(*ifs, storageInfo->second.name, stopOnFirstIncomplete);
+ return getLoadingProgressFromPath(*ifs, storageInfo->second.name);
}
IncrementalService::LoadingProgress IncrementalService::getLoadingProgressFromPath(
- const IncFsMount& ifs, std::string_view storagePath,
- const bool stopOnFirstIncomplete) const {
+ const IncFsMount& ifs, std::string_view storagePath) const {
ssize_t totalBlocks = 0, filledBlocks = 0, error = 0;
mFs->listFilesRecursive(storagePath, [&, this](auto filePath) {
const auto [filledBlocksCount, totalBlocksCount] =
@@ -2038,15 +2024,12 @@
}
if (filledBlocksCount < 0) {
LOG(ERROR) << "getLoadingProgress failed to get filled blocks count for: " << filePath
- << " errno: " << filledBlocksCount;
+ << ", errno: " << filledBlocksCount;
error = filledBlocksCount;
return false;
}
totalBlocks += totalBlocksCount;
filledBlocks += filledBlocksCount;
- if (stopOnFirstIncomplete && filledBlocks < totalBlocks) {
- return false;
- }
return true;
});
@@ -2055,7 +2038,7 @@
bool IncrementalService::updateLoadingProgress(
StorageId storage, const StorageLoadingProgressListener& progressListener) {
- const auto progress = getLoadingProgress(storage, /*stopOnFirstIncomplete=*/false);
+ const auto progress = getLoadingProgress(storage);
if (progress.isError()) {
// Failed to get progress from incfs, abort.
return false;
@@ -2592,7 +2575,7 @@
mHealthCheckParams.blockedTimeoutMs < mHealthCheckParams.unhealthyTimeoutMs;
}
-void IncrementalService::DataLoaderStub::onHealthStatus(StorageHealthListener healthListener,
+void IncrementalService::DataLoaderStub::onHealthStatus(const StorageHealthListener& healthListener,
int healthStatus) {
LOG(DEBUG) << id() << ": healthStatus: " << healthStatus;
if (healthListener) {
diff --git a/services/incremental/IncrementalService.h b/services/incremental/IncrementalService.h
index aa80bd4..c7c682f2 100644
--- a/services/incremental/IncrementalService.h
+++ b/services/incremental/IncrementalService.h
@@ -167,7 +167,7 @@
incfs::LoadingState isFileFullyLoaded(StorageId storage, std::string_view filePath) const;
incfs::LoadingState isMountFullyLoaded(StorageId storage) const;
- LoadingProgress getLoadingProgress(StorageId storage, bool stopOnFirstIncomplete) const;
+ LoadingProgress getLoadingProgress(StorageId storage) const;
bool registerLoadingProgressListener(StorageId storage,
const StorageLoadingProgressListener& progressListener);
@@ -255,7 +255,7 @@
bool fsmStep();
- void onHealthStatus(StorageHealthListener healthListener, int healthStatus);
+ void onHealthStatus(const StorageHealthListener& healthListener, int healthStatus);
void updateHealthStatus(bool baseline = false);
bool isValid() const { return id() != kInvalidStorageId; }
@@ -413,8 +413,7 @@
int setStorageParams(IncFsMount& ifs, StorageId storageId, bool enableReadLogs);
binder::Status applyStorageParams(IncFsMount& ifs, bool enableReadLogs);
- LoadingProgress getLoadingProgressFromPath(const IncFsMount& ifs, std::string_view path,
- bool stopOnFirstIncomplete) const;
+ LoadingProgress getLoadingProgressFromPath(const IncFsMount& ifs, std::string_view path) const;
int setFileContent(const IfsMountPtr& ifs, const incfs::FileId& fileId,
std::string_view debugFilePath, std::span<const uint8_t> data) const;
diff --git a/services/incremental/test/IncrementalServiceTest.cpp b/services/incremental/test/IncrementalServiceTest.cpp
index fb3a8a0..54bc95d 100644
--- a/services/incremental/test/IncrementalServiceTest.cpp
+++ b/services/incremental/test/IncrementalServiceTest.cpp
@@ -1606,9 +1606,7 @@
int storageId =
mIncrementalService->createStorage(tempDir.path, mDataLoaderParcel,
IncrementalService::CreateOptions::CreateNew);
- ASSERT_EQ(1,
- mIncrementalService->getLoadingProgress(storageId, /*stopOnFirstIncomplete=*/false)
- .getProgress());
+ ASSERT_EQ(1, mIncrementalService->getLoadingProgress(storageId).getProgress());
}
TEST_F(IncrementalServiceTest, testGetLoadingProgressFailsWithFailedRanges) {
@@ -1620,9 +1618,7 @@
mIncrementalService->createStorage(tempDir.path, mDataLoaderParcel,
IncrementalService::CreateOptions::CreateNew);
EXPECT_CALL(*mIncFs, countFilledBlocks(_, _)).Times(1);
- ASSERT_EQ(-1,
- mIncrementalService->getLoadingProgress(storageId, /*stopOnFirstIncomplete=*/false)
- .getProgress());
+ ASSERT_EQ(-1, mIncrementalService->getLoadingProgress(storageId).getProgress());
}
TEST_F(IncrementalServiceTest, testGetLoadingProgressSuccessWithEmptyRanges) {
@@ -1634,9 +1630,7 @@
mIncrementalService->createStorage(tempDir.path, mDataLoaderParcel,
IncrementalService::CreateOptions::CreateNew);
EXPECT_CALL(*mIncFs, countFilledBlocks(_, _)).Times(3);
- ASSERT_EQ(1,
- mIncrementalService->getLoadingProgress(storageId, /*stopOnFirstIncomplete=*/false)
- .getProgress());
+ ASSERT_EQ(1, mIncrementalService->getLoadingProgress(storageId).getProgress());
}
TEST_F(IncrementalServiceTest, testGetLoadingProgressSuccess) {
@@ -1648,9 +1642,7 @@
mIncrementalService->createStorage(tempDir.path, mDataLoaderParcel,
IncrementalService::CreateOptions::CreateNew);
EXPECT_CALL(*mIncFs, countFilledBlocks(_, _)).Times(3);
- ASSERT_EQ(0.5,
- mIncrementalService->getLoadingProgress(storageId, /*stopOnFirstIncomplete=*/false)
- .getProgress());
+ ASSERT_EQ(0.5, mIncrementalService->getLoadingProgress(storageId).getProgress());
}
TEST_F(IncrementalServiceTest, testRegisterLoadingProgressListenerSuccess) {
@@ -1833,8 +1825,12 @@
.WillOnce(Invoke(&checkPerUidTimeoutsEmpty));
EXPECT_CALL(*mTimedQueue, addJob(_, _, _)).Times(3);
- // Empty storage.
- mIncFs->countFilledBlocksEmpty();
+ // Loading storage.
+ EXPECT_CALL(*mIncFs, isEverythingFullyLoaded(_))
+ .WillOnce(Return(incfs::LoadingState::MissingBlocks))
+ .WillOnce(Return(incfs::LoadingState::MissingBlocks))
+ .WillOnce(Return(incfs::LoadingState::Full))
+ .WillOnce(Return(incfs::LoadingState::Full));
// Mark DataLoader as 'system' so that readlogs don't pollute the timed queue.
mDataLoaderParcel.packageName = "android";
@@ -1855,22 +1851,18 @@
const auto timedCallback = mTimedQueue->mWhat;
mTimedQueue->clearJob(storageId);
- // Still loading.
- mIncFs->countFilledBlocksSuccess();
-
// Call it again.
timedCallback();
}
{
- // Still present -> 0.5 progress.
+ // Still present -> some progress.
ASSERT_EQ(storageId, mTimedQueue->mId);
ASSERT_GE(mTimedQueue->mAfter, std::chrono::seconds(1));
const auto timedCallback = mTimedQueue->mWhat;
mTimedQueue->clearJob(storageId);
// Fully loaded but readlogs collection enabled.
- mIncFs->countFilledBlocksFullyLoaded();
ASSERT_GE(mDataLoader->setStorageParams(true), 0);
// Call it again.