Merge changes I72cf6f52,I9620e5b4
* changes:
libsnapshot: Fix crash in cow writer test due to missing ASSERT.
libsnapshot: Improve low space tests.
diff --git a/fs_mgr/libsnapshot/libsnapshot_cow/cow_writer.cpp b/fs_mgr/libsnapshot/libsnapshot_cow/cow_writer.cpp
index 3932fad..56b48f0 100644
--- a/fs_mgr/libsnapshot/libsnapshot_cow/cow_writer.cpp
+++ b/fs_mgr/libsnapshot/libsnapshot_cow/cow_writer.cpp
@@ -298,13 +298,12 @@
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 4014380..13314da 100644
--- a/fs_mgr/libsnapshot/snapshot_test.cpp
+++ b/fs_mgr/libsnapshot/snapshot_test.cpp
@@ -2669,44 +2669,46 @@
"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<uint64_t> {
+class ImageManagerTest : public SnapshotTest {
protected:
void SetUp() override {
SKIP_IF_NON_VIRTUAL_AB();
SnapshotTest::SetUp();
- userdata_ = std::make_unique<LowSpaceUserdata>();
- 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<LowSpaceUserdata> 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<uint64_t> ImageManagerTestParams() {
- std::vector<uint64_t> 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<LowSpaceUserdata>();
+ 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 da48eb9..a03632b 100644
--- a/fs_mgr/libsnapshot/snapshot_writer_test.cpp
+++ b/fs_mgr/libsnapshot/snapshot_writer_test.cpp
@@ -33,8 +33,8 @@
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<unsigned char> 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 b05123a..9f1d676 100644
--- a/fs_mgr/libsnapshot/test_helpers.cpp
+++ b/fs_mgr/libsnapshot/test_helpers.cpp
@@ -227,7 +227,7 @@
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<off_t>::max());
off_t allocated = 0;
while (next_consume > 0 && free_space_ > max_free_space) {