diff --git a/fs_mgr/libsnapshot/libsnapshot_cow/cow_writer.cpp b/fs_mgr/libsnapshot/libsnapshot_cow/cow_writer.cpp index 3932fad94..56b48f0c7 100644 --- a/fs_mgr/libsnapshot/libsnapshot_cow/cow_writer.cpp +++ b/fs_mgr/libsnapshot/libsnapshot_cow/cow_writer.cpp @@ -298,13 +298,12 @@ bool CowWriter::Initialize(borrowed_fd fd) { return false; } - bool ret = OpenForWrite(); - - if (ret) { - InitWorkers(); + if (!OpenForWrite()) { + return false; } - return ret; + InitWorkers(); + return true; } bool CowWriter::InitializeAppend(android::base::unique_fd&& fd, uint64_t label) { diff --git a/fs_mgr/libsnapshot/snapshot_test.cpp b/fs_mgr/libsnapshot/snapshot_test.cpp index 40143802b..13314daab 100644 --- a/fs_mgr/libsnapshot/snapshot_test.cpp +++ b/fs_mgr/libsnapshot/snapshot_test.cpp @@ -2669,44 +2669,46 @@ 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 { +class ImageManagerTest : public SnapshotTest { protected: void SetUp() override { SKIP_IF_NON_VIRTUAL_AB(); SnapshotTest::SetUp(); - userdata_ = std::make_unique(); - ASSERT_TRUE(userdata_->Init(GetParam())); } void TearDown() override { RETURN_IF_NON_VIRTUAL_AB(); - + CleanUp(); + } + void CleanUp() { EXPECT_TRUE(!image_manager_->BackingImageExists(kImageName) || image_manager_->DeleteBackingImage(kImageName)); } + static constexpr const char* kImageName = "my_image"; - std::unique_ptr userdata_; }; -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; +TEST_F(ImageManagerTest, CreateImageNoSpace) { + bool at_least_one_failure = false; for (uint64_t size = 1_MiB; size <= 512_MiB; size *= 2) { - ret.push_back(size); - } - return ret; -} + auto userdata = std::make_unique(); + ASSERT_TRUE(userdata->Init(size)); -INSTANTIATE_TEST_SUITE_P(ImageManagerTest, ImageManagerTest, ValuesIn(ImageManagerTestParams())); + uint64_t to_allocate = userdata->free_space() + userdata->bsize(); + + auto res = image_manager_->CreateBackingImage(kImageName, to_allocate, + IImageManager::CREATE_IMAGE_DEFAULT); + if (!res) { + at_least_one_failure = true; + } else { + ASSERT_EQ(res.error_code(), FiemapStatus::ErrorCode::NO_SPACE) << res.string(); + } + + CleanUp(); + } + + ASSERT_TRUE(at_least_one_failure) + << "We should have failed to allocate at least one over-sized image"; +} bool Mkdir(const std::string& path) { if (mkdir(path.c_str(), 0700) && errno != EEXIST) { diff --git a/fs_mgr/libsnapshot/snapshot_writer_test.cpp b/fs_mgr/libsnapshot/snapshot_writer_test.cpp index da48eb933..a03632b46 100644 --- a/fs_mgr/libsnapshot/snapshot_writer_test.cpp +++ b/fs_mgr/libsnapshot/snapshot_writer_test.cpp @@ -33,8 +33,8 @@ TEST_F(CompressedSnapshotWriterTest, ReadAfterWrite) { TemporaryFile cow_device_file{}; android::snapshot::CowOptions options{.block_size = BLOCK_SIZE}; android::snapshot::CompressedSnapshotWriter snapshot_writer{options}; - snapshot_writer.SetCowDevice(android::base::unique_fd{cow_device_file.fd}); - snapshot_writer.Initialize(); + ASSERT_TRUE(snapshot_writer.SetCowDevice(android::base::unique_fd{cow_device_file.fd})); + ASSERT_TRUE(snapshot_writer.Initialize()); std::vector buffer; buffer.resize(BLOCK_SIZE); std::fill(buffer.begin(), buffer.end(), 123); diff --git a/fs_mgr/libsnapshot/test_helpers.cpp b/fs_mgr/libsnapshot/test_helpers.cpp index b05123a23..9f1d6764f 100644 --- a/fs_mgr/libsnapshot/test_helpers.cpp +++ b/fs_mgr/libsnapshot/test_helpers.cpp @@ -227,7 +227,7 @@ AssertionResult LowSpaceUserdata::Init(uint64_t max_free_space) { return AssertionFailure() << "Temp file allocated to " << big_file_->path << ", not in " << kUserDataDevice; } - uint64_t next_consume = std::min(available_space_ - max_free_space, + uint64_t next_consume = std::min(std::max(available_space_, max_free_space) - max_free_space, (uint64_t)std::numeric_limits::max()); off_t allocated = 0; while (next_consume > 0 && free_space_ > max_free_space) {