Merge "liblog: simplify logd 'command' functions and struct logger"
diff --git a/fs_mgr/libsnapshot/Android.bp b/fs_mgr/libsnapshot/Android.bp
index 9256a16..1d72c70 100644
--- a/fs_mgr/libsnapshot/Android.bp
+++ b/fs_mgr/libsnapshot/Android.bp
@@ -72,6 +72,7 @@
     name: "libsnapshot_sources",
     srcs: [
         "android/snapshot/snapshot.proto",
+        "device_info.cpp",
         "snapshot.cpp",
         "snapshot_metadata_updater.cpp",
         "partition_cow_creator.cpp",
diff --git a/fs_mgr/libsnapshot/device_info.cpp b/fs_mgr/libsnapshot/device_info.cpp
new file mode 100644
index 0000000..bacb41c
--- /dev/null
+++ b/fs_mgr/libsnapshot/device_info.cpp
@@ -0,0 +1,123 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "device_info.h"
+
+#include <android-base/logging.h>
+#include <fs_mgr.h>
+#include <fs_mgr_overlayfs.h>
+
+namespace android {
+namespace snapshot {
+
+#ifdef LIBSNAPSHOT_USE_HAL
+using android::hardware::boot::V1_0::CommandResult;
+#endif
+
+using namespace std::string_literals;
+
+#ifdef __ANDROID_RECOVERY__
+constexpr bool kIsRecovery = true;
+#else
+constexpr bool kIsRecovery = false;
+#endif
+
+std::string DeviceInfo::GetGsidDir() const {
+    return "ota"s;
+}
+
+std::string DeviceInfo::GetMetadataDir() const {
+    return "/metadata/ota"s;
+}
+
+std::string DeviceInfo::GetSlotSuffix() const {
+    return fs_mgr_get_slot_suffix();
+}
+
+std::string DeviceInfo::GetOtherSlotSuffix() const {
+    return fs_mgr_get_other_slot_suffix();
+}
+
+const android::fs_mgr::IPartitionOpener& DeviceInfo::GetPartitionOpener() const {
+    return opener_;
+}
+
+std::string DeviceInfo::GetSuperDevice(uint32_t slot) const {
+    return fs_mgr_get_super_partition_name(slot);
+}
+
+bool DeviceInfo::IsOverlayfsSetup() const {
+    return fs_mgr_overlayfs_is_setup();
+}
+
+#ifdef LIBSNAPSHOT_USE_HAL
+bool DeviceInfo::EnsureBootHal() {
+    if (!boot_control_) {
+        auto hal = android::hardware::boot::V1_0::IBootControl::getService();
+        if (!hal) {
+            LOG(ERROR) << "Could not find IBootControl HAL";
+            return false;
+        }
+        boot_control_ = android::hardware::boot::V1_1::IBootControl::castFrom(hal);
+        if (!boot_control_) {
+            LOG(ERROR) << "Could not find IBootControl 1.1 HAL";
+            return false;
+        }
+    }
+    return true;
+}
+#endif
+
+bool DeviceInfo::SetBootControlMergeStatus([[maybe_unused]] MergeStatus status) {
+#ifdef LIBSNAPSHOT_USE_HAL
+    if (!EnsureBootHal()) {
+        return false;
+    }
+    if (!boot_control_->setSnapshotMergeStatus(status)) {
+        LOG(ERROR) << "Unable to set the snapshot merge status";
+        return false;
+    }
+    return true;
+#else
+    LOG(ERROR) << "HAL support not enabled.";
+    return false;
+#endif
+}
+
+bool DeviceInfo::IsRecovery() const {
+    return kIsRecovery;
+}
+
+bool DeviceInfo::SetSlotAsUnbootable([[maybe_unused]] unsigned int slot) {
+#ifdef LIBSNAPSHOT_USE_HAL
+    if (!EnsureBootHal()) {
+        return false;
+    }
+
+    CommandResult result = {};
+    auto cb = [&](CommandResult r) -> void { result = r; };
+    boot_control_->setSlotAsUnbootable(slot, cb);
+    if (!result.success) {
+        LOG(ERROR) << "Error setting slot " << slot << " unbootable: " << result.errMsg;
+        return false;
+    }
+    return true;
+#else
+    LOG(ERROR) << "HAL support not enabled.";
+    return false;
+#endif
+}
+
+}  // namespace snapshot
+}  // namespace android
diff --git a/fs_mgr/libsnapshot/device_info.h b/fs_mgr/libsnapshot/device_info.h
new file mode 100644
index 0000000..d8d3d91
--- /dev/null
+++ b/fs_mgr/libsnapshot/device_info.h
@@ -0,0 +1,53 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#pragma once
+
+#include <string>
+
+#ifdef LIBSNAPSHOT_USE_HAL
+#include <android/hardware/boot/1.1/IBootControl.h>
+#endif
+#include <liblp/partition_opener.h>
+#include <libsnapshot/snapshot.h>
+
+namespace android {
+namespace snapshot {
+
+class DeviceInfo final : public SnapshotManager::IDeviceInfo {
+    using MergeStatus = android::hardware::boot::V1_1::MergeStatus;
+
+  public:
+    std::string GetGsidDir() const override;
+    std::string GetMetadataDir() const override;
+    std::string GetSlotSuffix() const override;
+    std::string GetOtherSlotSuffix() const override;
+    const android::fs_mgr::IPartitionOpener& GetPartitionOpener() const override;
+    std::string GetSuperDevice(uint32_t slot) const override;
+    bool IsOverlayfsSetup() const override;
+    bool SetBootControlMergeStatus(MergeStatus status) override;
+    bool SetSlotAsUnbootable(unsigned int slot) override;
+    bool IsRecovery() const override;
+
+  private:
+    bool EnsureBootHal();
+
+    android::fs_mgr::PartitionOpener opener_;
+#ifdef LIBSNAPSHOT_USE_HAL
+    android::sp<android::hardware::boot::V1_1::IBootControl> boot_control_;
+#endif
+};
+
+}  // namespace snapshot
+}  // namespace android
diff --git a/fs_mgr/libsnapshot/include/libsnapshot/auto_device.h b/fs_mgr/libsnapshot/include/libsnapshot/auto_device.h
index d5ceb0e..73450db 100644
--- a/fs_mgr/libsnapshot/include/libsnapshot/auto_device.h
+++ b/fs_mgr/libsnapshot/include/libsnapshot/auto_device.h
@@ -27,6 +27,8 @@
     virtual ~AutoDevice(){};
     void Release();
 
+    bool HasDevice() const { return !name_.empty(); }
+
   protected:
     AutoDevice(const std::string& name) : name_(name) {}
     std::string name_;
diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
index c820395..8f24709 100644
--- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
+++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
@@ -122,6 +122,7 @@
         virtual const IPartitionOpener& GetPartitionOpener() const = 0;
         virtual bool IsOverlayfsSetup() const = 0;
         virtual bool SetBootControlMergeStatus(MergeStatus status) = 0;
+        virtual bool SetSlotAsUnbootable(unsigned int slot) = 0;
         virtual bool IsRecovery() const = 0;
     };
 
@@ -133,7 +134,7 @@
     static std::unique_ptr<SnapshotManager> New(IDeviceInfo* device = nullptr);
 
     // This is similar to New(), except designed specifically for first-stage
-    // init.
+    // init or recovery.
     static std::unique_ptr<SnapshotManager> NewForFirstStageMount(IDeviceInfo* device = nullptr);
 
     // Helper function for first-stage init to check whether a SnapshotManager
@@ -180,7 +181,10 @@
     //
     //   MergeCompleted indicates that the update has fully completed.
     //   GetUpdateState will return None, and a new update can begin.
-    UpdateState ProcessUpdateState();
+    //
+    // The optional callback allows the caller to periodically check the
+    // progress with GetUpdateState().
+    UpdateState ProcessUpdateState(const std::function<void()>& callback = {});
 
   public:
     // Initiate the merge if necessary, then wait for the merge to finish.
@@ -219,6 +223,18 @@
     // call to CreateLogicalPartitions when snapshots are present.
     bool CreateLogicalAndSnapshotPartitions(const std::string& super_device);
 
+    // This method should be called preceding any wipe or flash of metadata or
+    // userdata. It is only valid in recovery.
+    //
+    // When userdata will be wiped or flashed, it is necessary to clean up any
+    // snapshot state. If a merge is in progress, the merge must be finished.
+    // If a snapshot is present but not yet merged, the slot must be marked as
+    // unbootable.
+    //
+    // Returns true on success (or nothing to do), false on failure. The
+    // optional callback fires periodically to query progress via GetUpdateState.
+    bool HandleImminentDataWipe(const std::function<void()>& callback = {});
+
     // Dump debug information.
     bool Dump(std::ostream& os);
 
@@ -231,7 +247,7 @@
     // is not found in fstab.
     // Note: if this function is called the second time before the AutoDevice returned from the
     // first call is destroyed, the device will be unmounted when any of these AutoDevices is
-    // destroyed. FOr example:
+    // destroyed. For example:
     //   auto a = mgr->EnsureMetadataMounted(); // mounts
     //   auto b = mgr->EnsureMetadataMounted(); // does nothing
     //   b.reset() // unmounts
@@ -250,7 +266,10 @@
     FRIEND_TEST(SnapshotTest, Merge);
     FRIEND_TEST(SnapshotTest, NoMergeBeforeReboot);
     FRIEND_TEST(SnapshotTest, UpdateBootControlHal);
+    FRIEND_TEST(SnapshotUpdateTest, DataWipeAfterRollback);
+    FRIEND_TEST(SnapshotUpdateTest, DataWipeRollbackInRecovery);
     FRIEND_TEST(SnapshotUpdateTest, MergeCannotRemoveCow);
+    FRIEND_TEST(SnapshotUpdateTest, MergeInRecovery);
     FRIEND_TEST(SnapshotUpdateTest, SnapshotStatusFileWithoutCow);
     friend class SnapshotTest;
     friend class SnapshotUpdateTest;
@@ -464,6 +483,10 @@
             const LpMetadata* exported_target_metadata, const std::string& target_suffix,
             const std::map<std::string, SnapshotStatus>& all_snapshot_status);
 
+    // Unmap all partitions that were mapped by CreateLogicalAndSnapshotPartitions.
+    // This should only be called in recovery.
+    bool UnmapAllPartitions();
+
     std::string gsid_dir_;
     std::string metadata_dir_;
     std::unique_ptr<IDeviceInfo> device_;
diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp
index 2c516a2..1de7008 100644
--- a/fs_mgr/libsnapshot/snapshot.cpp
+++ b/fs_mgr/libsnapshot/snapshot.cpp
@@ -29,19 +29,16 @@
 #include <android-base/parseint.h>
 #include <android-base/strings.h>
 #include <android-base/unique_fd.h>
-#ifdef LIBSNAPSHOT_USE_HAL
-#include <android/hardware/boot/1.1/IBootControl.h>
-#endif
 #include <ext4_utils/ext4_utils.h>
 #include <fs_mgr.h>
 #include <fs_mgr_dm_linear.h>
-#include <fs_mgr_overlayfs.h>
 #include <fstab/fstab.h>
 #include <libdm/dm.h>
 #include <libfiemap/image_manager.h>
 #include <liblp/liblp.h>
 
 #include <android/snapshot/snapshot.pb.h>
+#include "device_info.h"
 #include "partition_cow_creator.h"
 #include "snapshot_metadata_updater.h"
 #include "utility.h"
@@ -77,57 +74,6 @@
 
 static constexpr char kBootIndicatorPath[] = "/metadata/ota/snapshot-boot";
 
-#ifdef __ANDROID_RECOVERY__
-constexpr bool kIsRecovery = true;
-#else
-constexpr bool kIsRecovery = false;
-#endif
-
-class DeviceInfo final : public SnapshotManager::IDeviceInfo {
-  public:
-    std::string GetGsidDir() const override { return "ota"s; }
-    std::string GetMetadataDir() const override { return "/metadata/ota"s; }
-    std::string GetSlotSuffix() const override { return fs_mgr_get_slot_suffix(); }
-    std::string GetOtherSlotSuffix() const override { return fs_mgr_get_other_slot_suffix(); }
-    const android::fs_mgr::IPartitionOpener& GetPartitionOpener() const { return opener_; }
-    std::string GetSuperDevice(uint32_t slot) const override {
-        return fs_mgr_get_super_partition_name(slot);
-    }
-    bool IsOverlayfsSetup() const override { return fs_mgr_overlayfs_is_setup(); }
-    bool SetBootControlMergeStatus(MergeStatus status) override;
-    bool IsRecovery() const override { return kIsRecovery; }
-
-  private:
-    android::fs_mgr::PartitionOpener opener_;
-#ifdef LIBSNAPSHOT_USE_HAL
-    android::sp<android::hardware::boot::V1_1::IBootControl> boot_control_;
-#endif
-};
-
-bool DeviceInfo::SetBootControlMergeStatus([[maybe_unused]] MergeStatus status) {
-#ifdef LIBSNAPSHOT_USE_HAL
-    if (!boot_control_) {
-        auto hal = android::hardware::boot::V1_0::IBootControl::getService();
-        if (!hal) {
-            LOG(ERROR) << "Could not find IBootControl HAL";
-            return false;
-        }
-        boot_control_ = android::hardware::boot::V1_1::IBootControl::castFrom(hal);
-        if (!boot_control_) {
-            LOG(ERROR) << "Could not find IBootControl 1.1 HAL";
-            return false;
-        }
-    }
-    if (!boot_control_->setSnapshotMergeStatus(status)) {
-        LOG(ERROR) << "Unable to set the snapshot merge status";
-        return false;
-    }
-    return true;
-#else
-    return false;
-#endif
-}
-
 // Note: IImageManager is an incomplete type in the header, so the default
 // destructor doesn't work.
 SnapshotManager::~SnapshotManager() {}
@@ -512,6 +458,14 @@
         return false;
     }
 
+    // We can't delete snapshots in recovery. The only way we'd try is it we're
+    // completing or canceling a merge in preparation for a data wipe, in which
+    // case, we don't care if the file sticks around.
+    if (device_->IsRecovery()) {
+        LOG(INFO) << "Skipping delete of snapshot " << name << " in recovery.";
+        return true;
+    }
+
     auto cow_image_name = GetCowImageDeviceName(name);
     if (images_->BackingImageExists(cow_image_name)) {
         if (!images_->DeleteBackingImage(cow_image_name)) {
@@ -744,7 +698,7 @@
 // Note that when a merge fails, we will *always* try again to complete the
 // merge each time the device boots. There is no harm in doing so, and if
 // the problem was transient, we might manage to get a new outcome.
-UpdateState SnapshotManager::ProcessUpdateState() {
+UpdateState SnapshotManager::ProcessUpdateState(const std::function<void()>& callback) {
     while (true) {
         UpdateState state = CheckMergeState();
         if (state == UpdateState::MergeFailed) {
@@ -756,6 +710,10 @@
             return state;
         }
 
+        if (callback) {
+            callback();
+        }
+
         // This wait is not super time sensitive, so we have a relatively
         // low polling frequency.
         std::this_thread::sleep_for(2s);
@@ -1718,9 +1676,9 @@
         case UpdateState::None:
         case UpdateState::MergeNeedsReboot:
         case UpdateState::MergeCompleted:
+        case UpdateState::Initiated:
             merge_status = MergeStatus::NONE;
             break;
-        case UpdateState::Initiated:
         case UpdateState::Unverified:
             merge_status = MergeStatus::SNAPSHOTTED;
             break;
@@ -2119,6 +2077,27 @@
     return UnmapPartitionWithSnapshot(lock.get(), target_partition_name);
 }
 
+bool SnapshotManager::UnmapAllPartitions() {
+    auto lock = LockExclusive();
+    if (!lock) return false;
+
+    const auto& opener = device_->GetPartitionOpener();
+    uint32_t slot = SlotNumberForSlotSuffix(device_->GetSlotSuffix());
+    auto super_device = device_->GetSuperDevice(slot);
+    auto metadata = android::fs_mgr::ReadMetadata(opener, super_device, slot);
+    if (!metadata) {
+        LOG(ERROR) << "Could not read dynamic partition metadata for device: " << super_device;
+        return false;
+    }
+
+    bool ok = true;
+    for (const auto& partition : metadata->partitions) {
+        auto partition_name = GetPartitionName(partition);
+        ok &= UnmapPartitionWithSnapshot(lock.get(), partition_name);
+    }
+    return ok;
+}
+
 bool SnapshotManager::Dump(std::ostream& os) {
     // Don't actually lock. Dump() is for debugging purposes only, so it is okay
     // if it is racy.
@@ -2192,5 +2171,66 @@
     return state;
 }
 
+bool SnapshotManager::HandleImminentDataWipe(const std::function<void()>& callback) {
+    if (!device_->IsRecovery()) {
+        LOG(ERROR) << "Data wipes are only allowed in recovery.";
+        return false;
+    }
+
+    auto mount = EnsureMetadataMounted();
+    if (!mount || !mount->HasDevice()) {
+        // We allow the wipe to continue, because if we can't mount /metadata,
+        // it is unlikely the device would have booted anyway. If there is no
+        // metadata partition, then the device predates Virtual A/B.
+        return true;
+    }
+
+    auto slot_number = SlotNumberForSlotSuffix(device_->GetSlotSuffix());
+    auto super_path = device_->GetSuperDevice(slot_number);
+    if (!CreateLogicalAndSnapshotPartitions(super_path)) {
+        LOG(ERROR) << "Unable to map partitions to complete merge.";
+        return false;
+    }
+
+    UpdateState state = ProcessUpdateState(callback);
+    LOG(INFO) << "Update state in recovery: " << state;
+    switch (state) {
+        case UpdateState::MergeFailed:
+            LOG(ERROR) << "Unrecoverable merge failure detected.";
+            return false;
+        case UpdateState::Unverified: {
+            // If an OTA was just applied but has not yet started merging, we
+            // have no choice but to revert slots, because the current slot will
+            // immediately become unbootable. Rather than wait for the device
+            // to reboot N times until a rollback, we proactively disable the
+            // new slot instead.
+            //
+            // Since the rollback is inevitable, we don't treat a HAL failure
+            // as an error here.
+            std::string old_slot;
+            auto boot_file = GetSnapshotBootIndicatorPath();
+            if (android::base::ReadFileToString(boot_file, &old_slot) &&
+                device_->GetSlotSuffix() != old_slot) {
+                LOG(ERROR) << "Reverting to slot " << old_slot << " since update will be deleted.";
+                device_->SetSlotAsUnbootable(slot_number);
+            }
+            break;
+        }
+        case UpdateState::MergeNeedsReboot:
+            // We shouldn't get here, because nothing is depending on
+            // logical partitions.
+            LOG(ERROR) << "Unexpected merge-needs-reboot state in recovery.";
+            break;
+        default:
+            break;
+    }
+
+    // Nothing should be depending on partitions now, so unmap them all.
+    if (!UnmapAllPartitions()) {
+        LOG(ERROR) << "Unable to unmap all partitions; fastboot may fail to flash.";
+    }
+    return true;
+}
+
 }  // namespace snapshot
 }  // namespace android
diff --git a/fs_mgr/libsnapshot/snapshot_test.cpp b/fs_mgr/libsnapshot/snapshot_test.cpp
index af268f9..8783526 100644
--- a/fs_mgr/libsnapshot/snapshot_test.cpp
+++ b/fs_mgr/libsnapshot/snapshot_test.cpp
@@ -1230,6 +1230,80 @@
     EXPECT_FALSE(IsMetadataMounted());
 }
 
+// Test that during a merge, we can wipe data in recovery.
+TEST_F(SnapshotUpdateTest, MergeInRecovery) {
+    // Execute the first update.
+    ASSERT_TRUE(sm->BeginUpdate());
+    ASSERT_TRUE(sm->CreateUpdateSnapshots(manifest_));
+    ASSERT_TRUE(sm->FinishedSnapshotWrites());
+
+    // Simulate shutting down the device.
+    ASSERT_TRUE(UnmapAll());
+
+    // After reboot, init does first stage mount.
+    auto init = SnapshotManager::NewForFirstStageMount(new TestDeviceInfo(fake_super, "_b"));
+    ASSERT_NE(init, nullptr);
+    ASSERT_TRUE(init->NeedSnapshotsInFirstStageMount());
+    ASSERT_TRUE(init->CreateLogicalAndSnapshotPartitions("super"));
+    init = nullptr;
+
+    // Initiate the merge and then immediately stop it to simulate a reboot.
+    auto new_sm = SnapshotManager::New(new TestDeviceInfo(fake_super, "_b"));
+    ASSERT_TRUE(new_sm->InitiateMerge());
+    ASSERT_TRUE(UnmapAll());
+
+    // Simulate a reboot into recovery.
+    auto test_device = std::make_unique<TestDeviceInfo>(fake_super, "_b");
+    test_device->set_recovery(true);
+    new_sm = SnapshotManager::NewForFirstStageMount(test_device.release());
+
+    ASSERT_TRUE(new_sm->HandleImminentDataWipe());
+    ASSERT_EQ(new_sm->GetUpdateState(), UpdateState::None);
+}
+
+// Test that after an OTA, before a merge, we can wipe data in recovery.
+TEST_F(SnapshotUpdateTest, DataWipeRollbackInRecovery) {
+    // Execute the first update.
+    ASSERT_TRUE(sm->BeginUpdate());
+    ASSERT_TRUE(sm->CreateUpdateSnapshots(manifest_));
+    ASSERT_TRUE(sm->FinishedSnapshotWrites());
+
+    // Simulate shutting down the device.
+    ASSERT_TRUE(UnmapAll());
+
+    // Simulate a reboot into recovery.
+    auto test_device = new TestDeviceInfo(fake_super, "_b");
+    test_device->set_recovery(true);
+    auto new_sm = SnapshotManager::NewForFirstStageMount(test_device);
+
+    ASSERT_TRUE(new_sm->HandleImminentDataWipe());
+    EXPECT_EQ(new_sm->GetUpdateState(), UpdateState::Unverified);
+    EXPECT_TRUE(test_device->IsSlotUnbootable(1));
+    EXPECT_FALSE(test_device->IsSlotUnbootable(0));
+}
+
+// Test that after an OTA and a bootloader rollback with no merge, we can wipe
+// data in recovery.
+TEST_F(SnapshotUpdateTest, DataWipeAfterRollback) {
+    // Execute the first update.
+    ASSERT_TRUE(sm->BeginUpdate());
+    ASSERT_TRUE(sm->CreateUpdateSnapshots(manifest_));
+    ASSERT_TRUE(sm->FinishedSnapshotWrites());
+
+    // Simulate shutting down the device.
+    ASSERT_TRUE(UnmapAll());
+
+    // Simulate a rollback, with reboot into recovery.
+    auto test_device = new TestDeviceInfo(fake_super, "_a");
+    test_device->set_recovery(true);
+    auto new_sm = SnapshotManager::NewForFirstStageMount(test_device);
+
+    ASSERT_TRUE(new_sm->HandleImminentDataWipe());
+    EXPECT_EQ(new_sm->GetUpdateState(), UpdateState::None);
+    EXPECT_FALSE(test_device->IsSlotUnbootable(0));
+    EXPECT_FALSE(test_device->IsSlotUnbootable(0));
+}
+
 class FlashAfterUpdateTest : public SnapshotUpdateTest,
                              public WithParamInterface<std::tuple<uint32_t, bool>> {
   public:
diff --git a/fs_mgr/libsnapshot/test_helpers.h b/fs_mgr/libsnapshot/test_helpers.h
index 0f70afe..9083843 100644
--- a/fs_mgr/libsnapshot/test_helpers.h
+++ b/fs_mgr/libsnapshot/test_helpers.h
@@ -16,6 +16,7 @@
 
 #include <optional>
 #include <string>
+#include <unordered_set>
 
 #include <android/hardware/boot/1.1/IBootControl.h>
 #include <gmock/gmock.h>
@@ -89,6 +90,12 @@
     }
     bool IsOverlayfsSetup() const override { return false; }
     bool IsRecovery() const override { return recovery_; }
+    bool SetSlotAsUnbootable(unsigned int slot) override {
+        unbootable_slots_.insert(slot);
+        return true;
+    }
+
+    bool IsSlotUnbootable(uint32_t slot) { return unbootable_slots_.count(slot) != 0; }
 
     void set_slot_suffix(const std::string& suffix) { slot_suffix_ = suffix; }
     void set_fake_super(const std::string& path) {
@@ -102,6 +109,7 @@
     std::unique_ptr<TestPartitionOpener> opener_;
     MergeStatus merge_status_;
     bool recovery_ = false;
+    std::unordered_set<uint32_t> unbootable_slots_;
 };
 
 class SnapshotTestPropertyFetcher : public android::fs_mgr::testing::MockPropertyFetcher {
diff --git a/init/action.cpp b/init/action.cpp
index 69e40d0..f05fa7c 100644
--- a/init/action.cpp
+++ b/init/action.cpp
@@ -180,21 +180,24 @@
 // It takes an optional (name, value) pair, which if provided must
 // be present in property_triggers_; it skips the check of the current
 // property value for this pair.
-bool Action::CheckPropertyTriggers(const std::string& name,
-                                   const std::string& value) const {
+bool Action::CheckPropertyTriggers(const std::string& name, const std::string& value) const {
     if (property_triggers_.empty()) {
         return true;
     }
 
-    bool found = name.empty();
+    if (!name.empty()) {
+        auto it = property_triggers_.find(name);
+        if (it == property_triggers_.end()) {
+            return false;
+        }
+        const auto& trigger_value = it->second;
+        if (trigger_value != "*" && trigger_value != value) {
+            return false;
+        }
+    }
+
     for (const auto& [trigger_name, trigger_value] : property_triggers_) {
-        if (trigger_name == name) {
-            if (trigger_value != "*" && trigger_value != value) {
-                return false;
-            } else {
-                found = true;
-            }
-        } else {
+        if (trigger_name != name) {
             std::string prop_value = android::base::GetProperty(trigger_name, "");
             if (trigger_value == "*" && !prop_value.empty()) {
                 continue;
@@ -202,7 +205,7 @@
             if (trigger_value != prop_value) return false;
         }
     }
-    return found;
+    return true;
 }
 
 bool Action::CheckEvent(const EventTrigger& event_trigger) const {
diff --git a/init/builtins.cpp b/init/builtins.cpp
index a55514b..8f58145 100644
--- a/init/builtins.cpp
+++ b/init/builtins.cpp
@@ -65,6 +65,7 @@
 
 #include "action_manager.h"
 #include "bootchart.h"
+#include "builtin_arguments.h"
 #include "fscrypt_init_extensions.h"
 #include "init.h"
 #include "mount_namespace.h"
@@ -1216,6 +1217,15 @@
     }
 }
 
+static Result<void> do_finish_userspace_reboot(const BuiltinArguments&) {
+    LOG(INFO) << "Userspace reboot successfully finished";
+    boot_clock::time_point now = boot_clock::now();
+    property_set("sys.init.userspace_reboot.last_finished",
+                 std::to_string(now.time_since_epoch().count()));
+    property_set(kUserspaceRebootInProgress, "0");
+    return {};
+}
+
 // Builtin-function-map start
 const BuiltinFunctionMap& GetBuiltinFunctionMap() {
     constexpr std::size_t kMax = std::numeric_limits<std::size_t>::max();
@@ -1237,6 +1247,7 @@
         {"exec_background",         {1,     kMax, {false,  do_exec_background}}},
         {"exec_start",              {1,     1,    {false,  do_exec_start}}},
         {"export",                  {2,     2,    {false,  do_export}}},
+        {"finish_userspace_reboot", {0,     0,    {false,  do_finish_userspace_reboot}}},
         {"hostname",                {1,     1,    {true,   do_hostname}}},
         {"ifup",                    {1,     1,    {true,   do_ifup}}},
         {"init_user0",              {0,     0,    {false,  do_init_user0}}},
diff --git a/init/reboot.cpp b/init/reboot.cpp
index 64ec1fb..7167672 100644
--- a/init/reboot.cpp
+++ b/init/reboot.cpp
@@ -69,10 +69,13 @@
 
 using namespace std::literals;
 
+using android::base::boot_clock;
 using android::base::GetBoolProperty;
+using android::base::SetProperty;
 using android::base::Split;
 using android::base::Timer;
 using android::base::unique_fd;
+using android::base::WaitForProperty;
 using android::base::WriteStringToFile;
 
 namespace android {
@@ -728,16 +731,21 @@
 
 static Result<void> DoUserspaceReboot() {
     LOG(INFO) << "Userspace reboot initiated";
+    boot_clock::time_point now = boot_clock::now();
+    property_set("sys.init.userspace_reboot.last_started",
+                 std::to_string(now.time_since_epoch().count()));
     auto guard = android::base::make_scope_guard([] {
         // Leave shutdown so that we can handle a full reboot.
         LeaveShutdown();
         trigger_shutdown("reboot,abort-userspace-reboot");
     });
-    // Triggering userspace-reboot-requested will result in a bunch of set_prop
+    // Triggering userspace-reboot-requested will result in a bunch of setprop
     // actions. We should make sure, that all of them are propagated before
-    // proceeding with userspace reboot.
-    // TODO(b/135984674): implement proper synchronization logic.
-    std::this_thread::sleep_for(500ms);
+    // proceeding with userspace reboot. Synchronously setting kUserspaceRebootInProgress property
+    // is not perfect, but it should do the trick.
+    if (property_set(kUserspaceRebootInProgress, "1") != 0) {
+        return Error() << "Failed to set property " << kUserspaceRebootInProgress;
+    }
     EnterShutdown();
     std::vector<Service*> stop_first;
     // Remember the services that were enabled. We will need to manually enable them again otherwise
@@ -752,6 +760,12 @@
             were_enabled.push_back(s);
         }
     }
+    {
+        Timer sync_timer;
+        LOG(INFO) << "sync() before terminating services...";
+        sync();
+        LOG(INFO) << "sync() took " << sync_timer;
+    }
     // TODO(b/135984674): do we need shutdown animation for userspace reboot?
     // TODO(b/135984674): control userspace timeout via read-only property?
     StopServicesAndLogViolations(stop_first, 10s, true /* SIGTERM */);
@@ -767,6 +781,12 @@
         // TODO(b/135984674): store information about offending services for debugging.
         return Error() << r << " debugging services are still running";
     }
+    {
+        Timer sync_timer;
+        LOG(INFO) << "sync() after stopping services...";
+        sync();
+        LOG(INFO) << "sync() took " << sync_timer;
+    }
     if (auto result = UnmountAllApexes(); !result) {
         return result;
     }
@@ -784,7 +804,38 @@
     return {};
 }
 
+static void UserspaceRebootWatchdogThread() {
+    if (!WaitForProperty("sys.init.userspace_reboot_in_progress", "1", 20s)) {
+        // TODO(b/135984674): should we reboot instead?
+        LOG(WARNING) << "Userspace reboot didn't start in 20 seconds. Stopping watchdog";
+        return;
+    }
+    LOG(INFO) << "Starting userspace reboot watchdog";
+    // TODO(b/135984674): this should be configured via a read-only sysprop.
+    std::chrono::milliseconds timeout = 60s;
+    if (!WaitForProperty("sys.boot_completed", "1", timeout)) {
+        LOG(ERROR) << "Failed to boot in " << timeout.count() << "ms. Switching to full reboot";
+        // In this case device is in a boot loop. Only way to recover is to do dirty reboot.
+        RebootSystem(ANDROID_RB_RESTART2, "userspace-reboot-watchdog-triggered");
+    }
+    LOG(INFO) << "Device booted, stopping userspace reboot watchdog";
+}
+
 static void HandleUserspaceReboot() {
+    // Spinnig up a separate thread will fail the setns call later in the boot sequence.
+    // Fork a new process to monitor userspace reboot while we are investigating a better solution.
+    pid_t pid = fork();
+    if (pid < 0) {
+        PLOG(ERROR) << "Failed to fork process for userspace reboot watchdog. Switching to full "
+                    << "reboot";
+        trigger_shutdown("reboot,userspace-reboot-failed-to-fork");
+        return;
+    }
+    if (pid == 0) {
+        // Child
+        UserspaceRebootWatchdogThread();
+        _exit(EXIT_SUCCESS);
+    }
     LOG(INFO) << "Clearing queue and starting userspace-reboot-requested trigger";
     auto& am = ActionManager::GetInstance();
     am.ClearQueue();
diff --git a/init/reboot.h b/init/reboot.h
index 81c3edc..cdfa024 100644
--- a/init/reboot.h
+++ b/init/reboot.h
@@ -22,6 +22,8 @@
 namespace android {
 namespace init {
 
+static const constexpr char* kUserspaceRebootInProgress = "sys.init.userspace_reboot.in_progress";
+
 // Parses and handles a setprop sys.powerctl message.
 void HandlePowerctlMessage(const std::string& command);
 
diff --git a/liblog/logger_read.cpp b/liblog/logger_read.cpp
index 79b40a9..0ce7a46 100644
--- a/liblog/logger_read.cpp
+++ b/liblog/logger_read.cpp
@@ -109,7 +109,7 @@
 
   int ret = (*transp->transport->read)(logger_list, transp, log_msg);
 
-  if (ret < 0) {
+  if (ret <= 0) {
     return ret;
   }
 
diff --git a/libziparchive/unzip.cpp b/libziparchive/unzip.cpp
index 56f594a..11b575e 100644
--- a/libziparchive/unzip.cpp
+++ b/libziparchive/unzip.cpp
@@ -448,6 +448,7 @@
 
   static const struct option opts[] = {
       {"help", no_argument, 0, 'h'},
+      {},
   };
 
   if (role == kUnzip) {
diff --git a/rootdir/init.rc b/rootdir/init.rc
index 674b737..6b61472 100644
--- a/rootdir/init.rc
+++ b/rootdir/init.rc
@@ -935,7 +935,6 @@
 
 on userspace-reboot-requested
   # TODO(b/135984674): reset all necessary properties here.
-  setprop sys.init.userspace_reboot_in_progress 1
   setprop sys.boot_completed 0
   setprop sys.init.updatable_crashing 0
   setprop apexd.status ""
@@ -955,3 +954,6 @@
   trigger zygote-start
   trigger early-boot
   trigger boot
+
+on property:sys.boot_completed=1 && property:sys.init.userspace_reboot.in_progress=1
+  finish_userspace_reboot