libsnapshot: Remove GetSnapshotSize
A single number is not enough to represent the ranges
that needs to be snapshotted in the target partition. For
prototyping now, just use device_size instead.
Add test in PartitionCowCreatorTest to ensure that this
doesn't regress. Also fix some trivial tests.
In follow-up CLs, snapshot ranges should be represented
with a more complicated structure. See b/141889746.
Bug: 141889746
Test: libsnapshot_test
Change-Id: I1a508c2464abce216ad4049cc2533ffdaa8cd14f
diff --git a/fs_mgr/libsnapshot/partition_cow_creator.cpp b/fs_mgr/libsnapshot/partition_cow_creator.cpp
index 4250d23..404ef27 100644
--- a/fs_mgr/libsnapshot/partition_cow_creator.cpp
+++ b/fs_mgr/libsnapshot/partition_cow_creator.cpp
@@ -67,59 +67,6 @@
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<uint64_t> PartitionCowCreator::GetSnapshotSize() {
- // Compute the number of sectors that needs to be snapshotted.
- uint64_t snapshot_sectors = 0;
- std::vector<std::unique_ptr<Extent>> 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<uint64_t> 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 @@
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 7c81418..0e645c6 100644
--- a/fs_mgr/libsnapshot/partition_cow_creator.h
+++ b/fs_mgr/libsnapshot/partition_cow_creator.h
@@ -59,7 +59,6 @@
private:
bool HasExtent(Partition* p, Extent* e);
- std::optional<uint64_t> GetSnapshotSize();
std::optional<uint64_t> 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 4c9afff..ccd087e 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 @@
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<BlockDeviceInfo> 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 c94fde5..f3994c1 100644
--- a/fs_mgr/libsnapshot/snapshot_test.cpp
+++ b/fs_mgr/libsnapshot/snapshot_test.cpp
@@ -58,15 +58,11 @@
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<SnapshotManager> 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 @@
}
// 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 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 bb19adc..769d21e 100644
--- a/fs_mgr/libsnapshot/test_helpers.h
+++ b/fs_mgr/libsnapshot/test_helpers.h
@@ -23,6 +23,7 @@
#include <liblp/mock_property_fetcher.h>
#include <liblp/partition_opener.h>
#include <libsnapshot/snapshot.h>
+#include <storage_literals/storage_literals.h>
#include <update_engine/update_metadata.pb.h>
namespace android {
@@ -38,8 +39,17 @@
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<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;
+
// Redirect requests for "super" to our fake super partition.
class TestPartitionOpener final : public android::fs_mgr::PartitionOpener {
public: