Merge changes I1da95645,I1a2219b5,I57dbf204
* changes:
libsnapshot: Fix incorrect CHECK in PerformInitTransition().
libsnapshot: Propagate the compression bit across state changes.
libsnapshot: Adjust partition sizes so tests pass with or without compression.
diff --git a/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h b/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h
index 7aef086..ff0047e 100644
--- a/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h
+++ b/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h
@@ -51,8 +51,8 @@
extern std::unique_ptr<SnapshotManager> sm;
extern class TestDeviceInfo* test_device;
extern std::string fake_super;
-static constexpr uint64_t kSuperSize = 16_MiB + 4_KiB;
-static constexpr uint64_t kGroupSize = 16_MiB;
+static constexpr uint64_t kSuperSize = 32_MiB + 4_KiB;
+static constexpr uint64_t kGroupSize = 32_MiB;
// Redirect requests for "super" to our fake super partition.
class TestPartitionOpener final : public android::fs_mgr::PartitionOpener {
diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp
index f0646fc..21b720e 100644
--- a/fs_mgr/libsnapshot/snapshot.cpp
+++ b/fs_mgr/libsnapshot/snapshot.cpp
@@ -676,6 +676,8 @@
}
}
+ bool compression_enabled = false;
+
uint64_t total_cow_file_size = 0;
DmTargetSnapshot::Status initial_target_values = {};
for (const auto& snapshot : snapshots) {
@@ -692,6 +694,8 @@
return false;
}
total_cow_file_size += snapshot_status.cow_file_size();
+
+ compression_enabled |= snapshot_status.compression_enabled();
}
if (cow_file_size) {
@@ -703,6 +707,7 @@
initial_status.set_sectors_allocated(initial_target_values.sectors_allocated);
initial_status.set_total_sectors(initial_target_values.total_sectors);
initial_status.set_metadata_sectors(initial_target_values.metadata_sectors);
+ initial_status.set_compression_enabled(compression_enabled);
// Point of no return - mark that we're starting a merge. From now on every
// snapshot must be a merge target.
@@ -1405,7 +1410,7 @@
return false;
}
- CHECK(base_sectors == target.spec.length);
+ CHECK(base_sectors <= target.spec.length);
if (!snapuserd_client_->AttachDmUser(misc_name)) {
// This error is unrecoverable. We cannot proceed because reads to
@@ -2285,9 +2290,15 @@
}
bool SnapshotManager::WriteUpdateState(LockedFile* lock, UpdateState state) {
- SnapshotUpdateStatus status = {};
+ SnapshotUpdateStatus status;
status.set_state(state);
- status.set_compression_enabled(IsCompressionEnabled());
+
+ // If we're transitioning between two valid states (eg, we're not beginning
+ // or ending an OTA), then make sure to propagate the compression bit.
+ if (!(state == UpdateState::Initiated || state == UpdateState::None)) {
+ SnapshotUpdateStatus old_status = ReadSnapshotUpdateStatus(lock);
+ status.set_compression_enabled(old_status.compression_enabled());
+ }
return WriteSnapshotUpdateStatus(lock, status);
}
@@ -2477,6 +2488,12 @@
auto lock = LockExclusive();
if (!lock) return Return::Error();
+ auto update_state = ReadUpdateState(lock.get());
+ if (update_state != UpdateState::Initiated) {
+ LOG(ERROR) << "Cannot create update snapshots in state " << update_state;
+ 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
// retrofit Virtual A/B devices.
@@ -2571,6 +2588,14 @@
return Return::Error();
}
+ SnapshotUpdateStatus status = {};
+ status.set_state(update_state);
+ status.set_compression_enabled(cow_creator.compression_enabled);
+ if (!WriteSnapshotUpdateStatus(lock.get(), status)) {
+ LOG(ERROR) << "Unable to write new update state";
+ return Return::Error();
+ }
+
created_devices.Release();
LOG(INFO) << "Successfully created all snapshots for target slot " << target_suffix;
diff --git a/fs_mgr/libsnapshot/snapshot_test.cpp b/fs_mgr/libsnapshot/snapshot_test.cpp
index bb44425..80a6074 100644
--- a/fs_mgr/libsnapshot/snapshot_test.cpp
+++ b/fs_mgr/libsnapshot/snapshot_test.cpp
@@ -854,15 +854,15 @@
group_->add_partition_names("prd");
sys_ = manifest_.add_partitions();
sys_->set_partition_name("sys");
- sys_->set_estimate_cow_size(6_MiB);
+ sys_->set_estimate_cow_size(2_MiB);
SetSize(sys_, 3_MiB);
vnd_ = manifest_.add_partitions();
vnd_->set_partition_name("vnd");
- vnd_->set_estimate_cow_size(6_MiB);
+ vnd_->set_estimate_cow_size(2_MiB);
SetSize(vnd_, 3_MiB);
prd_ = manifest_.add_partitions();
prd_->set_partition_name("prd");
- prd_->set_estimate_cow_size(6_MiB);
+ prd_->set_estimate_cow_size(2_MiB);
SetSize(prd_, 3_MiB);
// Initialize source partition metadata using |manifest_|.
@@ -1050,11 +1050,17 @@
ASSERT_TRUE(sm->UnmapUpdateSnapshot(name));
}
- // Grow all partitions.
+ // Grow all partitions. Set |prd| large enough that |sys| and |vnd|'s COWs
+ // fit in super, but not |prd|.
constexpr uint64_t partition_size = 3788_KiB;
SetSize(sys_, partition_size);
SetSize(vnd_, partition_size);
- SetSize(prd_, partition_size);
+ SetSize(prd_, 18_MiB);
+
+ // Make sure |prd| does not fit in super at all. On VABC, this means we
+ // fake an extra large COW for |vnd| to fill up super.
+ vnd_->set_estimate_cow_size(30_MiB);
+ prd_->set_estimate_cow_size(30_MiB);
AddOperationForPartitions();
@@ -1066,11 +1072,7 @@
auto tgt = MetadataBuilder::New(*opener_, "super", 1);
ASSERT_NE(tgt, nullptr);
ASSERT_NE(nullptr, tgt->FindPartition("sys_b-cow"));
- if (IsCompressionEnabled()) {
- ASSERT_EQ(nullptr, tgt->FindPartition("vnd_b-cow"));
- } else {
- ASSERT_NE(nullptr, tgt->FindPartition("vnd_b-cow"));
- }
+ ASSERT_NE(nullptr, tgt->FindPartition("vnd_b-cow"));
ASSERT_EQ(nullptr, tgt->FindPartition("prd_b-cow"));
// Write some data to target partitions.
@@ -1104,6 +1106,7 @@
// Initiate the merge and wait for it to be completed.
ASSERT_TRUE(init->InitiateMerge());
+ ASSERT_EQ(init->IsSnapuserdRequired(), IsCompressionEnabled());
ASSERT_EQ(UpdateState::MergeCompleted, init->ProcessUpdateState());
// Check that the target partitions have the same content after the merge.
@@ -1128,6 +1131,7 @@
SetSize(sys_, 4_MiB); // grows
SetSize(vnd_, 2_MiB); // shrinks
// prd_b is unchanged
+ ASSERT_TRUE(sm->BeginUpdate());
ASSERT_TRUE(sm->CreateUpdateSnapshots(manifest_));
ASSERT_EQ(4_MiB, GetSnapshotSize("sys_b").value_or(0));
}
@@ -1137,6 +1141,7 @@
TEST_F(SnapshotUpdateTest, CowPartitionDoNotTakeOldPartitions) {
SetSize(sys_, 2_MiB); // shrinks
// vnd_b and prd_b are unchanged.
+ ASSERT_TRUE(sm->BeginUpdate());
ASSERT_TRUE(sm->CreateUpdateSnapshots(manifest_));
auto tgt = MetadataBuilder::New(*opener_, "super", 1);
@@ -1260,6 +1265,11 @@
// Test that at the second update, old COW partition spaces are reclaimed.
TEST_F(SnapshotUpdateTest, ReclaimCow) {
+ // Make sure VABC cows are small enough that they fit in fake_super.
+ sys_->set_estimate_cow_size(64_KiB);
+ vnd_->set_estimate_cow_size(64_KiB);
+ prd_->set_estimate_cow_size(64_KiB);
+
// Execute the first update.
ASSERT_TRUE(sm->BeginUpdate());
ASSERT_TRUE(sm->CreateUpdateSnapshots(manifest_));
@@ -1376,9 +1386,13 @@
TEST_F(SnapshotUpdateTest, MergeCannotRemoveCow) {
// Make source partitions as big as possible to force COW image to be created.
- SetSize(sys_, 5_MiB);
- SetSize(vnd_, 5_MiB);
- SetSize(prd_, 5_MiB);
+ SetSize(sys_, 10_MiB);
+ SetSize(vnd_, 10_MiB);
+ SetSize(prd_, 10_MiB);
+ sys_->set_estimate_cow_size(12_MiB);
+ vnd_->set_estimate_cow_size(12_MiB);
+ prd_->set_estimate_cow_size(12_MiB);
+
src_ = MetadataBuilder::New(*opener_, "super", 0);
ASSERT_NE(src_, nullptr);
src_->RemoveGroupAndPartitions(group_->name() + "_a");
@@ -1676,6 +1690,8 @@
SetSize(sys_, partition_size);
AddOperation(sys_, data_size);
+ sys_->set_estimate_cow_size(partition_size + data_size);
+
// Set hastree extents.
sys_->mutable_hash_tree_data_extent()->set_start_block(0);
sys_->mutable_hash_tree_data_extent()->set_num_blocks(data_size / block_size);
@@ -1716,6 +1732,10 @@
// Test for overflow bit after update
TEST_F(SnapshotUpdateTest, Overflow) {
+ if (IsCompressionEnabled()) {
+ GTEST_SKIP() << "No overflow bit set for userspace COWs";
+ }
+
const auto actual_write_size = GetSize(sys_);
const auto declared_write_size = actual_write_size - 1_MiB;
@@ -1743,12 +1763,15 @@
auto userdata = std::make_unique<LowSpaceUserdata>();
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;
+ // Grow all partitions to 10_MiB, total 30_MiB. This requires 30 MiB of CoW space. After
+ // using the empty space in super (< 1 MiB), it uses 30 MiB of /userdata space.
+ constexpr uint64_t partition_size = 10_MiB;
SetSize(sys_, partition_size);
SetSize(vnd_, partition_size);
SetSize(prd_, partition_size);
+ sys_->set_estimate_cow_size(partition_size);
+ vnd_->set_estimate_cow_size(partition_size);
+ prd_->set_estimate_cow_size(partition_size);
AddOperationForPartitions();
@@ -1758,7 +1781,7 @@
ASSERT_FALSE(res);
ASSERT_EQ(Return::ErrorCode::NO_SPACE, res.error_code());
ASSERT_GE(res.required_size(), 14_MiB);
- ASSERT_LT(res.required_size(), 15_MiB);
+ ASSERT_LT(res.required_size(), 40_MiB);
}
class AutoKill final {
