diff --git a/fs_mgr/libsnapshot/partition_cow_creator.cpp b/fs_mgr/libsnapshot/partition_cow_creator.cpp index 4250d23e3..404ef2727 100644 --- a/fs_mgr/libsnapshot/partition_cow_creator.cpp +++ b/fs_mgr/libsnapshot/partition_cow_creator.cpp @@ -67,59 +67,6 @@ bool PartitionCowCreator::HasExtent(Partition* p, Extent* e) { return false; } -// Return the number of sectors, N, where |target_partition|[0..N] (from -// |target_metadata|) are the sectors that should be snapshotted. N is computed -// so that this range of sectors are used by partitions in |current_metadata|. -// -// The client code (update_engine) should have computed target_metadata by -// resizing partitions of current_metadata, so only the first N sectors should -// be snapshotted, not a range with start index != 0. -// -// Note that if partition A has shrunk and partition B has grown, the new -// extents of partition B may use the empty space that was used by partition A. -// In this case, that new extent cannot be written directly, as it may be used -// by the running system. Hence, all extents of the new partition B must be -// intersected with all old partitions (including old partition A and B) to get -// the region that needs to be snapshotted. -std::optional PartitionCowCreator::GetSnapshotSize() { - // Compute the number of sectors that needs to be snapshotted. - uint64_t snapshot_sectors = 0; - std::vector> intersections; - for (const auto& extent : target_partition->extents()) { - for (auto* existing_partition : - ListPartitionsWithSuffix(current_metadata, current_suffix)) { - for (const auto& existing_extent : existing_partition->extents()) { - auto intersection = Intersect(extent.get(), existing_extent.get()); - if (intersection != nullptr && intersection->num_sectors() > 0) { - snapshot_sectors += intersection->num_sectors(); - intersections.emplace_back(std::move(intersection)); - } - } - } - } - uint64_t snapshot_size = snapshot_sectors * kSectorSize; - - // Sanity check that all recorded intersections are indeed within - // target_partition[0..snapshot_sectors]. - Partition target_partition_snapshot = target_partition->GetBeginningExtents(snapshot_size); - for (const auto& intersection : intersections) { - if (!HasExtent(&target_partition_snapshot, intersection.get())) { - auto linear_intersection = intersection->AsLinearExtent(); - LOG(ERROR) << "Extent " - << (linear_intersection - ? (std::to_string(linear_intersection->physical_sector()) + "," + - std::to_string(linear_intersection->end_sector())) - : "") - << " is not part of Partition " << target_partition->name() << "[0.." - << snapshot_size - << "]. The metadata wasn't constructed correctly. This should not happen."; - return std::nullopt; - } - } - - return snapshot_size; -} - std::optional PartitionCowCreator::GetCowSize(uint64_t snapshot_size) { // TODO: Use |operations|. to determine a minimum COW size. // kCowEstimateFactor is good for prototyping but we can't use that in production. @@ -139,12 +86,11 @@ std::optional PartitionCowCreator::Run() { Return ret; ret.snapshot_status.device_size = target_partition->size(); - auto snapshot_size = GetSnapshotSize(); - if (!snapshot_size.has_value()) return std::nullopt; + // TODO(b/141889746): Optimize by using a smaller snapshot. Some ranges in target_partition + // may be written directly. + ret.snapshot_status.snapshot_size = target_partition->size(); - ret.snapshot_status.snapshot_size = *snapshot_size; - - auto cow_size = GetCowSize(*snapshot_size); + auto cow_size = GetCowSize(ret.snapshot_status.snapshot_size); if (!cow_size.has_value()) return std::nullopt; // Compute regions that are free in both current and target metadata. These are the regions diff --git a/fs_mgr/libsnapshot/partition_cow_creator.h b/fs_mgr/libsnapshot/partition_cow_creator.h index 7c814189b..0e645c6d1 100644 --- a/fs_mgr/libsnapshot/partition_cow_creator.h +++ b/fs_mgr/libsnapshot/partition_cow_creator.h @@ -59,7 +59,6 @@ struct PartitionCowCreator { private: bool HasExtent(Partition* p, Extent* e); - std::optional GetSnapshotSize(); std::optional GetCowSize(uint64_t snapshot_size); }; diff --git a/fs_mgr/libsnapshot/partition_cow_creator_test.cpp b/fs_mgr/libsnapshot/partition_cow_creator_test.cpp index 4c9afff81..ccd087e71 100644 --- a/fs_mgr/libsnapshot/partition_cow_creator_test.cpp +++ b/fs_mgr/libsnapshot/partition_cow_creator_test.cpp @@ -20,7 +20,7 @@ #include "partition_cow_creator.h" #include "test_helpers.h" -using ::android::fs_mgr::MetadataBuilder; +using namespace android::fs_mgr; namespace android { namespace snapshot { @@ -55,5 +55,46 @@ TEST_F(PartitionCowCreatorTest, IntersectSelf) { ASSERT_EQ(40 * 1024, ret->snapshot_status.snapshot_size); } +TEST_F(PartitionCowCreatorTest, Holes) { + const auto& opener = test_device->GetPartitionOpener(); + + constexpr auto slack_space = 1_MiB; + constexpr auto big_size = (kSuperSize - slack_space) / 2; + constexpr auto small_size = big_size / 2; + + BlockDeviceInfo super_device("super", kSuperSize, 0, 0, 4_KiB); + std::vector devices = {super_device}; + auto source = MetadataBuilder::New(devices, "super", 1024, 2); + auto system = source->AddPartition("system_a", 0); + ASSERT_NE(nullptr, system); + ASSERT_TRUE(source->ResizePartition(system, big_size)); + auto vendor = source->AddPartition("vendor_a", 0); + ASSERT_NE(nullptr, vendor); + ASSERT_TRUE(source->ResizePartition(vendor, big_size)); + // Create a hole between system and vendor + ASSERT_TRUE(source->ResizePartition(system, small_size)); + auto source_metadata = source->Export(); + ASSERT_NE(nullptr, source_metadata); + ASSERT_TRUE(FlashPartitionTable(opener, fake_super, *source_metadata.get())); + + auto target = MetadataBuilder::NewForUpdate(opener, "super", 0, 1); + // Shrink vendor + vendor = target->FindPartition("vendor_b"); + ASSERT_NE(nullptr, vendor); + ASSERT_TRUE(target->ResizePartition(vendor, small_size)); + // Grow system to take hole & saved space from vendor + system = target->FindPartition("system_b"); + ASSERT_NE(nullptr, system); + ASSERT_TRUE(target->ResizePartition(system, big_size * 2 - small_size)); + + PartitionCowCreator creator{.target_metadata = target.get(), + .target_suffix = "_b", + .target_partition = system, + .current_metadata = source.get(), + .current_suffix = "_a"}; + auto ret = creator.Run(); + ASSERT_TRUE(ret.has_value()); +} + } // namespace snapshot } // namespace android diff --git a/fs_mgr/libsnapshot/snapshot_test.cpp b/fs_mgr/libsnapshot/snapshot_test.cpp index c94fde568..f3994c1a0 100644 --- a/fs_mgr/libsnapshot/snapshot_test.cpp +++ b/fs_mgr/libsnapshot/snapshot_test.cpp @@ -58,15 +58,11 @@ using namespace android::storage_literals; using namespace std::chrono_literals; using namespace std::string_literals; -// These are not reset between each test because it's expensive to create -// these resources (starting+connecting to gsid, zero-filling images). +// Global states. See test_helpers.h. std::unique_ptr sm; TestDeviceInfo* test_device = nullptr; std::string fake_super; -static constexpr uint64_t kSuperSize = 16_MiB + 4_KiB; -static constexpr uint64_t kGroupSize = 16_MiB; - class SnapshotTest : public ::testing::Test { public: SnapshotTest() : dm_(DeviceMapper::Instance()) {} @@ -743,9 +739,9 @@ TEST_F(SnapshotUpdateTest, FullUpdateFlow) { } // Grow all partitions. - SetSize(sys_, 4_MiB); - SetSize(vnd_, 4_MiB); - SetSize(prd_, 4_MiB); + SetSize(sys_, 3788_KiB); + SetSize(vnd_, 3788_KiB); + SetSize(prd_, 3788_KiB); // Execute the update. ASSERT_TRUE(sm->BeginUpdate()); @@ -810,6 +806,7 @@ TEST_F(SnapshotUpdateTest, FullUpdateFlow) { // Test that if new system partitions uses empty space in super, that region is not snapshotted. TEST_F(SnapshotUpdateTest, DirectWriteEmptySpace) { + GTEST_SKIP() << "b/141889746"; SetSize(sys_, 4_MiB); // vnd_b and prd_b are unchanged. ASSERT_TRUE(sm->CreateUpdateSnapshots(manifest_)); diff --git a/fs_mgr/libsnapshot/test_helpers.h b/fs_mgr/libsnapshot/test_helpers.h index bb19adcef..769d21e57 100644 --- a/fs_mgr/libsnapshot/test_helpers.h +++ b/fs_mgr/libsnapshot/test_helpers.h @@ -23,6 +23,7 @@ #include #include #include +#include #include namespace android { @@ -38,8 +39,17 @@ using testing::AssertionResult; using testing::NiceMock; using testing::Return; +using namespace android::storage_literals; using namespace std::string_literals; +// These are not reset between each test because it's expensive to create +// these resources (starting+connecting to gsid, zero-filling images). +extern std::unique_ptr 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; + // Redirect requests for "super" to our fake super partition. class TestPartitionOpener final : public android::fs_mgr::PartitionOpener { public: