From dfe6d07a4bb9735dd97152e06ccd2da44d95de7d Mon Sep 17 00:00:00 2001 From: David Anderson Date: Wed, 9 Oct 2019 16:24:03 -0700 Subject: [PATCH] Integrate libsnapshot with the boot control HAL. This patch translates UpdateState states into a MergeStatus from IBootControl 1.1, and asks the HAL to store it. Unfortunately this patch has to work around a few issues. The first issue is that Soong doesn't allow including only the headers from a HAL. The second issue is that entraining the headers requires linking to libraries that would otherwise not be needed in init. To address this, we now have three ways of linking to libsnapshot: 1. libsnapshot - Has access to gsid and HALs. 2. libsnapshot_nobinder - Has access to HALs, but not binder (for recovery). 3. libsnapshot_init - Does not use binder or HALs. The HAL code is #ifdef'd behind LIBSNAPSHOT_USE_HAL and we make use of forward declarations and dependency injection to minimize its spread. Bug: 139154945 Test: libsnapshot_test gtest Change-Id: I21ffd8a79a43d0589f2f71f346ac1b019584a183 --- fs_mgr/libsnapshot/Android.bp | 40 +++++++++++- .../include/libsnapshot/snapshot.h | 13 ++++ fs_mgr/libsnapshot/snapshot.cpp | 61 +++++++++++++++++++ fs_mgr/libsnapshot/snapshot_test.cpp | 25 ++++++++ fs_mgr/libsnapshot/test_helpers.h | 8 +++ init/Android.bp | 2 +- init/Android.mk | 2 +- 7 files changed, 146 insertions(+), 5 deletions(-) diff --git a/fs_mgr/libsnapshot/Android.bp b/fs_mgr/libsnapshot/Android.bp index 8cf0f3b4e..ba43949f9 100644 --- a/fs_mgr/libsnapshot/Android.bp +++ b/fs_mgr/libsnapshot/Android.bp @@ -28,6 +28,7 @@ cc_defaults { "liblp", ], static_libs: [ + "libcutils", "libdm", "libfs_mgr", "libfstab", @@ -56,6 +57,17 @@ cc_defaults { }, } +cc_defaults { + name: "libsnapshot_hal_deps", + cflags: [ + "-DLIBSNAPSHOT_USE_HAL", + ], + shared_libs: [ + "android.hardware.boot@1.0", + "android.hardware.boot@1.1", + ], +} + filegroup { name: "libsnapshot_sources", srcs: [ @@ -75,7 +87,10 @@ cc_library_headers { cc_library_static { name: "libsnapshot", - defaults: ["libsnapshot_defaults"], + defaults: [ + "libsnapshot_defaults", + "libsnapshot_hal_deps", + ], srcs: [":libsnapshot_sources"], whole_static_libs: [ "libfiemap_binder", @@ -83,7 +98,7 @@ cc_library_static { } cc_library_static { - name: "libsnapshot_nobinder", + name: "libsnapshot_init", defaults: ["libsnapshot_defaults"], srcs: [":libsnapshot_sources"], recovery_available: true, @@ -92,6 +107,19 @@ cc_library_static { ], } +cc_library_static { + name: "libsnapshot_nobinder", + defaults: [ + "libsnapshot_defaults", + "libsnapshot_hal_deps", + ], + srcs: [":libsnapshot_sources"], + recovery_available: true, + whole_static_libs: [ + "libfiemap_passthrough", + ], +} + cc_test { name: "libsnapshot_test", defaults: ["libsnapshot_defaults"], @@ -103,11 +131,13 @@ cc_test { ], shared_libs: [ "libbinder", + "libhidlbase", "libprotobuf-cpp-lite", "libutils", ], static_libs: [ - "libcutils", + "android.hardware.boot@1.0", + "android.hardware.boot@1.1", "libcrypto_static", "libfs_mgr", "libgmock", @@ -134,10 +164,14 @@ cc_binary { "libsnapshot", ], shared_libs: [ + "android.hardware.boot@1.0", + "android.hardware.boot@1.1", "libbase", "libbinder", + "libbinderthreadstate", "libext4_utils", "libfs_mgr", + "libhidlbase", "liblog", "liblp", "libprotobuf-cpp-lite", diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h index 69f28952b..fcaa73ad7 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h @@ -49,6 +49,16 @@ struct CreateLogicalPartitionParams; class IPartitionOpener; } // namespace fs_mgr +// Forward declare IBootControl types since we cannot include only the headers +// with Soong. Note: keep the enum width in sync. +namespace hardware { +namespace boot { +namespace V1_1 { +enum class MergeStatus : int32_t; +} // namespace V1_1 +} // namespace boot +} // namespace hardware + namespace snapshot { struct AutoDeleteCowImage; @@ -94,6 +104,7 @@ class SnapshotManager final { using LpMetadata = android::fs_mgr::LpMetadata; using MetadataBuilder = android::fs_mgr::MetadataBuilder; using DeltaArchiveManifest = chromeos_update_engine::DeltaArchiveManifest; + using MergeStatus = android::hardware::boot::V1_1::MergeStatus; public: // Dependency injection for testing. @@ -107,6 +118,7 @@ class SnapshotManager final { virtual std::string GetSuperDevice(uint32_t slot) const = 0; virtual const IPartitionOpener& GetPartitionOpener() const = 0; virtual bool IsOverlayfsSetup() const = 0; + virtual bool SetBootControlMergeStatus(MergeStatus status) = 0; }; ~SnapshotManager(); @@ -208,6 +220,7 @@ class SnapshotManager final { FRIEND_TEST(SnapshotTest, Merge); FRIEND_TEST(SnapshotTest, MergeCannotRemoveCow); FRIEND_TEST(SnapshotTest, NoMergeBeforeReboot); + FRIEND_TEST(SnapshotTest, UpdateBootControlHal); FRIEND_TEST(SnapshotUpdateTest, SnapshotStatusFileWithoutCow); friend class SnapshotTest; friend class SnapshotUpdateTest; diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp index 5b758c958..cee5cd455 100644 --- a/fs_mgr/libsnapshot/snapshot.cpp +++ b/fs_mgr/libsnapshot/snapshot.cpp @@ -29,6 +29,9 @@ #include #include #include +#ifdef LIBSNAPSHOT_USE_HAL +#include +#endif #include #include #include @@ -63,6 +66,7 @@ using android::fs_mgr::GetPartitionName; using android::fs_mgr::LpMetadata; using android::fs_mgr::MetadataBuilder; using android::fs_mgr::SlotNumberForSlotSuffix; +using android::hardware::boot::V1_1::MergeStatus; using chromeos_update_engine::DeltaArchiveManifest; using chromeos_update_engine::InstallOperation; template @@ -86,11 +90,39 @@ class DeviceInfo final : public SnapshotManager::IDeviceInfo { return fs_mgr_get_super_partition_name(slot); } bool IsOverlayfsSetup() const override { return fs_mgr_overlayfs_is_setup(); } + bool SetBootControlMergeStatus(MergeStatus status) override; private: android::fs_mgr::PartitionOpener opener_; +#ifdef LIBSNAPSHOT_USE_HAL + android::sp boot_control_; +#endif }; +bool DeviceInfo::SetBootControlMergeStatus([[maybe_unused]] MergeStatus status) { +#ifdef LIBSNAPSHOT_USE_HAL + if (!boot_control_) { + auto hal = android::hardware::boot::V1_0::IBootControl::getService(); + if (!hal) { + LOG(ERROR) << "Could not find IBootControl HAL"; + return false; + } + boot_control_ = android::hardware::boot::V1_1::IBootControl::castFrom(hal); + if (!boot_control_) { + LOG(ERROR) << "Could not find IBootControl 1.1 HAL"; + return false; + } + } + if (!boot_control_->setSnapshotMergeStatus(status)) { + LOG(ERROR) << "Unable to set the snapshot merge status"; + return false; + } + return true; +#else + return false; +#endif +} + // Note: IImageManager is an incomplete type in the header, so the default // destructor doesn't work. SnapshotManager::~SnapshotManager() {} @@ -1592,6 +1624,35 @@ bool SnapshotManager::WriteUpdateState(LockedFile* file, UpdateState state) { PLOG(ERROR) << "Could not write to state file"; return false; } + +#ifdef LIBSNAPSHOT_USE_HAL + auto merge_status = MergeStatus::UNKNOWN; + switch (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: + case UpdateState::MergeNeedsReboot: + case UpdateState::MergeCompleted: + merge_status = MergeStatus::NONE; + break; + case UpdateState::Initiated: + case UpdateState::Unverified: + merge_status = MergeStatus::SNAPSHOTTED; + break; + case UpdateState::Merging: + case UpdateState::MergeFailed: + merge_status = MergeStatus::MERGING; + break; + 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; + break; + } + if (!device_->SetBootControlMergeStatus(merge_status)) { + return false; + } +#endif return true; } diff --git a/fs_mgr/libsnapshot/snapshot_test.cpp b/fs_mgr/libsnapshot/snapshot_test.cpp index fd7754ed5..974587b38 100644 --- a/fs_mgr/libsnapshot/snapshot_test.cpp +++ b/fs_mgr/libsnapshot/snapshot_test.cpp @@ -617,6 +617,31 @@ TEST_F(SnapshotTest, FlashSuperDuringMerge) { ASSERT_EQ(sm->GetUpdateState(), UpdateState::None); } +TEST_F(SnapshotTest, UpdateBootControlHal) { + ASSERT_TRUE(AcquireLock()); + + ASSERT_TRUE(sm->WriteUpdateState(lock_.get(), UpdateState::None)); + ASSERT_EQ(test_device->merge_status(), MergeStatus::NONE); + + ASSERT_TRUE(sm->WriteUpdateState(lock_.get(), UpdateState::Initiated)); + ASSERT_EQ(test_device->merge_status(), MergeStatus::SNAPSHOTTED); + + ASSERT_TRUE(sm->WriteUpdateState(lock_.get(), UpdateState::Unverified)); + ASSERT_EQ(test_device->merge_status(), MergeStatus::SNAPSHOTTED); + + ASSERT_TRUE(sm->WriteUpdateState(lock_.get(), UpdateState::Merging)); + ASSERT_EQ(test_device->merge_status(), MergeStatus::MERGING); + + ASSERT_TRUE(sm->WriteUpdateState(lock_.get(), UpdateState::MergeNeedsReboot)); + ASSERT_EQ(test_device->merge_status(), MergeStatus::NONE); + + ASSERT_TRUE(sm->WriteUpdateState(lock_.get(), UpdateState::MergeCompleted)); + ASSERT_EQ(test_device->merge_status(), MergeStatus::NONE); + + ASSERT_TRUE(sm->WriteUpdateState(lock_.get(), UpdateState::MergeFailed)); + ASSERT_EQ(test_device->merge_status(), MergeStatus::MERGING); +} + class SnapshotUpdateTest : public SnapshotTest { public: void SetUp() override { diff --git a/fs_mgr/libsnapshot/test_helpers.h b/fs_mgr/libsnapshot/test_helpers.h index 769d21e57..ea2c5b652 100644 --- a/fs_mgr/libsnapshot/test_helpers.h +++ b/fs_mgr/libsnapshot/test_helpers.h @@ -17,6 +17,7 @@ #include #include +#include #include #include #include @@ -32,6 +33,7 @@ namespace snapshot { using android::fs_mgr::IPropertyFetcher; using android::fs_mgr::MetadataBuilder; using android::fs_mgr::testing::MockPropertyFetcher; +using android::hardware::boot::V1_1::MergeStatus; using chromeos_update_engine::DeltaArchiveManifest; using chromeos_update_engine::PartitionUpdate; using testing::_; @@ -81,16 +83,22 @@ class TestDeviceInfo : public SnapshotManager::IDeviceInfo { const android::fs_mgr::IPartitionOpener& GetPartitionOpener() const override { return *opener_.get(); } + bool SetBootControlMergeStatus(MergeStatus status) override { + merge_status_ = status; + return true; + } bool IsOverlayfsSetup() const override { return false; } void set_slot_suffix(const std::string& suffix) { slot_suffix_ = suffix; } void set_fake_super(const std::string& path) { opener_ = std::make_unique(path); } + MergeStatus merge_status() const { return merge_status_; } private: std::string slot_suffix_ = "_a"; std::unique_ptr opener_; + MergeStatus merge_status_; }; class SnapshotTestPropertyFetcher : public android::fs_mgr::testing::MockPropertyFetcher { diff --git a/init/Android.bp b/init/Android.bp index 9b2ddc0f5..ce5b12a09 100644 --- a/init/Android.bp +++ b/init/Android.bp @@ -69,7 +69,7 @@ cc_defaults { "libprotobuf-cpp-lite", "libpropertyinfoserializer", "libpropertyinfoparser", - "libsnapshot_nobinder", + "libsnapshot_init", ], shared_libs: [ "libbacktrace", diff --git a/init/Android.mk b/init/Android.mk index 8fc44dad7..4e4c00210 100644 --- a/init/Android.mk +++ b/init/Android.mk @@ -114,7 +114,7 @@ LOCAL_STATIC_LIBRARIES := \ libmodprobe \ libext2_uuid \ libprotobuf-cpp-lite \ - libsnapshot_nobinder \ + libsnapshot_init \ LOCAL_SANITIZE := signed-integer-overflow # First stage init is weird: it may start without stdout/stderr, and no /proc.