From e4b44fc50116769c3bb948a68211e93267b0f9ef Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Mon, 23 Dec 2019 16:07:52 -0800 Subject: [PATCH] ImageManager returns FiemapStatus. IImageManager::CreateBackingImage and ZeroFillNewImage now returns FiemapStatus, which is an error code and provides more information about the reason of error. In particular, the error code is NO_SPACE if disk space is full during allocation / writing. Bug: 138808058 Test: libsnapshot_test Change-Id: I500a3d9f55a2d7e60438b5b4ae70f8b5fed162fa --- fs_mgr/libfiemap/binder.cpp | 60 ++++++++++++----- fs_mgr/libfiemap/image_manager.cpp | 45 +++++++------ .../include/libfiemap/fiemap_status.h | 6 ++ .../include/libfiemap/image_manager.h | 15 +++-- .../include_test/libsnapshot/test_helpers.h | 25 +++++++ fs_mgr/libsnapshot/snapshot_test.cpp | 50 ++++++++++++++ fs_mgr/libsnapshot/test_helpers.cpp | 65 +++++++++++++++++++ 7 files changed, 220 insertions(+), 46 deletions(-) diff --git a/fs_mgr/libfiemap/binder.cpp b/fs_mgr/libfiemap/binder.cpp index f99055af6..96c36ed7c 100644 --- a/fs_mgr/libfiemap/binder.cpp +++ b/fs_mgr/libfiemap/binder.cpp @@ -17,6 +17,7 @@ #if !defined(__ANDROID_RECOVERY__) #include #include +#include #include #include #include @@ -29,10 +30,29 @@ namespace fiemap { using namespace android::gsi; using namespace std::chrono_literals; +class ProgressCallback final : public BnProgressCallback { + public: + ProgressCallback(std::function&& callback) + : callback_(std::move(callback)) { + CHECK(callback_); + } + android::binder::Status onProgress(int64_t current, int64_t total) { + if (callback_(static_cast(current), static_cast(total))) { + return android::binder::Status::ok(); + } + return android::binder::Status::fromServiceSpecificError(UNKNOWN_ERROR, + "Progress callback failed"); + } + + private: + std::function callback_; +}; + class ImageManagerBinder final : public IImageManager { public: ImageManagerBinder(android::sp&& service, android::sp&& manager); - bool CreateBackingImage(const std::string& name, uint64_t size, int flags) override; + FiemapStatus CreateBackingImage(const std::string& name, uint64_t size, int flags, + std::function&& on_progress) override; bool DeleteBackingImage(const std::string& name) override; bool MapImageDevice(const std::string& name, const std::chrono::milliseconds& timeout_ms, std::string* path) override; @@ -41,7 +61,7 @@ class ImageManagerBinder final : public IImageManager { bool IsImageMapped(const std::string& name) override; bool MapImageWithDeviceMapper(const IPartitionOpener& opener, const std::string& name, std::string* dev) override; - bool ZeroFillNewImage(const std::string& name, uint64_t bytes) override; + FiemapStatus ZeroFillNewImage(const std::string& name, uint64_t bytes) override; bool RemoveAllImages() override; bool DisableImage(const std::string& name) override; bool RemoveDisabledImages() override; @@ -55,18 +75,31 @@ class ImageManagerBinder final : public IImageManager { android::sp manager_; }; +static FiemapStatus ToFiemapStatus(const char* func, const binder::Status& status) { + if (!status.isOk()) { + LOG(ERROR) << func << " binder returned: " << status.toString8().string(); + if (status.serviceSpecificErrorCode() != 0) { + return FiemapStatus::FromErrorCode(status.serviceSpecificErrorCode()); + } else { + return FiemapStatus::Error(); + } + } + return FiemapStatus::Ok(); +} + ImageManagerBinder::ImageManagerBinder(android::sp&& service, android::sp&& manager) : service_(std::move(service)), manager_(std::move(manager)) {} -bool ImageManagerBinder::CreateBackingImage(const std::string& name, uint64_t size, int flags) { - auto status = manager_->createBackingImage(name, size, flags); - if (!status.isOk()) { - LOG(ERROR) << __PRETTY_FUNCTION__ - << " binder returned: " << status.exceptionMessage().string(); - return false; +FiemapStatus ImageManagerBinder::CreateBackingImage( + const std::string& name, uint64_t size, int flags, + std::function&& on_progress) { + sp callback = nullptr; + if (on_progress) { + callback = new ProgressCallback(std::move(on_progress)); } - return true; + auto status = manager_->createBackingImage(name, size, flags, callback); + return ToFiemapStatus(__PRETTY_FUNCTION__, status); } bool ImageManagerBinder::DeleteBackingImage(const std::string& name) { @@ -147,14 +180,9 @@ std::vector ImageManagerBinder::GetAllBackingImages() { return retval; } -bool ImageManagerBinder::ZeroFillNewImage(const std::string& name, uint64_t bytes) { +FiemapStatus ImageManagerBinder::ZeroFillNewImage(const std::string& name, uint64_t bytes) { auto status = manager_->zeroFillNewImage(name, bytes); - if (!status.isOk()) { - LOG(ERROR) << __PRETTY_FUNCTION__ - << " binder returned: " << status.exceptionMessage().string(); - return false; - } - return true; + return ToFiemapStatus(__PRETTY_FUNCTION__, status); } bool ImageManagerBinder::RemoveAllImages() { diff --git a/fs_mgr/libfiemap/image_manager.cpp b/fs_mgr/libfiemap/image_manager.cpp index baa5de430..30eb5a035 100644 --- a/fs_mgr/libfiemap/image_manager.cpp +++ b/fs_mgr/libfiemap/image_manager.cpp @@ -133,27 +133,25 @@ bool ImageManager::BackingImageExists(const std::string& name) { return access(header_file.c_str(), F_OK) == 0; } -bool ImageManager::CreateBackingImage(const std::string& name, uint64_t size, int flags) { - return CreateBackingImage(name, size, flags, nullptr); -} - static bool IsUnreliablePinningAllowed(const std::string& path) { return android::base::StartsWith(path, "/data/gsi/dsu/") || android::base::StartsWith(path, "/data/gsi/test/") || android::base::StartsWith(path, "/data/gsi/ota/test/"); } -bool ImageManager::CreateBackingImage(const std::string& name, uint64_t size, int flags, - std::function&& on_progress) { +FiemapStatus ImageManager::CreateBackingImage( + const std::string& name, uint64_t size, int flags, + std::function&& on_progress) { auto data_path = GetImageHeaderPath(name); - auto fw = SplitFiemap::Create(data_path, size, 0, on_progress); - if (!fw) { - return false; + std::unique_ptr fw; + auto status = SplitFiemap::Create(data_path, size, 0, &fw, on_progress); + if (!status.is_ok()) { + return status; } bool reliable_pinning; if (!FilesystemHasReliablePinning(data_path, &reliable_pinning)) { - return false; + return FiemapStatus::Error(); } if (!reliable_pinning && !IsUnreliablePinningAllowed(data_path)) { // For historical reasons, we allow unreliable pinning for certain use @@ -164,7 +162,7 @@ bool ImageManager::CreateBackingImage(const std::string& name, uint64_t size, in // proper pinning. LOG(ERROR) << "File system does not have reliable block pinning"; SplitFiemap::RemoveSplitFiles(data_path); - return false; + return FiemapStatus::Error(); } // Except for testing, we do not allow persisting metadata that references @@ -180,24 +178,25 @@ bool ImageManager::CreateBackingImage(const std::string& name, uint64_t size, in fw = {}; SplitFiemap::RemoveSplitFiles(data_path); - return false; + return FiemapStatus::Error(); } bool readonly = !!(flags & CREATE_IMAGE_READONLY); if (!UpdateMetadata(metadata_dir_, name, fw.get(), size, readonly)) { - return false; + return FiemapStatus::Error(); } if (flags & CREATE_IMAGE_ZERO_FILL) { - if (!ZeroFillNewImage(name, 0)) { + auto res = ZeroFillNewImage(name, 0); + if (!res.is_ok()) { DeleteBackingImage(name); - return false; + return res; } } - return true; + return FiemapStatus::Ok(); } -bool ImageManager::ZeroFillNewImage(const std::string& name, uint64_t bytes) { +FiemapStatus ImageManager::ZeroFillNewImage(const std::string& name, uint64_t bytes) { auto data_path = GetImageHeaderPath(name); // See the comment in MapImageDevice() about how this works. @@ -205,13 +204,13 @@ bool ImageManager::ZeroFillNewImage(const std::string& name, uint64_t bytes) { bool can_use_devicemapper; if (!FiemapWriter::GetBlockDeviceForFile(data_path, &block_device, &can_use_devicemapper)) { LOG(ERROR) << "Could not determine block device for " << data_path; - return false; + return FiemapStatus::Error(); } if (!can_use_devicemapper) { // We've backed with loop devices, and since we store files in an // unencrypted folder, the initial zeroes we wrote will suffice. - return true; + return FiemapStatus::Ok(); } // data is dm-crypt, or FBE + dm-default-key. This means the zeroes written @@ -219,7 +218,7 @@ bool ImageManager::ZeroFillNewImage(const std::string& name, uint64_t bytes) { // this. auto device = MappedDevice::Open(this, 10s, name); if (!device) { - return false; + return FiemapStatus::Error(); } static constexpr size_t kChunkSize = 4096; @@ -232,7 +231,7 @@ bool ImageManager::ZeroFillNewImage(const std::string& name, uint64_t bytes) { remaining = get_block_device_size(device->fd()); if (!remaining) { PLOG(ERROR) << "Could not get block device size for " << device->path(); - return false; + return FiemapStatus::FromErrno(errno); } } while (remaining) { @@ -240,11 +239,11 @@ bool ImageManager::ZeroFillNewImage(const std::string& name, uint64_t bytes) { if (!android::base::WriteFully(device->fd(), zeroes.data(), static_cast(to_write))) { PLOG(ERROR) << "write failed: " << device->path(); - return false; + return FiemapStatus::FromErrno(errno); } remaining -= to_write; } - return true; + return FiemapStatus::Ok(); } bool ImageManager::DeleteBackingImage(const std::string& name) { diff --git a/fs_mgr/libfiemap/include/libfiemap/fiemap_status.h b/fs_mgr/libfiemap/include/libfiemap/fiemap_status.h index 2444ba83a..56917cc5f 100644 --- a/fs_mgr/libfiemap/include/libfiemap/fiemap_status.h +++ b/fs_mgr/libfiemap/include/libfiemap/fiemap_status.h @@ -37,6 +37,12 @@ class FiemapStatus { // Create from a given errno (specified in errno,h) static FiemapStatus FromErrno(int error_num) { return FiemapStatus(CastErrorCode(-error_num)); } + // Create from an integer error code that is expected to be an ErrorCode + // value. If it isn't, Error() is returned. + static FiemapStatus FromErrorCode(int32_t error_code) { + return FiemapStatus(CastErrorCode(error_code)); + } + // Generic error. static FiemapStatus Error() { return FiemapStatus(ErrorCode::ERROR); } diff --git a/fs_mgr/libfiemap/include/libfiemap/image_manager.h b/fs_mgr/libfiemap/include/libfiemap/image_manager.h index 7b907c036..2c1322956 100644 --- a/fs_mgr/libfiemap/include/libfiemap/image_manager.h +++ b/fs_mgr/libfiemap/include/libfiemap/image_manager.h @@ -25,6 +25,7 @@ #include #include +#include #include namespace android { @@ -52,7 +53,9 @@ class IImageManager { // of the image is undefined. If zero-fill is requested, and the operation // cannot be completed, the image will be deleted and this function will // return false. - virtual bool CreateBackingImage(const std::string& name, uint64_t size, int flags) = 0; + virtual FiemapStatus CreateBackingImage( + const std::string& name, uint64_t size, int flags, + std::function&& on_progress = nullptr) = 0; // Delete an image created with CreateBackingImage. Its entry will be // removed from the associated lp_metadata file. @@ -113,7 +116,7 @@ class IImageManager { // Writes |bytes| zeros to |name| file. If |bytes| is 0, then the // whole file if filled with zeros. - virtual bool ZeroFillNewImage(const std::string& name, uint64_t bytes) = 0; + virtual FiemapStatus ZeroFillNewImage(const std::string& name, uint64_t bytes) = 0; // Find and remove all images and metadata for this manager. virtual bool RemoveAllImages() = 0; @@ -133,7 +136,8 @@ class ImageManager final : public IImageManager { static std::unique_ptr Open(const std::string& dir_prefix); // Methods that must be implemented from IImageManager. - bool CreateBackingImage(const std::string& name, uint64_t size, int flags) override; + FiemapStatus CreateBackingImage(const std::string& name, uint64_t size, int flags, + std::function&& on_progress) override; bool DeleteBackingImage(const std::string& name) override; bool MapImageDevice(const std::string& name, const std::chrono::milliseconds& timeout_ms, std::string* path) override; @@ -149,9 +153,6 @@ class ImageManager final : public IImageManager { bool MapAllImages(const std::function)>& init) override; std::vector GetAllBackingImages(); - // Same as CreateBackingImage, but provides a progress notification. - bool CreateBackingImage(const std::string& name, uint64_t size, int flags, - std::function&& on_progress); // Returns true if the named partition exists. This does not check the // consistency of the backing image/data file. @@ -164,7 +165,7 @@ class ImageManager final : public IImageManager { void set_partition_opener(std::unique_ptr&& opener); // Writes |bytes| zeros at the beginning of the passed image - bool ZeroFillNewImage(const std::string& name, uint64_t bytes); + FiemapStatus ZeroFillNewImage(const std::string& name, uint64_t bytes); private: ImageManager(const std::string& metadata_dir, const std::string& data_dir); diff --git a/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h b/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h index 2bf1b57b7..11de6edde 100644 --- a/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h +++ b/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h @@ -14,10 +14,12 @@ #pragma once +#include #include #include #include +#include #include #include #include @@ -155,5 +157,28 @@ void SetSize(PartitionUpdate* partition_update, uint64_t size); // Get partition size from update package metadata. uint64_t GetSize(PartitionUpdate* partition_update); +// Util class for test cases on low space scenario. These tests assumes image manager +// uses /data as backup device. +class LowSpaceUserdata { + public: + // Set the maximum free space allowed for this test. If /userdata has more space than the given + // number, a file is allocated to consume space. + AssertionResult Init(uint64_t max_free_space); + + uint64_t free_space() const; + uint64_t available_space() const; + uint64_t bsize() const; + + private: + AssertionResult ReadUserdataStats(); + + static constexpr const char* kUserDataDevice = "/data"; + std::unique_ptr big_file_; + bool initialized_ = false; + uint64_t free_space_ = 0; + uint64_t available_space_ = 0; + uint64_t bsize_ = 0; +}; + } // namespace snapshot } // namespace android diff --git a/fs_mgr/libsnapshot/snapshot_test.cpp b/fs_mgr/libsnapshot/snapshot_test.cpp index 2da01033b..9e2719f07 100644 --- a/fs_mgr/libsnapshot/snapshot_test.cpp +++ b/fs_mgr/libsnapshot/snapshot_test.cpp @@ -47,6 +47,7 @@ namespace snapshot { using android::base::unique_fd; using android::dm::DeviceMapper; using android::dm::DmDeviceState; +using android::fiemap::FiemapStatus; using android::fiemap::IImageManager; using android::fs_mgr::BlockDeviceInfo; using android::fs_mgr::CreateLogicalPartitionParams; @@ -1700,6 +1701,55 @@ INSTANTIATE_TEST_SUITE_P(Snapshot, FlashAfterUpdateTest, Combine(Values(0, 1), B "Merge"s; }); +// Test behavior of ImageManager::Create on low space scenario. These tests assumes image manager +// uses /data as backup device. +class ImageManagerTest : public SnapshotTest, public WithParamInterface { + public: + void SetUp() override { + SnapshotTest::SetUp(); + userdata_ = std::make_unique(); + ASSERT_TRUE(userdata_->Init(GetParam())); + } + void TearDown() override { + EXPECT_TRUE(!image_manager_->BackingImageExists(kImageName) || + image_manager_->DeleteBackingImage(kImageName)); + } + static constexpr const char* kImageName = "my_image"; + std::unique_ptr userdata_; +}; + +TEST_P(ImageManagerTest, CreateImageEnoughAvailSpace) { + if (userdata_->available_space() == 0) { + GTEST_SKIP() << "/data is full (" << userdata_->available_space() + << " bytes available), skipping"; + } + ASSERT_TRUE(image_manager_->CreateBackingImage(kImageName, userdata_->available_space(), + IImageManager::CREATE_IMAGE_DEFAULT)) + << "Should be able to create image with size = " << userdata_->available_space() + << " bytes"; + ASSERT_TRUE(image_manager_->DeleteBackingImage(kImageName)) + << "Should be able to delete created image"; +} + +TEST_P(ImageManagerTest, CreateImageNoSpace) { + uint64_t to_allocate = userdata_->free_space() + userdata_->bsize(); + auto res = image_manager_->CreateBackingImage(kImageName, to_allocate, + IImageManager::CREATE_IMAGE_DEFAULT); + ASSERT_FALSE(res) << "Should not be able to create image with size = " << to_allocate + << " bytes because only " << userdata_->free_space() << " bytes are free"; + ASSERT_EQ(FiemapStatus::ErrorCode::NO_SPACE, res.error_code()) << res.string(); +} + +std::vector ImageManagerTestParams() { + std::vector ret; + for (uint64_t size = 1_MiB; size <= 512_MiB; size *= 2) { + ret.push_back(size); + } + return ret; +} + +INSTANTIATE_TEST_SUITE_P(ImageManagerTest, ImageManagerTest, ValuesIn(ImageManagerTestParams())); + } // namespace snapshot } // namespace android diff --git a/fs_mgr/libsnapshot/test_helpers.cpp b/fs_mgr/libsnapshot/test_helpers.cpp index f7f25afd4..b036606e2 100644 --- a/fs_mgr/libsnapshot/test_helpers.cpp +++ b/fs_mgr/libsnapshot/test_helpers.cpp @@ -14,8 +14,11 @@ #include +#include + #include #include +#include #include #include #include @@ -167,5 +170,67 @@ uint64_t GetSize(PartitionUpdate* partition_update) { return partition_update->mutable_new_partition_info()->size(); } +AssertionResult LowSpaceUserdata::Init(uint64_t max_free_space) { + auto res = ReadUserdataStats(); + if (!res) return res; + + // Try to fill up the disk as much as possible until free_space_ <= max_free_space. + big_file_ = std::make_unique(); + if (big_file_->fd == -1) { + return AssertionFailure() << strerror(errno); + } + if (!android::base::StartsWith(big_file_->path, kUserDataDevice)) { + return AssertionFailure() << "Temp file allocated to " << big_file_->path << ", not in " + << kUserDataDevice; + } + uint64_t next_consume = + std::min(free_space_ - max_free_space, (uint64_t)std::numeric_limits::max()); + off_t allocated = 0; + while (next_consume > 0 && free_space_ > max_free_space) { + int status = fallocate(big_file_->fd, 0, allocated, next_consume); + if (status == -1 && errno == ENOSPC) { + next_consume /= 2; + continue; + } + if (status == -1) { + return AssertionFailure() << strerror(errno); + } + allocated += next_consume; + + res = ReadUserdataStats(); + if (!res) return res; + } + + LOG(INFO) << allocated << " bytes allocated to " << big_file_->path; + initialized_ = true; + return AssertionSuccess(); +} + +AssertionResult LowSpaceUserdata::ReadUserdataStats() { + struct statvfs buf; + if (statvfs(kUserDataDevice, &buf) == -1) { + return AssertionFailure() << strerror(errno); + } + bsize_ = buf.f_bsize; + free_space_ = buf.f_bsize * buf.f_bfree; + available_space_ = buf.f_bsize * buf.f_bavail; + return AssertionSuccess(); +} + +uint64_t LowSpaceUserdata::free_space() const { + CHECK(initialized_); + return free_space_; +} + +uint64_t LowSpaceUserdata::available_space() const { + CHECK(initialized_); + return available_space_; +} + +uint64_t LowSpaceUserdata::bsize() const { + CHECK(initialized_); + return bsize_; +} + } // namespace snapshot } // namespace android