From cd232f9734d8efaec7b129a4e9dea8ef72257ff0 Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Fri, 3 Jan 2020 14:36:35 -0800 Subject: [PATCH] SnapshotManager expose no space error When there is not enough space on /userdata, CreateUpdateSnapshot returns SnapshotManager::Return with ErrorCode::NO_SPACE. Test: libsnapshot_test Bug: 138808058 Change-Id: If2effe63f6a4324eff8d05d4db4ce98be8190262 --- .../include/libfiemap/fiemap_status.h | 4 +- .../include/libsnapshot/snapshot.h | 36 ++++-- fs_mgr/libsnapshot/snapshot.cpp | 110 +++++++++++------- fs_mgr/libsnapshot/snapshot_test.cpp | 23 ++++ fs_mgr/libsnapshot/utility.cpp | 13 ++- fs_mgr/libsnapshot/utility.h | 3 +- 6 files changed, 131 insertions(+), 58 deletions(-) diff --git a/fs_mgr/libfiemap/include/libfiemap/fiemap_status.h b/fs_mgr/libfiemap/include/libfiemap/fiemap_status.h index 56917cc5f..d7b2cf18e 100644 --- a/fs_mgr/libfiemap/include/libfiemap/fiemap_status.h +++ b/fs_mgr/libfiemap/include/libfiemap/fiemap_status.h @@ -56,10 +56,12 @@ class FiemapStatus { // For logging and debugging only. std::string string() const; + protected: + FiemapStatus(ErrorCode code) : error_code_(code) {} + private: ErrorCode error_code_; - FiemapStatus(ErrorCode code) : error_code_(code) {} static ErrorCode CastErrorCode(int error); }; diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h index 52f8794e6..6e613ba67 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h @@ -116,8 +116,30 @@ class SnapshotManager final { using MetadataBuilder = android::fs_mgr::MetadataBuilder; using DeltaArchiveManifest = chromeos_update_engine::DeltaArchiveManifest; using MergeStatus = android::hardware::boot::V1_1::MergeStatus; + using FiemapStatus = android::fiemap::FiemapStatus; public: + // SnapshotManager functions return either bool or Return objects. "Return" types provides + // more information about the reason of the failure. + class Return : public FiemapStatus { + public: + // Total required size on /userdata. + uint64_t required_size() const { return required_size_; } + + static Return Ok() { return Return(FiemapStatus::ErrorCode::SUCCESS); } + static Return Error() { return Return(FiemapStatus::ErrorCode::ERROR); } + static Return NoSpace(uint64_t size) { + return Return(FiemapStatus::ErrorCode::NO_SPACE, size); + } + // Does not set required_size_ properly even when status.error_code() == NO_SPACE. + explicit Return(const FiemapStatus& status) : Return(status.error_code()) {} + + private: + uint64_t required_size_; + Return(FiemapStatus::ErrorCode code, uint64_t required_size = 0) + : FiemapStatus(code), required_size_(required_size) {} + }; + // Dependency injection for testing. class IDeviceInfo { public: @@ -222,7 +244,7 @@ class SnapshotManager final { // Create necessary COW device / files for OTA clients. New logical partitions will be added to // group "cow" in target_metadata. Regions of partitions of current_metadata will be // "write-protected" and snapshotted. - bool CreateUpdateSnapshots(const DeltaArchiveManifest& manifest); + Return CreateUpdateSnapshots(const DeltaArchiveManifest& manifest); // Map a snapshotted partition for OTA clients to write to. Write-protected regions are // determined previously in CreateSnapshots. @@ -359,7 +381,7 @@ class SnapshotManager final { // |name| should be the base partition name (e.g. "system_a"). Create the // backing COW image using the size previously passed to CreateSnapshot(). - bool CreateCowImage(LockedFile* lock, const std::string& name); + Return CreateCowImage(LockedFile* lock, const std::string& name); // Map a snapshot device that was previously created with CreateSnapshot. // If a merge was previously initiated, the device-mapper table will have a @@ -499,14 +521,14 @@ class SnapshotManager final { // Helper for CreateUpdateSnapshots. // Creates all underlying images, COW partitions and snapshot files. Does not initialize them. - bool CreateUpdateSnapshotsInternal(LockedFile* lock, const DeltaArchiveManifest& manifest, - PartitionCowCreator* cow_creator, - AutoDeviceList* created_devices, - std::map* all_snapshot_status); + Return CreateUpdateSnapshotsInternal( + LockedFile* lock, const DeltaArchiveManifest& manifest, + PartitionCowCreator* cow_creator, AutoDeviceList* created_devices, + std::map* all_snapshot_status); // Initialize snapshots so that they can be mapped later. // Map the COW partition and zero-initialize the header. - bool InitializeUpdateSnapshots( + Return InitializeUpdateSnapshots( LockedFile* lock, MetadataBuilder* target_metadata, const LpMetadata* exported_target_metadata, const std::string& target_suffix, const std::map& all_snapshot_status); diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp index fd89ca0dc..b79b65c63 100644 --- a/fs_mgr/libsnapshot/snapshot.cpp +++ b/fs_mgr/libsnapshot/snapshot.cpp @@ -54,6 +54,7 @@ using android::dm::DmTargetLinear; using android::dm::DmTargetSnapshot; using android::dm::kSectorSize; using android::dm::SnapshotStorageMode; +using android::fiemap::FiemapStatus; using android::fiemap::IImageManager; using android::fs_mgr::CreateDmTable; using android::fs_mgr::CreateLogicalPartition; @@ -289,14 +290,14 @@ bool SnapshotManager::CreateSnapshot(LockedFile* lock, SnapshotStatus* status) { return true; } -bool SnapshotManager::CreateCowImage(LockedFile* lock, const std::string& name) { +SnapshotManager::Return SnapshotManager::CreateCowImage(LockedFile* lock, const std::string& name) { CHECK(lock); CHECK(lock->lock_mode() == LOCK_EX); - if (!EnsureImageManager()) return false; + if (!EnsureImageManager()) return Return::Error(); SnapshotStatus status; if (!ReadSnapshotStatus(lock, name, &status)) { - return false; + return Return::Error(); } // The COW file size should have been rounded up to the nearest sector in CreateSnapshot. @@ -304,12 +305,12 @@ bool SnapshotManager::CreateCowImage(LockedFile* lock, const std::string& name) if (status.cow_file_size() % kSectorSize != 0) { LOG(ERROR) << "Snapshot " << name << " COW file size is not a multiple of the sector size: " << status.cow_file_size(); - return false; + return Return::Error(); } std::string cow_image_name = GetCowImageDeviceName(name); int cow_flags = IImageManager::CREATE_IMAGE_DEFAULT; - return images_->CreateBackingImage(cow_image_name, status.cow_file_size(), cow_flags); + return Return(images_->CreateBackingImage(cow_image_name, status.cow_file_size(), cow_flags)); } bool SnapshotManager::MapSnapshot(LockedFile* lock, const std::string& name, @@ -1844,9 +1845,23 @@ static void UnmapAndDeleteCowPartition(MetadataBuilder* current_metadata) { } } -bool SnapshotManager::CreateUpdateSnapshots(const DeltaArchiveManifest& manifest) { +static SnapshotManager::Return AddRequiredSpace( + SnapshotManager::Return orig, + const std::map& all_snapshot_status) { + if (orig.error_code() != SnapshotManager::Return::ErrorCode::NO_SPACE) { + return orig; + } + uint64_t sum = 0; + for (auto&& [name, status] : all_snapshot_status) { + sum += status.cow_file_size(); + } + return SnapshotManager::Return::NoSpace(sum); +} + +SnapshotManager::Return SnapshotManager::CreateUpdateSnapshots( + const DeltaArchiveManifest& manifest) { auto lock = LockExclusive(); - if (!lock) return false; + if (!lock) return Return::Error(); // TODO(b/134949511): remove this check. Right now, with overlayfs mounted, the scratch // partition takes up a big chunk of space in super, causing COW images to be created on @@ -1854,7 +1869,7 @@ bool SnapshotManager::CreateUpdateSnapshots(const DeltaArchiveManifest& manifest if (device_->IsOverlayfsSetup()) { LOG(ERROR) << "Cannot create update snapshots with overlayfs setup. Run `adb enable-verity`" << ", reboot, then try again."; - return false; + return Return::Error(); } const auto& opener = device_->GetPartitionOpener(); @@ -1879,7 +1894,7 @@ bool SnapshotManager::CreateUpdateSnapshots(const DeltaArchiveManifest& manifest SnapshotMetadataUpdater metadata_updater(target_metadata.get(), target_slot, manifest); if (!metadata_updater.Update()) { LOG(ERROR) << "Cannot calculate new metadata."; - return false; + return Return::Error(); } // Delete previous COW partitions in current_metadata so that PartitionCowCreator marks those as @@ -1911,36 +1926,34 @@ bool SnapshotManager::CreateUpdateSnapshots(const DeltaArchiveManifest& manifest .extra_extents = {}, }; - if (!CreateUpdateSnapshotsInternal(lock.get(), manifest, &cow_creator, &created_devices, - &all_snapshot_status)) { - return false; - } + auto ret = CreateUpdateSnapshotsInternal(lock.get(), manifest, &cow_creator, &created_devices, + &all_snapshot_status); + if (!ret.is_ok()) return ret; auto exported_target_metadata = target_metadata->Export(); if (exported_target_metadata == nullptr) { LOG(ERROR) << "Cannot export target metadata"; - return false; + return Return::Error(); } - if (!InitializeUpdateSnapshots(lock.get(), target_metadata.get(), - exported_target_metadata.get(), target_suffix, - all_snapshot_status)) { - return false; - } + ret = InitializeUpdateSnapshots(lock.get(), target_metadata.get(), + exported_target_metadata.get(), target_suffix, + all_snapshot_status); + if (!ret.is_ok()) return ret; if (!UpdatePartitionTable(opener, device_->GetSuperDevice(target_slot), *exported_target_metadata, target_slot)) { LOG(ERROR) << "Cannot write target metadata"; - return false; + return Return::Error(); } created_devices.Release(); LOG(INFO) << "Successfully created all snapshots for target slot " << target_suffix; - return true; + return Return::Ok(); } -bool SnapshotManager::CreateUpdateSnapshotsInternal( +SnapshotManager::Return SnapshotManager::CreateUpdateSnapshotsInternal( LockedFile* lock, const DeltaArchiveManifest& manifest, PartitionCowCreator* cow_creator, AutoDeviceList* created_devices, std::map* all_snapshot_status) { @@ -1951,7 +1964,7 @@ bool SnapshotManager::CreateUpdateSnapshotsInternal( if (!target_metadata->AddGroup(kCowGroupName, 0)) { LOG(ERROR) << "Cannot add group " << kCowGroupName; - return false; + return Return::Error(); } std::map*> install_operation_map; @@ -1963,7 +1976,7 @@ bool SnapshotManager::CreateUpdateSnapshotsInternal( if (!inserted) { LOG(ERROR) << "Duplicated partition " << partition_update.partition_name() << " in update manifest."; - return false; + return Return::Error(); } auto& extra_extents = extra_extents_map[suffixed_name]; @@ -1992,7 +2005,7 @@ bool SnapshotManager::CreateUpdateSnapshotsInternal( // Compute the device sizes for the partition. auto cow_creator_ret = cow_creator->Run(); if (!cow_creator_ret.has_value()) { - return false; + return Return::Error(); } LOG(INFO) << "For partition " << target_partition->name() @@ -2006,7 +2019,7 @@ bool SnapshotManager::CreateUpdateSnapshotsInternal( if (!DeleteSnapshot(lock, target_partition->name())) { LOG(ERROR) << "Cannot delete existing snapshot before creating a new one for partition " << target_partition->name(); - return false; + return Return::Error(); } // It is possible that the whole partition uses free space in super, and snapshot / COW @@ -2024,7 +2037,7 @@ bool SnapshotManager::CreateUpdateSnapshotsInternal( // Store these device sizes to snapshot status file. if (!CreateSnapshot(lock, &cow_creator_ret->snapshot_status)) { - return false; + return Return::Error(); } created_devices->EmplaceBack(this, lock, target_partition->name()); @@ -2038,7 +2051,7 @@ bool SnapshotManager::CreateUpdateSnapshotsInternal( auto cow_partition = target_metadata->AddPartition(GetCowName(target_partition->name()), kCowGroupName, 0 /* flags */); if (cow_partition == nullptr) { - return false; + return Return::Error(); } if (!target_metadata->ResizePartition( @@ -2046,28 +2059,34 @@ bool SnapshotManager::CreateUpdateSnapshotsInternal( cow_creator_ret->cow_partition_usable_regions)) { LOG(ERROR) << "Cannot create COW partition on metadata with size " << cow_creator_ret->snapshot_status.cow_partition_size(); - return false; + return Return::Error(); } // Only the in-memory target_metadata is modified; nothing to clean up if there is an // error in the future. } - // Create the backing COW image if necessary. - if (cow_creator_ret->snapshot_status.cow_file_size() > 0) { - if (!CreateCowImage(lock, target_partition->name())) { - return false; - } - } - all_snapshot_status->emplace(target_partition->name(), std::move(cow_creator_ret->snapshot_status)); - LOG(INFO) << "Successfully created snapshot for " << target_partition->name(); + LOG(INFO) << "Successfully created snapshot partition for " << target_partition->name(); } - return true; + + LOG(INFO) << "Allocating CoW images."; + + for (auto&& [name, snapshot_status] : *all_snapshot_status) { + // Create the backing COW image if necessary. + if (snapshot_status.cow_file_size() > 0) { + auto ret = CreateCowImage(lock, name); + if (!ret.is_ok()) return AddRequiredSpace(ret, *all_snapshot_status); + } + + LOG(INFO) << "Successfully created snapshot for " << name; + } + + return Return::Ok(); } -bool SnapshotManager::InitializeUpdateSnapshots( +SnapshotManager::Return SnapshotManager::InitializeUpdateSnapshots( LockedFile* lock, MetadataBuilder* target_metadata, const LpMetadata* exported_target_metadata, const std::string& target_suffix, const std::map& all_snapshot_status) { @@ -2086,7 +2105,7 @@ bool SnapshotManager::InitializeUpdateSnapshots( if (!UnmapPartitionWithSnapshot(lock, target_partition->name())) { LOG(ERROR) << "Cannot unmap existing COW devices before re-mapping them for zero-fill: " << target_partition->name(); - return false; + return Return::Error(); } auto it = all_snapshot_status.find(target_partition->name()); @@ -2094,23 +2113,24 @@ bool SnapshotManager::InitializeUpdateSnapshots( cow_params.partition_name = target_partition->name(); std::string cow_name; if (!MapCowDevices(lock, cow_params, it->second, &created_devices_for_cow, &cow_name)) { - return false; + return Return::Error(); } std::string cow_path; if (!dm.GetDmDevicePathByName(cow_name, &cow_path)) { LOG(ERROR) << "Cannot determine path for " << cow_name; - return false; + return Return::Error(); } - if (!InitializeCow(cow_path)) { + auto ret = InitializeCow(cow_path); + if (!ret.is_ok()) { LOG(ERROR) << "Can't zero-fill COW device for " << target_partition->name() << ": " << cow_path; - return false; + return AddRequiredSpace(ret, all_snapshot_status); } // Let destructor of created_devices_for_cow to unmap the COW devices. }; - return true; + return Return::Ok(); } bool SnapshotManager::MapUpdateSnapshot(const CreateLogicalPartitionParams& params, diff --git a/fs_mgr/libsnapshot/snapshot_test.cpp b/fs_mgr/libsnapshot/snapshot_test.cpp index 9e2719f07..cea9d69f3 100644 --- a/fs_mgr/libsnapshot/snapshot_test.cpp +++ b/fs_mgr/libsnapshot/snapshot_test.cpp @@ -1586,6 +1586,29 @@ TEST_F(SnapshotUpdateTest, WaitForMerge) { ASSERT_THAT(merger.get(), AnyOf(UpdateState::None, UpdateState::MergeCompleted)); } +TEST_F(SnapshotUpdateTest, LowSpace) { + static constexpr auto kMaxFree = 10_MiB; + auto userdata = std::make_unique(); + ASSERT_TRUE(userdata->Init(kMaxFree)); + + // Grow all partitions to 5_MiB, total 15_MiB. This requires 15 MiB of CoW space. After + // using the empty space in super (< 1 MiB), it uses at least 14 MiB of /userdata space. + constexpr uint64_t partition_size = 5_MiB; + SetSize(sys_, partition_size); + SetSize(vnd_, partition_size); + SetSize(prd_, partition_size); + + AddOperationForPartitions(); + + // Execute the update. + ASSERT_TRUE(sm->BeginUpdate()); + auto res = sm->CreateUpdateSnapshots(manifest_); + ASSERT_FALSE(res); + ASSERT_EQ(SnapshotManager::Return::ErrorCode::NO_SPACE, res.error_code()); + ASSERT_GE(res.required_size(), 14_MiB); + ASSERT_LT(res.required_size(), 15_MiB); +} + class FlashAfterUpdateTest : public SnapshotUpdateTest, public WithParamInterface> { public: diff --git a/fs_mgr/libsnapshot/utility.cpp b/fs_mgr/libsnapshot/utility.cpp index fa1d7f054..f01500f4d 100644 --- a/fs_mgr/libsnapshot/utility.cpp +++ b/fs_mgr/libsnapshot/utility.cpp @@ -14,12 +14,15 @@ #include "utility.h" +#include + #include #include #include #include using android::dm::kSectorSize; +using android::fiemap::FiemapStatus; using android::fs_mgr::EnsurePathMounted; using android::fs_mgr::EnsurePathUnmounted; using android::fs_mgr::Fstab; @@ -83,7 +86,9 @@ AutoDeleteSnapshot::~AutoDeleteSnapshot() { } } -bool InitializeCow(const std::string& device) { +SnapshotManager::Return InitializeCow(const std::string& device) { + using Return = SnapshotManager::Return; + // When the kernel creates a persistent dm-snapshot, it requires a CoW file // to store the modifications. The kernel interface does not specify how // the CoW is used, and there is no standard associated. @@ -103,15 +108,15 @@ bool InitializeCow(const std::string& device) { android::base::unique_fd fd(open(device.c_str(), O_WRONLY | O_BINARY)); if (fd < 0) { PLOG(ERROR) << "Can't open COW device: " << device; - return false; + return Return(FiemapStatus::FromErrno(errno)); } LOG(INFO) << "Zero-filling COW device: " << device; if (!android::base::WriteFully(fd, zeros.data(), kDmSnapZeroFillSize)) { PLOG(ERROR) << "Can't zero-fill COW device for " << device; - return false; + return Return(FiemapStatus::FromErrno(errno)); } - return true; + return Return::Ok(); } std::unique_ptr AutoUnmountDevice::New(const std::string& path) { diff --git a/fs_mgr/libsnapshot/utility.h b/fs_mgr/libsnapshot/utility.h index 5cc572e66..0453256a7 100644 --- a/fs_mgr/libsnapshot/utility.h +++ b/fs_mgr/libsnapshot/utility.h @@ -26,6 +26,7 @@ #include #include +#include namespace android { namespace snapshot { @@ -110,7 +111,7 @@ std::vector ListPartitionsWithSuffix( android::fs_mgr::MetadataBuilder* builder, const std::string& suffix); // Initialize a device before using it as the COW device for a dm-snapshot device. -bool InitializeCow(const std::string& device); +SnapshotManager::Return InitializeCow(const std::string& device); // "Atomically" write string to file. This is done by a series of actions: // 1. Write to path + ".tmp"