ACPR/android_system_core
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge "libsnapshot: Removed the unused "linear" optimization."

Browse source
This commit is contained in:
David Anderson 2021-01-21 20:49:42 +00:00 • committed by Gerrit Code Review
commit 531e15e38c
3 changed files with 36 additions and 177 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
View file

@ -569,12 +569,6 @@ class SnapshotManager final : public ISnapshotManager {
std::string GetRollbackIndicatorPath();
std::string GetForwardMergeIndicatorPath();
// Return the name of the device holding the "snapshot" or "snapshot-merge"
// target. This may not be the final device presented via MapSnapshot(), if
// for example there is a linear segment.
std::string GetSnapshotDeviceName(const std::string& snapshot_name,
const SnapshotStatus& status);
bool MapAllPartitions(LockedFile* lock, const std::string& super_device, uint32_t slot,
const std::chrono::milliseconds& timeout_ms);
@ -673,8 +667,8 @@ class SnapshotManager final : public ISnapshotManager {
// Helper for RemoveAllSnapshots.
// Check whether |name| should be deleted as a snapshot name.
bool ShouldDeleteSnapshot(LockedFile* lock, const std::map<std::string, bool>& flashing_status,
Slot current_slot, const std::string& name);
bool ShouldDeleteSnapshot(const std::map<std::string, bool>& flashing_status, Slot current_slot,
const std::string& name);
// Create or delete forward merge indicator given |wipe|. Iff wipe is scheduled,
// allow forward merge on FDR.

172
fs_mgr/libsnapshot/snapshot.cpp
View file

@ -132,10 +132,6 @@ static std::string GetBaseDeviceName(const std::string& partition_name) {
return partition_name + "-base";
}
static std::string GetSnapshotExtraDeviceName(const std::string& snapshot_name) {
return snapshot_name + "-inner";
}
bool SnapshotManager::BeginUpdate() {
bool needs_merge = false;
if (!TryCancelUpdate(&needs_merge)) {
@ -465,8 +461,13 @@ bool SnapshotManager::MapSnapshot(LockedFile* lock, const std::string& name,
LOG(ERROR) << "Invalid snapshot size for " << base_device << ": " << status.snapshot_size();
return false;
}
if (status.device_size() != status.snapshot_size()) {
LOG(ERROR) << "Device size and snapshot size must be the same (device size = "
<< status.device_size() << ", snapshot size = " << status.snapshot_size();
return false;
}
uint64_t snapshot_sectors = status.snapshot_size() / kSectorSize;
uint64_t linear_sectors = (status.device_size() - status.snapshot_size()) / kSectorSize;
auto& dm = DeviceMapper::Instance();
@ -491,45 +492,13 @@ bool SnapshotManager::MapSnapshot(LockedFile* lock, const std::string& name,
break;
}
// The kernel (tested on 4.19) crashes horribly if a device has both a snapshot
// and a linear target in the same table. Instead, we stack them, and give the
// snapshot device a different name. It is not exposed to the caller in this
// case.
auto snap_name = (linear_sectors > 0) ? GetSnapshotExtraDeviceName(name) : name;
DmTable table;
table.Emplace<DmTargetSnapshot>(0, snapshot_sectors, base_device, cow_device, mode,
kSnapshotChunkSize);
if (!dm.CreateDevice(snap_name, table, dev_path, timeout_ms)) {
LOG(ERROR) << "Could not create snapshot device: " << snap_name;
if (!dm.CreateDevice(name, table, dev_path, timeout_ms)) {
LOG(ERROR) << "Could not create snapshot device: " << name;
return false;
}
if (linear_sectors) {
std::string snap_dev;
if (!dm.GetDeviceString(snap_name, &snap_dev)) {
LOG(ERROR) << "Cannot determine major/minor for: " << snap_name;
return false;
}
// Our stacking will looks like this:
// [linear, linear] ; to snapshot, and non-snapshot region of base device
// [snapshot-inner]
// [base device] [cow]
DmTable table;
table.Emplace<DmTargetLinear>(0, snapshot_sectors, snap_dev, 0);
table.Emplace<DmTargetLinear>(snapshot_sectors, linear_sectors, base_device,
snapshot_sectors);
if (!dm.CreateDevice(name, table, dev_path, timeout_ms)) {
LOG(ERROR) << "Could not create outer snapshot device: " << name;
dm.DeleteDevice(snap_name);
return false;
}
}
// :TODO: when merging is implemented, we need to add an argument to the
// status indicating how much progress is left to merge. (device-mapper
// does not retain the initial values, so we can't derive them.)
return true;
}
@ -565,13 +534,6 @@ bool SnapshotManager::UnmapSnapshot(LockedFile* lock, const std::string& name) {
LOG(ERROR) << "Could not delete snapshot device: " << name;
return false;
}
auto snapshot_extra_device = GetSnapshotExtraDeviceName(name);
if (!dm.DeleteDeviceIfExists(snapshot_extra_device)) {
LOG(ERROR) << "Could not delete snapshot inner device: " << snapshot_extra_device;
return false;
}
return true;
}
@ -749,16 +711,15 @@ bool SnapshotManager::SwitchSnapshotToMerge(LockedFile* lock, const std::string&
// After this, we return true because we technically did switch to a merge
// target. Everything else we do here is just informational.
auto dm_name = GetSnapshotDeviceName(name, status);
if (!RewriteSnapshotDeviceTable(dm_name)) {
if (!RewriteSnapshotDeviceTable(name)) {
return false;
}
status.set_state(SnapshotState::MERGING);
DmTargetSnapshot::Status dm_status;
if (!QuerySnapshotStatus(dm_name, nullptr, &dm_status)) {
LOG(ERROR) << "Could not query merge status for snapshot: " << dm_name;
if (!QuerySnapshotStatus(name, nullptr, &dm_status)) {
LOG(ERROR) << "Could not query merge status for snapshot: " << name;
}
status.set_sectors_allocated(dm_status.sectors_allocated);
status.set_metadata_sectors(dm_status.metadata_sectors);
@ -768,33 +729,33 @@ bool SnapshotManager::SwitchSnapshotToMerge(LockedFile* lock, const std::string&
return true;
}
bool SnapshotManager::RewriteSnapshotDeviceTable(const std::string& dm_name) {
bool SnapshotManager::RewriteSnapshotDeviceTable(const std::string& name) {
auto& dm = DeviceMapper::Instance();
std::vector<DeviceMapper::TargetInfo> old_targets;
if (!dm.GetTableInfo(dm_name, &old_targets)) {
LOG(ERROR) << "Could not read snapshot device table: " << dm_name;
if (!dm.GetTableInfo(name, &old_targets)) {
LOG(ERROR) << "Could not read snapshot device table: " << name;
return false;
}
if (old_targets.size() != 1 || DeviceMapper::GetTargetType(old_targets[0].spec) != "snapshot") {
LOG(ERROR) << "Unexpected device-mapper table for snapshot: " << dm_name;
LOG(ERROR) << "Unexpected device-mapper table for snapshot: " << name;
return false;
}
std::string base_device, cow_device;
if (!DmTargetSnapshot::GetDevicesFromParams(old_targets[0].data, &base_device, &cow_device)) {
LOG(ERROR) << "Could not derive underlying devices for snapshot: " << dm_name;
LOG(ERROR) << "Could not derive underlying devices for snapshot: " << name;
return false;
}
DmTable table;
table.Emplace<DmTargetSnapshot>(0, old_targets[0].spec.length, base_device, cow_device,
SnapshotStorageMode::Merge, kSnapshotChunkSize);
if (!dm.LoadTableAndActivate(dm_name, table)) {
LOG(ERROR) << "Could not swap device-mapper tables on snapshot device " << dm_name;
if (!dm.LoadTableAndActivate(name, table)) {
LOG(ERROR) << "Could not swap device-mapper tables on snapshot device " << name;
return false;
}
LOG(INFO) << "Successfully switched snapshot device to a merge target: " << dm_name;
LOG(INFO) << "Successfully switched snapshot device to a merge target: " << name;
return true;
}
@ -1003,11 +964,9 @@ UpdateState SnapshotManager::CheckTargetMergeState(LockedFile* lock, const std::
return UpdateState::MergeFailed;
}
std::string dm_name = GetSnapshotDeviceName(name, snapshot_status);
std::unique_ptr<LpMetadata> current_metadata;
if (!IsSnapshotDevice(dm_name)) {
if (!IsSnapshotDevice(name)) {
if (!current_metadata) {
current_metadata = ReadCurrentMetadata();
}
@ -1029,7 +988,7 @@ UpdateState SnapshotManager::CheckTargetMergeState(LockedFile* lock, const std::
return UpdateState::MergeCompleted;
}
LOG(ERROR) << "Expected snapshot or snapshot-merge for device: " << dm_name;
LOG(ERROR) << "Expected snapshot or snapshot-merge for device: " << name;
return UpdateState::MergeFailed;
}
@ -1039,7 +998,7 @@ UpdateState SnapshotManager::CheckTargetMergeState(LockedFile* lock, const std::
std::string target_type;
DmTargetSnapshot::Status status;
if (!QuerySnapshotStatus(dm_name, &target_type, &status)) {
if (!QuerySnapshotStatus(name, &target_type, &status)) {
return UpdateState::MergeFailed;
}
if (target_type != "snapshot-merge") {
@ -1124,21 +1083,20 @@ void SnapshotManager::AcknowledgeMergeFailure() {
bool SnapshotManager::OnSnapshotMergeComplete(LockedFile* lock, const std::string& name,
const SnapshotStatus& status) {
auto dm_name = GetSnapshotDeviceName(name, status);
if (IsSnapshotDevice(dm_name)) {
if (IsSnapshotDevice(name)) {
// We are extra-cautious here, to avoid deleting the wrong table.
std::string target_type;
DmTargetSnapshot::Status dm_status;
if (!QuerySnapshotStatus(dm_name, &target_type, &dm_status)) {
if (!QuerySnapshotStatus(name, &target_type, &dm_status)) {
return false;
}
if (target_type != "snapshot-merge") {
LOG(ERROR) << "Unexpected target type " << target_type
<< " for snapshot device: " << dm_name;
<< " for snapshot device: " << name;
return false;
}
if (dm_status.sectors_allocated != dm_status.metadata_sectors) {
LOG(ERROR) << "Merge is unexpectedly incomplete for device " << dm_name;
LOG(ERROR) << "Merge is unexpectedly incomplete for device " << name;
return false;
}
if (!CollapseSnapshotDevice(name, status)) {
@ -1159,23 +1117,21 @@ bool SnapshotManager::OnSnapshotMergeComplete(LockedFile* lock, const std::strin
bool SnapshotManager::CollapseSnapshotDevice(const std::string& name,
const SnapshotStatus& status) {
auto& dm = DeviceMapper::Instance();
auto dm_name = GetSnapshotDeviceName(name, status);
// Verify we have a snapshot-merge device.
DeviceMapper::TargetInfo target;
if (!GetSingleTarget(dm_name, TableQuery::Table, &target)) {
if (!GetSingleTarget(name, TableQuery::Table, &target)) {
return false;
}
if (DeviceMapper::GetTargetType(target.spec) != "snapshot-merge") {
// This should be impossible, it was checked earlier.
LOG(ERROR) << "Snapshot device has invalid target type: " << dm_name;
LOG(ERROR) << "Snapshot device has invalid target type: " << name;
return false;
}
std::string base_device, cow_device;
if (!DmTargetSnapshot::GetDevicesFromParams(target.data, &base_device, &cow_device)) {
LOG(ERROR) << "Could not parse snapshot device " << dm_name
<< " parameters: " << target.data;
LOG(ERROR) << "Could not parse snapshot device " << name << " parameters: " << target.data;
return false;
}
@ -1186,42 +1142,6 @@ bool SnapshotManager::CollapseSnapshotDevice(const std::string& name,
return false;
}
if (dm_name != name) {
// We've derived the base device, but we actually need to replace the
// table of the outermost device. Do a quick verification that this
// device looks like we expect it to.
std::vector<DeviceMapper::TargetInfo> outer_table;
if (!dm.GetTableInfo(name, &outer_table)) {
LOG(ERROR) << "Could not validate outer snapshot table: " << name;
return false;
}
if (outer_table.size() != 2) {
LOG(ERROR) << "Expected 2 dm-linear targets for table " << name
<< ", got: " << outer_table.size();
return false;
}
for (const auto& target : outer_table) {
auto target_type = DeviceMapper::GetTargetType(target.spec);
if (target_type != "linear") {
LOG(ERROR) << "Outer snapshot table may only contain linear targets, but " << name
<< " has target: " << target_type;
return false;
}
}
if (outer_table[0].spec.length != snapshot_sectors) {
LOG(ERROR) << "dm-snapshot " << name << " should have " << snapshot_sectors
<< " sectors, got: " << outer_table[0].spec.length;
return false;
}
uint64_t expected_device_sectors = status.device_size() / kSectorSize;
uint64_t actual_device_sectors = outer_table[0].spec.length + outer_table[1].spec.length;
if (expected_device_sectors != actual_device_sectors) {
LOG(ERROR) << "Outer device " << name << " should have " << expected_device_sectors
<< " sectors, got: " << actual_device_sectors;
return false;
}
}
uint32_t slot = SlotNumberForSlotSuffix(device_->GetSlotSuffix());
// Create a DmTable that is identical to the base device.
CreateLogicalPartitionParams base_device_params{
@ -1236,7 +1156,6 @@ bool SnapshotManager::CollapseSnapshotDevice(const std::string& name,
return false;
}
// Note: we are replacing the *outer* table here, so we do not use dm_name.
if (!dm.LoadTableAndActivate(name, table)) {
return false;
}
@ -1246,18 +1165,9 @@ bool SnapshotManager::CollapseSnapshotDevice(const std::string& name,
// flushed remaining I/O. We could in theory replace with dm-zero (or
// re-use the table above), but for now it's better to know why this
// would fail.
if (dm_name != name && !dm.DeleteDeviceIfExists(dm_name)) {
LOG(ERROR) << "Unable to delete snapshot device " << dm_name << ", COW cannot be "
<< "reclaimed until after reboot.";
return false;
}
if (status.compression_enabled()) {
UnmapDmUserDevice(name);
}
// Cleanup the base device as well, since it is no longer used. This does
// not block cleanup.
auto base_name = GetBaseDeviceName(name);
if (!dm.DeleteDeviceIfExists(base_name)) {
LOG(ERROR) << "Unable to delete base device for snapshot: " << base_name;
@ -1548,7 +1458,7 @@ bool SnapshotManager::RemoveAllSnapshots(LockedFile* lock) {
// Otherwise (UPDATED flag), only delete snapshots if they are not mapped
// as dm-snapshot (for example, after merge completes).
bool should_unmap = current_slot != Slot::Target;
bool should_delete = ShouldDeleteSnapshot(lock, flashing_status, current_slot, name);
bool should_delete = ShouldDeleteSnapshot(flashing_status, current_slot, name);
bool partition_ok = true;
if (should_unmap && !UnmapPartitionWithSnapshot(lock, name)) {
@ -1582,8 +1492,7 @@ bool SnapshotManager::RemoveAllSnapshots(LockedFile* lock) {
}
// See comments in RemoveAllSnapshots().
bool SnapshotManager::ShouldDeleteSnapshot(LockedFile* lock,
const std::map<std::string, bool>& flashing_status,
bool SnapshotManager::ShouldDeleteSnapshot(const std::map<std::string, bool>& flashing_status,
Slot current_slot, const std::string& name) {
if (current_slot != Slot::Target) {
return true;
@ -1597,16 +1506,7 @@ bool SnapshotManager::ShouldDeleteSnapshot(LockedFile* lock,
// partition flashed, okay to delete obsolete snapshots
return true;
}
// partition updated, only delete if not dm-snapshot
SnapshotStatus status;
if (!ReadSnapshotStatus(lock, name, &status)) {
LOG(WARNING) << "Unable to read snapshot status for " << name
<< ", guessing snapshot device name";
auto extra_name = GetSnapshotExtraDeviceName(name);
return !IsSnapshotDevice(name) && !IsSnapshotDevice(extra_name);
}
auto dm_name = GetSnapshotDeviceName(name, status);
return !IsSnapshotDevice(dm_name);
return !IsSnapshotDevice(name);
}
UpdateState SnapshotManager::GetUpdateState(double* progress) {
@ -2409,14 +2309,6 @@ bool SnapshotManager::WriteSnapshotStatus(LockedFile* lock, const SnapshotStatus
return true;
}
std::string SnapshotManager::GetSnapshotDeviceName(const std::string& snapshot_name,
const SnapshotStatus& status) {
if (status.device_size() != status.snapshot_size()) {
return GetSnapshotExtraDeviceName(snapshot_name);
}
return snapshot_name;
}
bool SnapshotManager::EnsureImageManager() {
if (images_) return true;

31
fs_mgr/libsnapshot/snapshot_test.cpp
View file

@ -467,31 +467,6 @@ TEST_F(SnapshotTest, MapSnapshot) {
ASSERT_TRUE(android::base::StartsWith(snap_device, "/dev/block/dm-"));
}
TEST_F(SnapshotTest, MapPartialSnapshot) {
ASSERT_TRUE(AcquireLock());
static const uint64_t kSnapshotSize = 1024 * 1024;
static const uint64_t kDeviceSize = 1024 * 1024 * 2;
SnapshotStatus status;
status.set_name("test-snapshot");
status.set_device_size(kDeviceSize);
status.set_snapshot_size(kSnapshotSize);
status.set_cow_file_size(kSnapshotSize);
ASSERT_TRUE(sm->CreateSnapshot(lock_.get(), &status));
ASSERT_TRUE(CreateCowImage("test-snapshot"));
std::string base_device;
ASSERT_TRUE(CreatePartition("base-device", kDeviceSize, &base_device));
std::string cow_device;
ASSERT_TRUE(MapCowImage("test-snapshot", 10s, &cow_device));
std::string snap_device;
ASSERT_TRUE(sm->MapSnapshot(lock_.get(), "test-snapshot", base_device, cow_device, 10s,
&snap_device));
ASSERT_TRUE(android::base::StartsWith(snap_device, "/dev/block/dm-"));
}
TEST_F(SnapshotTest, NoMergeBeforeReboot) {
ASSERT_TRUE(sm->FinishedSnapshotWrites(false));
@ -590,8 +565,7 @@ TEST_F(SnapshotTest, FirstStageMountAndMerge) {
ASSERT_EQ(status.state(), SnapshotState::CREATED);
DeviceMapper::TargetInfo target;
auto dm_name = init->GetSnapshotDeviceName("test_partition_b", status);
ASSERT_TRUE(init->IsSnapshotDevice(dm_name, &target));
ASSERT_TRUE(init->IsSnapshotDevice("test_partition_b", &target));
ASSERT_EQ(DeviceMapper::GetTargetType(target.spec), "snapshot");
}
@ -618,8 +592,7 @@ TEST_F(SnapshotTest, FlashSuperDuringUpdate) {
// We should not get a snapshot device now.
DeviceMapper::TargetInfo target;
auto dm_name = init->GetSnapshotDeviceName("test_partition_b", status);
ASSERT_FALSE(init->IsSnapshotDevice(dm_name, &target));
ASSERT_FALSE(init->IsSnapshotDevice("test_partition_b", &target));
// We should see a cancelled update as well.
lock_ = nullptr;