From 3c0fea923b233641640b73b6f3593e6f5509bd91 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Fri, 9 Aug 2019 16:26:31 -0700 Subject: [PATCH] libsnapshot: Track the source slot across reboots. The purpose of this is twofold. First, we need a way to detect whether the device has booted into the new slot after an update. Second, we need a quick-and-dirty test for first-stage init so it can avoid searching for snapshots if it doesn't need to. We can solve both of these problems by storing the source slot in /metadata/ota/snapshot-boot. The existence of the file implies a more complex check is needed, and the file stores the source slot. Test: libsnapshot_test gtest Bug: 139204329 Bug: 139089801 Change-Id: I919dd1c27a65734c61f3a9bdffefb32bdebd723b --- fs_mgr/libsnapshot/Android.bp | 1 + .../include/libsnapshot/snapshot.h | 14 +++-- fs_mgr/libsnapshot/snapshot.cpp | 59 ++++++++++++++++--- fs_mgr/libsnapshot/snapshot_test.cpp | 27 ++++----- 4 files changed, 71 insertions(+), 30 deletions(-) diff --git a/fs_mgr/libsnapshot/Android.bp b/fs_mgr/libsnapshot/Android.bp index 52aad1207..746987e2f 100644 --- a/fs_mgr/libsnapshot/Android.bp +++ b/fs_mgr/libsnapshot/Android.bp @@ -33,6 +33,7 @@ cc_defaults { "libext2_uuid", "libext4_utils", "libfiemap", + "libfstab", ], export_include_dirs: ["include"], } diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h index f7608dc5f..4fb6808a9 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h @@ -71,11 +71,7 @@ class SnapshotManager final { virtual ~IDeviceInfo() {} virtual std::string GetGsidDir() const = 0; virtual std::string GetMetadataDir() const = 0; - - // Return true if the device is currently running off snapshot devices, - // indicating that we have booted after applying (but not merging) an - // OTA. - virtual bool IsRunningSnapshot() const = 0; + virtual std::string GetSlotSuffix() const = 0; }; ~SnapshotManager(); @@ -93,6 +89,11 @@ class SnapshotManager final { // state != Initiated or None. bool CancelUpdate(); + // Mark snapshot writes as having completed. After this, new snapshots cannot + // be created, and the device must either cancel the OTA (either before + // rebooting or after rolling back), or merge the OTA. + bool FinishedSnapshotWrites(); + // Initiate a merge on all snapshot devices. This should only be used after an // update has been marked successful after booting. bool InitiateMerge(); @@ -261,6 +262,9 @@ class SnapshotManager final { bool ReadSnapshotStatus(LockedFile* lock, const std::string& name, SnapshotStatus* status); std::string GetSnapshotStatusFilePath(const std::string& name); + std::string GetSnapshotBootIndicatorPath(); + void RemoveSnapshotBootIndicator(); + // Return the name of the device holding the "snapshot" or "snapshot-merge" // target. This may not be the final device presented via MapSnapshot(), if // for example there is a linear segment. diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp index 63a01f36c..75a1f265f 100644 --- a/fs_mgr/libsnapshot/snapshot.cpp +++ b/fs_mgr/libsnapshot/snapshot.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -48,18 +49,15 @@ using namespace std::string_literals; // Unit is sectors, this is a 4K chunk. static constexpr uint32_t kSnapshotChunkSize = 8; +static constexpr char kSnapshotBootIndicatorFile[] = "snapshot-boot"; + class DeviceInfo final : public SnapshotManager::IDeviceInfo { public: std::string GetGsidDir() const override { return "ota"s; } std::string GetMetadataDir() const override { return "/metadata/ota"s; } - bool IsRunningSnapshot() const override; + std::string GetSlotSuffix() const override { return fs_mgr_get_slot_suffix(); } }; -bool DeviceInfo::IsRunningSnapshot() const { - // :TODO: implement this check. - return true; -} - // Note: IIMageManager is an incomplete type in the header, so the default // destructor doesn't work. SnapshotManager::~SnapshotManager() {} @@ -115,6 +113,27 @@ bool SnapshotManager::CancelUpdate() { return true; } +bool SnapshotManager::FinishedSnapshotWrites() { + auto lock = LockExclusive(); + if (!lock) return false; + + if (ReadUpdateState(lock.get()) != UpdateState::Initiated) { + LOG(ERROR) << "Can only transition to the Unverified state from the Initiated state."; + return false; + } + + // This file acts as both a quick indicator for init (it can use access(2) + // to decide how to do first-stage mounts), and it stores the old slot, so + // we can tell whether or not we performed a rollback. + auto contents = device_->GetSlotSuffix(); + auto boot_file = GetSnapshotBootIndicatorPath(); + if (!android::base::WriteStringToFile(contents, boot_file)) { + PLOG(ERROR) << "write failed: " << boot_file; + return false; + } + return WriteUpdateState(lock.get(), UpdateState::Unverified); +} + bool SnapshotManager::CreateSnapshot(LockedFile* lock, const std::string& name, uint64_t device_size, uint64_t snapshot_size, uint64_t cow_size) { @@ -339,8 +358,16 @@ bool SnapshotManager::InitiateMerge() { LOG(ERROR) << "Cannot begin a merge if an update has not been verified"; return false; } - if (!device_->IsRunningSnapshot()) { - LOG(ERROR) << "Cannot begin a merge if the device is not booted off a snapshot"; + + std::string old_slot; + auto boot_file = GetSnapshotBootIndicatorPath(); + if (!android::base::ReadFileToString(boot_file, &old_slot)) { + LOG(ERROR) << "Could not determine the previous slot; aborting merge"; + return false; + } + auto new_slot = device_->GetSlotSuffix(); + if (new_slot == old_slot) { + LOG(ERROR) << "Device cannot merge while booting off old slot " << old_slot; return false; } @@ -676,7 +703,23 @@ UpdateState SnapshotManager::CheckTargetMergeState(LockedFile* lock, const std:: return UpdateState::MergeCompleted; } +std::string SnapshotManager::GetSnapshotBootIndicatorPath() { + return metadata_dir_ + "/" + kSnapshotBootIndicatorFile; +} + +void SnapshotManager::RemoveSnapshotBootIndicator() { + // It's okay if this fails - first-stage init performs a deeper check after + // reading the indicator file, so it's not a problem if it still exists + // after the update completes. + auto boot_file = GetSnapshotBootIndicatorPath(); + if (unlink(boot_file.c_str()) == -1 && errno != ENOENT) { + PLOG(ERROR) << "unlink " << boot_file; + } +} + void SnapshotManager::AcknowledgeMergeSuccess(LockedFile* lock) { + RemoveSnapshotBootIndicator(); + if (!WriteUpdateState(lock, UpdateState::None)) { // We'll try again next reboot, ad infinitum. return; diff --git a/fs_mgr/libsnapshot/snapshot_test.cpp b/fs_mgr/libsnapshot/snapshot_test.cpp index 4903224c0..34ea331ef 100644 --- a/fs_mgr/libsnapshot/snapshot_test.cpp +++ b/fs_mgr/libsnapshot/snapshot_test.cpp @@ -43,12 +43,12 @@ class TestDeviceInfo : public SnapshotManager::IDeviceInfo { public: std::string GetGsidDir() const override { return "ota/test"s; } std::string GetMetadataDir() const override { return "/metadata/ota/test"s; } - bool IsRunningSnapshot() const override { return is_running_snapshot_; } + std::string GetSlotSuffix() const override { return slot_suffix_; } - void set_is_running_snapshot(bool value) { is_running_snapshot_ = value; } + void set_slot_suffix(const std::string& suffix) { slot_suffix_ = suffix; } private: - bool is_running_snapshot_; + std::string slot_suffix_; }; std::unique_ptr sm; @@ -60,7 +60,7 @@ class SnapshotTest : public ::testing::Test { protected: void SetUp() override { - test_device->set_is_running_snapshot(false); + test_device->set_slot_suffix("_a"); if (sm->GetUpdateState() != UpdateState::None) { CleanupTestArtifacts(); @@ -189,15 +189,9 @@ TEST_F(SnapshotTest, MapPartialSnapshot) { } TEST_F(SnapshotTest, NoMergeBeforeReboot) { - ASSERT_TRUE(AcquireLock()); + ASSERT_TRUE(sm->FinishedSnapshotWrites()); - // Set the state to Unverified, as if we finished an update. - ASSERT_TRUE(sm->WriteUpdateState(lock_.get(), UpdateState::Unverified)); - - // Release the lock. - lock_ = nullptr; - - // Merge should fail, since we didn't mark the device as rebooted. + // Merge should fail, since the slot hasn't changed. ASSERT_FALSE(sm->InitiateMerge()); } @@ -231,7 +225,7 @@ TEST_F(SnapshotTest, Merge) { // Release the lock. lock_ = nullptr; - test_device->set_is_running_snapshot(true); + test_device->set_slot_suffix("_b"); ASSERT_TRUE(sm->InitiateMerge()); // The device should have been switched to a snapshot-merge target. @@ -273,13 +267,12 @@ TEST_F(SnapshotTest, MergeCannotRemoveCow) { unique_fd fd(open(cow_path.c_str(), O_RDONLY | O_CLOEXEC)); ASSERT_GE(fd, 0); - // Set the state to Unverified, as if we finished an update. - ASSERT_TRUE(sm->WriteUpdateState(lock_.get(), UpdateState::Unverified)); - // Release the lock. lock_ = nullptr; - test_device->set_is_running_snapshot(true); + ASSERT_TRUE(sm->FinishedSnapshotWrites()); + + test_device->set_slot_suffix("_b"); ASSERT_TRUE(sm->InitiateMerge()); // COW cannot be removed due to open fd, so expect a soft failure.