From 824b77d24bca5b4d529299db63787a860eb252e0 Mon Sep 17 00:00:00 2001 From: Alessio Balsini Date: Tue, 7 Jan 2020 17:28:38 +0000 Subject: [PATCH] Refactor update status management as protobuf Convert UpdateState and introduce SnapshotUpdateStatus to protobuf to simplify its access. In addition, SnapshotUpdateStatus also stores the sum of all the snapshot sector information. This additional data is used to improve the merge progress progress. Bug: 139088917 Test: manual OTA Change-Id: Ic777d50244c1afa1cdd75fe9b2ffc6dd9ba19ade Signed-off-by: Alessio Balsini --- fs_mgr/libsnapshot/Android.bp | 1 + .../android/snapshot/snapshot.proto | 46 +++++++++++ .../include/libsnapshot/snapshot.h | 32 +------- fs_mgr/libsnapshot/snapshot.cpp | 77 +++++++++++++++---- 4 files changed, 111 insertions(+), 45 deletions(-) diff --git a/fs_mgr/libsnapshot/Android.bp b/fs_mgr/libsnapshot/Android.bp index eadcecc5d..9e67c66b5 100644 --- a/fs_mgr/libsnapshot/Android.bp +++ b/fs_mgr/libsnapshot/Android.bp @@ -127,6 +127,7 @@ cc_library_static { "include_test", ], srcs: [ + "android/snapshot/snapshot.proto", "test_helpers.cpp", ], shared_libs: [ diff --git a/fs_mgr/libsnapshot/android/snapshot/snapshot.proto b/fs_mgr/libsnapshot/android/snapshot/snapshot.proto index 629c3a4bd..a3a518d0d 100644 --- a/fs_mgr/libsnapshot/android/snapshot/snapshot.proto +++ b/fs_mgr/libsnapshot/android/snapshot/snapshot.proto @@ -85,3 +85,49 @@ message SnapshotStatus { // This is non-zero when |state| == MERGING or MERGE_COMPLETED. uint64 metadata_sectors = 8; } + +// Next: 8 +enum UpdateState { + // No update or merge is in progress. + None = 0; + + // An update is applying; snapshots may already exist. + Initiated = 1; + + // An update is pending, but has not been successfully booted yet. + Unverified = 2; + + // The kernel is merging in the background. + Merging = 3; + + // Post-merge cleanup steps could not be completed due to a transient + // error, but the next reboot will finish any pending operations. + MergeNeedsReboot = 4; + + // Merging is complete, and needs to be acknowledged. + MergeCompleted = 5; + + // Merging failed due to an unrecoverable error. + MergeFailed = 6; + + // The update was implicitly cancelled, either by a rollback or a flash + // operation via fastboot. This state can only be returned by WaitForMerge. + Cancelled = 7; +}; + +// Next: 5 +message SnapshotUpdateStatus { + UpdateState state = 1; + + // Total number of sectors allocated in the COW files before performing the + // merge operation. This field is used to keep track of the total number + // of sectors modified to monitor and show the progress of the merge during + // an update. + uint64 sectors_allocated = 2; + + // Total number of sectors of all the snapshot devices. + uint64 total_sectors = 3; + + // Sectors allocated for metadata in all the snapshot devices. + uint64 metadata_sectors = 4; +} diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h index 6e613ba67..61946f706 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h @@ -26,6 +26,7 @@ #include #include +#include #include #include #include @@ -80,35 +81,6 @@ enum class CreateResult : unsigned int { NOT_CREATED, }; -enum class UpdateState : unsigned int { - // No update or merge is in progress. - None, - - // An update is applying; snapshots may already exist. - Initiated, - - // An update is pending, but has not been successfully booted yet. - Unverified, - - // The kernel is merging in the background. - Merging, - - // Post-merge cleanup steps could not be completed due to a transient - // error, but the next reboot will finish any pending operations. - MergeNeedsReboot, - - // Merging is complete, and needs to be acknowledged. - MergeCompleted, - - // Merging failed due to an unrecoverable error. - MergeFailed, - - // The update was implicitly cancelled, either by a rollback or a flash - // operation via fastboot. This state can only be returned by WaitForMerge. - Cancelled -}; -std::ostream& operator<<(std::ostream& os, UpdateState state); - class SnapshotManager final { using CreateLogicalPartitionParams = android::fs_mgr::CreateLogicalPartitionParams; using IPartitionOpener = android::fs_mgr::IPartitionOpener; @@ -433,7 +405,9 @@ class SnapshotManager final { // Interact with /metadata/ota/state. UpdateState ReadUpdateState(LockedFile* file); + SnapshotUpdateStatus ReadSnapshotUpdateStatus(LockedFile* file); bool WriteUpdateState(LockedFile* file, UpdateState state); + bool WriteSnapshotUpdateStatus(LockedFile* file, const SnapshotUpdateStatus& status); std::string GetStateFilePath() const; // Helpers for merging. diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp index b79b65c63..70a69a578 100644 --- a/fs_mgr/libsnapshot/snapshot.cpp +++ b/fs_mgr/libsnapshot/snapshot.cpp @@ -560,9 +560,26 @@ bool SnapshotManager::InitiateMerge() { } } + DmTargetSnapshot::Status initial_target_values = {}; + for (const auto& snapshot : snapshots) { + DmTargetSnapshot::Status current_status; + if (!QuerySnapshotStatus(snapshot, nullptr, ¤t_status)) { + return false; + } + initial_target_values.sectors_allocated += current_status.sectors_allocated; + initial_target_values.total_sectors += current_status.total_sectors; + initial_target_values.metadata_sectors += current_status.metadata_sectors; + } + + SnapshotUpdateStatus initial_status; + initial_status.set_state(UpdateState::Merging); + initial_status.set_sectors_allocated(initial_target_values.sectors_allocated); + initial_status.set_total_sectors(initial_target_values.total_sectors); + initial_status.set_metadata_sectors(initial_target_values.metadata_sectors); + // Point of no return - mark that we're starting a merge. From now on every // snapshot must be a merge target. - if (!WriteUpdateState(lock.get(), UpdateState::Merging)) { + if (!WriteSnapshotUpdateStatus(lock.get(), initial_status)) { return false; } @@ -1643,15 +1660,7 @@ std::unique_ptr SnapshotManager::LockExclusive() { return OpenLock(LOCK_EX); } -UpdateState SnapshotManager::ReadUpdateState(LockedFile* lock) { - CHECK(lock); - - std::string contents; - if (!android::base::ReadFileToString(GetStateFilePath(), &contents)) { - PLOG(ERROR) << "Read state file failed"; - return UpdateState::None; - } - +static UpdateState UpdateStateFromString(const std::string& contents) { if (contents.empty() || contents == "none") { return UpdateState::None; } else if (contents == "initiated") { @@ -1694,18 +1703,54 @@ std::ostream& operator<<(std::ostream& os, UpdateState state) { } } +UpdateState SnapshotManager::ReadUpdateState(LockedFile* lock) { + SnapshotUpdateStatus status = ReadSnapshotUpdateStatus(lock); + return status.state(); +} + +SnapshotUpdateStatus SnapshotManager::ReadSnapshotUpdateStatus(LockedFile* lock) { + CHECK(lock); + + SnapshotUpdateStatus status = {}; + std::string contents; + if (!android::base::ReadFileToString(GetStateFilePath(), &contents)) { + PLOG(ERROR) << "Read state file failed"; + status.set_state(UpdateState::None); + return status; + } + + if (!status.ParseFromString(contents)) { + LOG(WARNING) << "Unable to parse state file as SnapshotUpdateStatus, using the old format"; + + // Try to rollback to legacy file to support devices that are + // currently using the old file format. + // TODO(b/147409432) + status.set_state(UpdateStateFromString(contents)); + } + + return status; +} + bool SnapshotManager::WriteUpdateState(LockedFile* lock, UpdateState state) { + SnapshotUpdateStatus status = {}; + status.set_state(state); + return WriteSnapshotUpdateStatus(lock, status); +} + +bool SnapshotManager::WriteSnapshotUpdateStatus(LockedFile* lock, + const SnapshotUpdateStatus& status) { CHECK(lock); CHECK(lock->lock_mode() == LOCK_EX); - std::stringstream ss; - ss << state; - std::string contents = ss.str(); - if (contents.empty()) return false; + std::string contents; + if (!status.SerializeToString(&contents)) { + LOG(ERROR) << "Unable to serialize SnapshotUpdateStatus."; + return false; + } #ifdef LIBSNAPSHOT_USE_HAL auto merge_status = MergeStatus::UNKNOWN; - switch (state) { + switch (status.state()) { // The needs-reboot and completed cases imply that /data and /metadata // can be safely wiped, so we don't report a merge status. case UpdateState::None: @@ -1724,7 +1769,7 @@ bool SnapshotManager::WriteUpdateState(LockedFile* lock, UpdateState state) { default: // Note that Cancelled flows to here - it is never written, since // it only communicates a transient state to the caller. - LOG(ERROR) << "Unexpected update status: " << state; + LOG(ERROR) << "Unexpected update status: " << status.state(); break; }