commit f86fca236fe5ae29cdba649a4efa9f13de1e55df
author Akilesh Kailash <akailash@google.com> Tue Apr 12 21:59:05 2022 +0000
committer Akilesh Kailash <akailash@google.com> Sat Apr 16 16:43:35 2022 +0000
tree 6f8411aa22bd1c7debdfd23758a28a75aaa199a5
parent 4691a163c114b28f87db961827915f14a8810779

libsnapshot: Remove invalid snapshot metadata

If there are snapshot metadata persisting in /metadata/ota/snapshots,
remove them before applying a new update. Make sure that
the snapshots are indeed invalid before removing them.

On a sidenote, add a comment in init.cpp related to
b/223076262.

Bug: 228250473
Test: 1: Apply OTA in recovery through adb sideload
2: Reboot
3: Apply OTA OTA again through update_device.py
4: Re-run Full OTA updates just from update_device.py

Signed-off-by: Akilesh Kailash <akailash@google.com>
Change-Id: I116bbafae09042b9c391ccd58c102704571c214e

fs_mgr/libsnapshot/include/libsnapshot/snapshot.h

diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
index 38b47d5..11da568 100644
--- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
+++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
@@ -538,6 +538,9 @@
     // Unmap a COW and remove it from a MetadataBuilder.
     void UnmapAndDeleteCowPartition(MetadataBuilder* current_metadata);
 
+    // Remove invalid snapshots if any
+    void RemoveInvalidSnapshots(LockedFile* lock);
+
     // Unmap and remove all known snapshots.
     bool RemoveAllSnapshots(LockedFile* lock);
 

fs_mgr/libsnapshot/snapshot.cpp

diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp
index d086f29..a83f535 100644
--- a/fs_mgr/libsnapshot/snapshot.cpp
+++ b/fs_mgr/libsnapshot/snapshot.cpp
@@ -218,7 +218,10 @@
     if (!file) return false;
 
     UpdateState state = ReadUpdateState(file.get());
-    if (state == UpdateState::None) return true;
+    if (state == UpdateState::None) {
+        RemoveInvalidSnapshots(file.get());
+        return true;
+    }
 
     if (state == UpdateState::Initiated) {
         LOG(INFO) << "Update has been initiated, now canceling";
@@ -1903,6 +1906,33 @@
     return true;
 }
 
+void SnapshotManager::RemoveInvalidSnapshots(LockedFile* lock) {
+    std::vector<std::string> snapshots;
+
+    // Remove the stale snapshot metadata
+    //
+    // We make sure that all the three cases
+    // are valid before removing the snapshot metadata:
+    //
+    // 1: dm state is active
+    // 2: Root fs is not mounted off as a snapshot device
+    // 3: Snapshot slot suffix should match current device slot
+    if (!ListSnapshots(lock, &snapshots, device_->GetSlotSuffix()) || snapshots.empty()) {
+        return;
+    }
+
+    // We indeed have some invalid snapshots
+    for (const auto& name : snapshots) {
+        if (dm_.GetState(name) == DmDeviceState::ACTIVE && !IsSnapshotDevice(name)) {
+            if (!DeleteSnapshot(lock, name)) {
+                LOG(ERROR) << "Failed to delete invalid snapshot: " << name;
+            } else {
+                LOG(INFO) << "Invalid snapshot: " << name << " deleted";
+            }
+        }
+    }
+}
+
 bool SnapshotManager::RemoveAllSnapshots(LockedFile* lock) {
     std::vector<std::string> snapshots;
     if (!ListSnapshots(lock, &snapshots)) {

init/init.cpp

diff --git a/init/init.cpp b/init/init.cpp
index d5bdd12..038f172 100644
--- a/init/init.cpp
+++ b/init/init.cpp
@@ -795,6 +795,10 @@
         InstallRebootSignalHandlers();
     }
 
+    // No threads should be spin up until signalfd
+    // is registered. If the threads are indeed required,
+    // each of these threads _should_ make sure SIGCHLD signal
+    // is blocked. See b/223076262
     boot_clock::time_point start_time = boot_clock::now();
 
     trigger_shutdown = [](const std::string& command) { shutdown_state.TriggerShutdown(command); };