tequilaOS/platform_system_core
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Merge "libsnapshot: Don't compare strings for snapshot state."

Browse source 
am: f79af03011

Change-Id: I15c66ebd7af0d17cae47b4836a1620374d279957
This commit is contained in:
David Anderson 2019-08-16 08:20:09 -07:00 committed by android-build-merger
commit 8cbf0b9397
3 changed files with 38 additions and 12 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -219,9 +219,12 @@ class SnapshotManager final {
bool WriteUpdateState(LockedFile* file, UpdateState state);
std::string GetStateFilePath() const;
enum class SnapshotState : int { Created, Merging, MergeCompleted };
static std::string to_string(SnapshotState state);
// This state is persisted per-snapshot in /metadata/ota/snapshots/.
struct SnapshotStatus {
std::string state;
SnapshotState state;
uint64_t device_size;
uint64_t snapshot_size;
// These are non-zero when merging.

43
fs_mgr/libsnapshot/snapshot.cpp
View file

@ -165,7 +165,7 @@ bool SnapshotManager::CreateSnapshot(LockedFile* lock, const std::string& name,
// actual backing image. This is harmless, since it'll get removed when
// CancelUpdate is called.
SnapshotStatus status = {
.state = "created",
.state = SnapshotState::Created,
.device_size = device_size,
.snapshot_size = snapshot_size,
};
@ -190,7 +190,7 @@ bool SnapshotManager::MapSnapshot(LockedFile* lock, const std::string& name,
if (!ReadSnapshotStatus(lock, name, &status)) {
return false;
}
if (status.state == "merge-completed") {
if (status.state == SnapshotState::MergeCompleted) {
LOG(ERROR) << "Should not create a snapshot device for " << name
<< " after merging has completed.";
return false;
@ -423,8 +423,8 @@ bool SnapshotManager::SwitchSnapshotToMerge(LockedFile* lock, const std::string&
if (!ReadSnapshotStatus(lock, name, &status)) {
return false;
}
if (status.state != "created") {
LOG(WARNING) << "Snapshot " << name << " has unexpected state: " << status.state;
if (status.state != SnapshotState::Created) {
LOG(WARNING) << "Snapshot " << name << " has unexpected state: " << to_string(status.state);
}
// After this, we return true because we technically did switch to a merge
@ -434,7 +434,7 @@ bool SnapshotManager::SwitchSnapshotToMerge(LockedFile* lock, const std::string&
return false;
}
status.state = "merging";
status.state = SnapshotState::Merging;
DmTargetSnapshot::Status dm_status;
if (!QuerySnapshotStatus(dm_name, nullptr, &dm_status)) {
@ -658,7 +658,7 @@ UpdateState SnapshotManager::CheckTargetMergeState(LockedFile* lock, const std::
// rebooted after this check, the device will still be a snapshot-merge
// target. If the have rebooted, the device will now be a linear target,
// and we can try cleanup again.
if (snapshot_status.state == "merge-complete" && !IsSnapshotDevice(dm_name)) {
if (snapshot_status.state == SnapshotState::MergeCompleted && !IsSnapshotDevice(dm_name)) {
// NB: It's okay if this fails now, we gave cleanup our best effort.
OnSnapshotMergeComplete(lock, name, snapshot_status);
return UpdateState::MergeCompleted;
@ -679,7 +679,7 @@ UpdateState SnapshotManager::CheckTargetMergeState(LockedFile* lock, const std::
// These two values are equal when merging is complete.
if (status.sectors_allocated != status.metadata_sectors) {
if (snapshot_status.state == "merge-complete") {
if (snapshot_status.state == SnapshotState::MergeCompleted) {
LOG(ERROR) << "Snapshot " << name << " is merging after being marked merge-complete.";
return UpdateState::MergeFailed;
}
@ -694,7 +694,7 @@ UpdateState SnapshotManager::CheckTargetMergeState(LockedFile* lock, const std::
// This makes it simpler to reason about the next reboot: no matter what
// part of cleanup failed, first-stage init won't try to create another
// snapshot device for this partition.
snapshot_status.state = "merge-complete";
snapshot_status.state = SnapshotState::MergeCompleted;
if (!WriteSnapshotStatus(lock, name, snapshot_status)) {
return UpdateState::MergeFailed;
}
@ -1069,7 +1069,16 @@ bool SnapshotManager::ReadSnapshotStatus(LockedFile* lock, const std::string& na
return false;
}
status->state = pieces[0];
if (pieces[0] == "created") {
status->state = SnapshotState::Created;
} else if (pieces[0] == "merging") {
status->state = SnapshotState::Merging;
} else if (pieces[0] == "merge-completed") {
status->state = SnapshotState::MergeCompleted;
} else {
LOG(ERROR) << "Unrecognized state " << pieces[0] << " for snapshot: " << name;
}
if (!android::base::ParseUint(pieces[1], &status->device_size)) {
LOG(ERROR) << "Invalid device size in status line for: " << path;
return false;
@ -1089,6 +1098,20 @@ bool SnapshotManager::ReadSnapshotStatus(LockedFile* lock, const std::string& na
return true;
}
std::string SnapshotManager::to_string(SnapshotState state) {
switch (state) {
case SnapshotState::Created:
return "created";
case SnapshotState::Merging:
return "merging";
case SnapshotState::MergeCompleted:
return "merge-completed";
default:
LOG(ERROR) << "Unknown snapshot state: " << (int)state;
return "unknown";
}
}
bool SnapshotManager::WriteSnapshotStatus(LockedFile* lock, const std::string& name,
const SnapshotStatus& status) {
// The caller must take an exclusive lock to modify snapshots.
@ -1103,7 +1126,7 @@ bool SnapshotManager::WriteSnapshotStatus(LockedFile* lock, const std::string& n
}
std::vector<std::string> pieces = {
status.state,
to_string(status.state),
std::to_string(status.device_size),
std::to_string(status.snapshot_size),
std::to_string(status.sectors_allocated),

2
fs_mgr/libsnapshot/snapshot_test.cpp
View file

@ -148,7 +148,7 @@ TEST_F(SnapshotTest, CreateSnapshot) {
{
SnapshotManager::SnapshotStatus status;
ASSERT_TRUE(sm->ReadSnapshotStatus(lock_.get(), "test-snapshot", &status));
ASSERT_EQ(status.state, "created");
ASSERT_EQ(status.state, SnapshotManager::SnapshotState::Created);
ASSERT_EQ(status.device_size, kDeviceSize);
ASSERT_EQ(status.snapshot_size, kDeviceSize);
}