From f41c7bbb9639e04d2aa5df749a408b8f1d0dc6ae Mon Sep 17 00:00:00 2001 From: David Anderson Date: Mon, 16 Dec 2019 19:52:14 -0800 Subject: [PATCH] libfiemap: Add helpers to remove images from recovery. ImageManager can map images in recovery, but not delete them, because /data is not mounted. libsnapshot handles this by storing extra state files, but this is complex to manage and inconvenient for fs_mgr_overlayfs. Instead, this patch introduces two new calls: - DisableImage(), which indicates the image should not be used. This is implemented by adding a new DISABLED attribute to LpPartitionMetadata. CreateLogicalPartitions ignores this flag, and thus recovery/fastbootd can disable the scratch partition and communicate that it can be deleted. This cannot be called from binder since it is intended for recovery/first-stage init only. - RemoveDisabledImages(), which walks the images for a given folder on /metadata and deletes any that are disabled. This can be called from binder. Note that there is no metadata version bump for this flag. It's considered to be included in the flag list for minor version 1, and currently is not used for the actual super partition. Bug: 134949511 Test: adb remount, fastboot flash system Test: fiemap_image_test Change-Id: Iaeca2d1eddb5637dd9a20202cafd11ae60b4d0e3 --- fs_mgr/fs_mgr_dm_linear.cpp | 4 ++++ fs_mgr/libfiemap/binder.cpp | 17 +++++++++++++++ fs_mgr/libfiemap/image_manager.cpp | 21 +++++++++++++++++++ fs_mgr/libfiemap/image_test.cpp | 8 +++++++ .../include/libfiemap/image_manager.h | 12 +++++++++++ fs_mgr/libfiemap/metadata.cpp | 18 ++++++++++++++++ fs_mgr/libfiemap/metadata.h | 2 ++ fs_mgr/liblp/builder.cpp | 2 +- fs_mgr/liblp/include/liblp/builder.h | 1 + fs_mgr/liblp/include/liblp/metadata_format.h | 8 +++++-- fs_mgr/liblp/reader.cpp | 6 ++---- 11 files changed, 92 insertions(+), 7 deletions(-) diff --git a/fs_mgr/fs_mgr_dm_linear.cpp b/fs_mgr/fs_mgr_dm_linear.cpp index 0dcb9fe8a..ea9c957b5 100644 --- a/fs_mgr/fs_mgr_dm_linear.cpp +++ b/fs_mgr/fs_mgr_dm_linear.cpp @@ -151,6 +151,10 @@ bool CreateLogicalPartitions(const LpMetadata& metadata, const std::string& supe LINFO << "Skipping zero-length logical partition: " << GetPartitionName(partition); continue; } + if (partition.attributes & LP_PARTITION_ATTR_DISABLED) { + LINFO << "Skipping disabled partition: " << GetPartitionName(partition); + continue; + } params.partition = &partition; diff --git a/fs_mgr/libfiemap/binder.cpp b/fs_mgr/libfiemap/binder.cpp index 49779f435..5aa456067 100644 --- a/fs_mgr/libfiemap/binder.cpp +++ b/fs_mgr/libfiemap/binder.cpp @@ -43,6 +43,8 @@ class ImageManagerBinder final : public IImageManager { std::string* dev) override; bool ZeroFillNewImage(const std::string& name, uint64_t bytes) override; bool RemoveAllImages() override; + bool DisableImage(const std::string& name) override; + bool RemoveDisabledImages() override; std::vector GetAllBackingImages() override; @@ -163,6 +165,21 @@ bool ImageManagerBinder::RemoveAllImages() { return true; } +bool ImageManagerBinder::DisableImage(const std::string&) { + LOG(ERROR) << __PRETTY_FUNCTION__ << " is not available over binder"; + return false; +} + +bool ImageManagerBinder::RemoveDisabledImages() { + auto status = manager_->removeDisabledImages(); + if (!status.isOk()) { + LOG(ERROR) << __PRETTY_FUNCTION__ + << " binder returned: " << status.exceptionMessage().string(); + return false; + } + return true; +} + static android::sp AcquireIGsid(const std::chrono::milliseconds& timeout_ms) { if (android::base::GetProperty("init.svc.gsid", "") != "running") { if (!android::base::SetProperty("ctl.start", "gsid") || diff --git a/fs_mgr/libfiemap/image_manager.cpp b/fs_mgr/libfiemap/image_manager.cpp index fe2018d4d..5b22eaa9c 100644 --- a/fs_mgr/libfiemap/image_manager.cpp +++ b/fs_mgr/libfiemap/image_manager.cpp @@ -632,6 +632,27 @@ bool ImageManager::Validate() { return true; } +bool ImageManager::DisableImage(const std::string& name) { + return AddAttributes(metadata_dir_, name, LP_PARTITION_ATTR_DISABLED); +} + +bool ImageManager::RemoveDisabledImages() { + if (!MetadataExists(metadata_dir_)) { + return true; + } + + auto metadata = OpenMetadata(metadata_dir_); + if (!metadata) { + return false; + } + + bool ok = true; + for (const auto& partition : metadata->partitions) { + ok &= DeleteBackingImage(GetPartitionName(partition)); + } + return ok; +} + std::unique_ptr MappedDevice::Open(IImageManager* manager, const std::chrono::milliseconds& timeout_ms, const std::string& name) { diff --git a/fs_mgr/libfiemap/image_test.cpp b/fs_mgr/libfiemap/image_test.cpp index f05825c88..90fdb4cd1 100644 --- a/fs_mgr/libfiemap/image_test.cpp +++ b/fs_mgr/libfiemap/image_test.cpp @@ -112,6 +112,14 @@ TEST_F(NativeTest, CreateAndMap) { ASSERT_EQ(android::base::GetProperty(PropertyName(), ""), ""); } +TEST_F(NativeTest, DisableImage) { + ASSERT_TRUE(manager_->CreateBackingImage(base_name_, kTestImageSize, false, nullptr)); + ASSERT_TRUE(manager_->BackingImageExists(base_name_)); + ASSERT_TRUE(manager_->DisableImage(base_name_)); + ASSERT_TRUE(manager_->RemoveDisabledImages()); + ASSERT_TRUE(!manager_->BackingImageExists(base_name_)); +} + // This fixture is for tests against a simulated device environment. Rather // than use /data, we create an image and then layer a new filesystem within // it. Each test then decides how to mount and create layered images. This diff --git a/fs_mgr/libfiemap/include/libfiemap/image_manager.h b/fs_mgr/libfiemap/include/libfiemap/image_manager.h index 5ff462817..fbc61d13a 100644 --- a/fs_mgr/libfiemap/include/libfiemap/image_manager.h +++ b/fs_mgr/libfiemap/include/libfiemap/image_manager.h @@ -84,6 +84,16 @@ class IImageManager { virtual bool MapImageWithDeviceMapper(const IPartitionOpener& opener, const std::string& name, std::string* dev) = 0; + // Mark an image as disabled. This is useful for marking an image as + // will-be-deleted in recovery, since recovery cannot mount /data. + // + // This is not available in binder, since it is intended for recovery. + // When binder is available, images can simply be removed. + virtual bool DisableImage(const std::string& name) = 0; + + // Remove all images that been marked as disabled. + virtual bool RemoveDisabledImages() = 0; + // Get all backing image names. virtual std::vector GetAllBackingImages() = 0; @@ -119,6 +129,8 @@ class ImageManager final : public IImageManager { bool MapImageWithDeviceMapper(const IPartitionOpener& opener, const std::string& name, std::string* dev) override; bool RemoveAllImages() override; + bool DisableImage(const std::string& name) override; + bool RemoveDisabledImages() override; std::vector GetAllBackingImages(); // Same as CreateBackingImage, but provides a progress notification. diff --git a/fs_mgr/libfiemap/metadata.cpp b/fs_mgr/libfiemap/metadata.cpp index 597efe987..ea1f5084e 100644 --- a/fs_mgr/libfiemap/metadata.cpp +++ b/fs_mgr/libfiemap/metadata.cpp @@ -192,5 +192,23 @@ bool UpdateMetadata(const std::string& metadata_dir, const std::string& partitio return SaveMetadata(builder.get(), metadata_dir); } +bool AddAttributes(const std::string& metadata_dir, const std::string& partition_name, + uint32_t attributes) { + auto metadata = OpenMetadata(metadata_dir); + if (!metadata) { + return false; + } + auto builder = MetadataBuilder::New(*metadata.get()); + if (!builder) { + return false; + } + auto partition = builder->FindPartition(partition_name); + if (!partition) { + return false; + } + partition->set_attributes(partition->attributes() | attributes); + return SaveMetadata(builder.get(), metadata_dir); +} + } // namespace fiemap } // namespace android diff --git a/fs_mgr/libfiemap/metadata.h b/fs_mgr/libfiemap/metadata.h index f0ce23ef2..4eb3ad5c5 100644 --- a/fs_mgr/libfiemap/metadata.h +++ b/fs_mgr/libfiemap/metadata.h @@ -29,6 +29,8 @@ bool MetadataExists(const std::string& metadata_dir); std::unique_ptr OpenMetadata(const std::string& metadata_dir); bool UpdateMetadata(const std::string& metadata_dir, const std::string& partition_name, SplitFiemap* file, uint64_t partition_size, bool readonly); +bool AddAttributes(const std::string& metadata_dir, const std::string& partition_name, + uint32_t attributes); bool RemoveImageMetadata(const std::string& metadata_dir, const std::string& partition_name); bool RemoveAllMetadata(const std::string& dir); diff --git a/fs_mgr/liblp/builder.cpp b/fs_mgr/liblp/builder.cpp index 4406696ae..8e29cc616 100644 --- a/fs_mgr/liblp/builder.cpp +++ b/fs_mgr/liblp/builder.cpp @@ -852,7 +852,7 @@ std::unique_ptr MetadataBuilder::Export() { return nullptr; } - if (partition->attributes() & LP_PARTITION_ATTR_UPDATED) { + if (partition->attributes() & LP_PARTITION_ATTRIBUTE_MASK_V1) { static const uint16_t kMinVersion = LP_METADATA_VERSION_FOR_UPDATED_ATTR; metadata->header.minor_version = std::max(metadata->header.minor_version, kMinVersion); } diff --git a/fs_mgr/liblp/include/liblp/builder.h b/fs_mgr/liblp/include/liblp/builder.h index 7a334fbd9..d9a17e399 100644 --- a/fs_mgr/liblp/include/liblp/builder.h +++ b/fs_mgr/liblp/include/liblp/builder.h @@ -145,6 +145,7 @@ class Partition final { std::vector> extents_; uint32_t attributes_; uint64_t size_; + bool disabled_; }; // An interval in the metadata. This is similar to a LinearExtent with one difference. diff --git a/fs_mgr/liblp/include/liblp/metadata_format.h b/fs_mgr/liblp/include/liblp/metadata_format.h index 26cbf07ba..8afc1e761 100644 --- a/fs_mgr/liblp/include/liblp/metadata_format.h +++ b/fs_mgr/liblp/include/liblp/metadata_format.h @@ -72,13 +72,17 @@ extern "C" { */ #define LP_PARTITION_ATTR_UPDATED (1 << 2) +/* This flag marks a partition as disabled. It should not be used or mapped. */ +#define LP_PARTITION_ATTR_DISABLED (1 << 3) + /* Mask that defines all valid attributes. When changing this, make sure to * update ParseMetadata(). */ #define LP_PARTITION_ATTRIBUTE_MASK_V0 \ (LP_PARTITION_ATTR_READONLY | LP_PARTITION_ATTR_SLOT_SUFFIXED) -#define LP_PARTITION_ATTRIBUTE_MASK_V1 (LP_PARTITION_ATTRIBUTE_MASK_V0 | LP_PARTITION_ATTR_UPDATED) -#define LP_PARTITION_ATTRIBUTE_MASK LP_PARTITION_ATTRIBUTE_MASK_V1 +#define LP_PARTITION_ATTRIBUTE_MASK_V1 (LP_PARTITION_ATTR_UPDATED | LP_PARTITION_ATTR_DISABLED) +#define LP_PARTITION_ATTRIBUTE_MASK \ + (LP_PARTITION_ATTRIBUTE_MASK_V0 | LP_PARTITION_ATTRIBUTE_MASK_V1) /* Default name of the physical partition that holds logical partition entries. * The layout of this partition will look like: diff --git a/fs_mgr/liblp/reader.cpp b/fs_mgr/liblp/reader.cpp index 30c17e42a..e6fd9f72a 100644 --- a/fs_mgr/liblp/reader.cpp +++ b/fs_mgr/liblp/reader.cpp @@ -280,11 +280,9 @@ static std::unique_ptr ParseMetadata(const LpMetadataGeometry& geome return nullptr; } - uint32_t valid_attributes = 0; + uint32_t valid_attributes = LP_PARTITION_ATTRIBUTE_MASK_V0; if (metadata->header.minor_version >= LP_METADATA_VERSION_FOR_UPDATED_ATTR) { - valid_attributes = LP_PARTITION_ATTRIBUTE_MASK_V1; - } else { - valid_attributes = LP_PARTITION_ATTRIBUTE_MASK_V0; + valid_attributes |= LP_PARTITION_ATTRIBUTE_MASK_V1; } // ValidateTableSize ensured that |cursor| is valid for the number of