Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 5a0177d9 authored by David Anderson's avatar David Anderson
Browse files

fastboot: Fix snapshot-update merge behavior. 

When merging in recovery, the "imminent data wipe" code was used, which
made the assumption the /metadata and /data state would be zapped. This
caused future OTAs to error because the old snapshots were detected.

This CL allows OTAs to proceed even if unexpected snapshots are present.
It also forces the state to "MergeCompleted" after a merge in recovery,
so that the next normal boot can perform cleanup.

Bug: 155339165
Test: fastboot snapshot-update merge, then take another OTA
      vts_libsnapshot_test
Change-Id: Ief6dea3ba76323044e61307272dda320a4494aea
parent f4cd49af

fastboot/device/commands.cpp

+1 −1
Original line number Diff line number Diff line
@@ -625,7 +625,7 @@ bool SnapshotUpdateHandler(FastbootDevice* device, const std::vector<std::string 
        if (!sm) {

            return device->WriteFail("Unable to create SnapshotManager");

        }

        if (!sm->HandleImminentDataWipe()) {

        if (!sm->FinishMergeInRecovery()) {

            return device->WriteFail("Unable to finish snapshot merge");

        }

    } else {

fs_mgr/libsnapshot/include/libsnapshot/mock_snapshot.h

+1 −0
Original line number Diff line number Diff line
@@ -43,6 +43,7 @@ class MockSnapshotManager : public ISnapshotManager { 
                (const std::string& super_device, const std::chrono::milliseconds& timeout_ms),

                (override));

    MOCK_METHOD(bool, HandleImminentDataWipe, (const std::function<void()>& callback), (override));

    MOCK_METHOD(bool, FinishMergeInRecovery, (), (override));

    MOCK_METHOD(CreateResult, RecoveryCreateSnapshotDevices, (), (override));

    MOCK_METHOD(CreateResult, RecoveryCreateSnapshotDevices,

                (const std::unique_ptr<AutoDevice>& metadata_device), (override));

fs_mgr/libsnapshot/include/libsnapshot/snapshot.h

+6 −0
Original line number Diff line number Diff line
@@ -202,6 +202,10 @@ class ISnapshotManager { 
    // optional callback fires periodically to query progress via GetUpdateState.

    virtual bool HandleImminentDataWipe(const std::function<void()>& callback = {}) = 0;



    // Force a merge to complete in recovery. This is similar to HandleImminentDataWipe

    // but does not expect a data wipe after.

    virtual bool FinishMergeInRecovery() = 0;



    // This method is only allowed in recovery and is used as a helper to

    // initialize the snapshot devices as a requirement to mount a snapshotted

    // /system in recovery.
@@ -290,6 +294,7 @@ class SnapshotManager final : public ISnapshotManager { 
            const std::string& super_device,

            const std::chrono::milliseconds& timeout_ms = {}) override;

    bool HandleImminentDataWipe(const std::function<void()>& callback = {}) override;

    bool FinishMergeInRecovery() override;

    CreateResult RecoveryCreateSnapshotDevices() override;

    CreateResult RecoveryCreateSnapshotDevices(

            const std::unique_ptr<AutoDevice>& metadata_device) override;
@@ -580,6 +585,7 @@ class SnapshotManager final : public ISnapshotManager { 
    std::unique_ptr<IDeviceInfo> device_;

    std::unique_ptr<IImageManager> images_;

    bool has_local_image_manager_ = false;

    bool in_factory_data_reset_ = false;

};



}  // namespace snapshot

fs_mgr/libsnapshot/include/libsnapshot/snapshot_stub.h

+1 −0
Original line number Diff line number Diff line
@@ -41,6 +41,7 @@ class SnapshotManagerStub : public ISnapshotManager { 
            const std::string& super_device,

            const std::chrono::milliseconds& timeout_ms = {}) override;

    bool HandleImminentDataWipe(const std::function<void()>& callback = {}) override;

    bool FinishMergeInRecovery() override;

    CreateResult RecoveryCreateSnapshotDevices() override;

    CreateResult RecoveryCreateSnapshotDevices(

            const std::unique_ptr<AutoDevice>& metadata_device) override;

fs_mgr/libsnapshot/snapshot.cpp

+54 −1
Original line number Diff line number Diff line
@@ -577,8 +577,16 @@ bool SnapshotManager::InitiateMerge() { 
        return false;

    }



    auto other_suffix = device_->GetOtherSlotSuffix();



    auto& dm = DeviceMapper::Instance();

    for (const auto& snapshot : snapshots) {

        if (android::base::EndsWith(snapshot, other_suffix)) {

            // Allow the merge to continue, but log this unexpected case.

            LOG(ERROR) << "Unexpected snapshot found during merge: " << snapshot;

            continue;

        }



        // The device has to be mapped, since everything should be merged at

        // the same time. This is a fairly serious error. We could forcefully

        // map everything here, but it should have been mapped during first-
@@ -1008,6 +1016,15 @@ std::string SnapshotManager::GetForwardMergeIndicatorPath() { 
}



void SnapshotManager::AcknowledgeMergeSuccess(LockedFile* lock) {

    // It's not possible to remove update state in recovery, so write an

    // indicator that cleanup is needed on reboot. If a factory data reset

    // was requested, it doesn't matter, everything will get wiped anyway.

    // To make testing easier we consider a /data wipe as cleaned up.

    if (device_->IsRecovery() && !in_factory_data_reset_) {

        WriteUpdateState(lock, UpdateState::MergeCompleted);

        return;

    }



    RemoveAllUpdateState(lock);

}
@@ -2528,7 +2545,43 @@ bool SnapshotManager::HandleImminentDataWipe(const std::function<void()>& callba 
        }

        return true;

    };

    if (!ProcessUpdateStateOnDataWipe(true /* allow_forward_merge */, process_callback)) {



    in_factory_data_reset_ = true;

    bool ok = ProcessUpdateStateOnDataWipe(true /* allow_forward_merge */, process_callback);

    in_factory_data_reset_ = false;



    if (!ok) {

        return false;

    }



    // 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;

}



bool SnapshotManager::FinishMergeInRecovery() {

    if (!device_->IsRecovery()) {

        LOG(ERROR) << "Data wipes are only allowed in recovery.";

        return false;

    }



    auto mount = EnsureMetadataMounted();

    if (!mount || !mount->HasDevice()) {

        return false;

    }



    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();

    if (state != UpdateState::MergeCompleted) {

        LOG(ERROR) << "Merge returned unexpected status: " << state;

        return false;

    }