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

Commit 116be4e2 authored by David Anderson's avatar David Anderson
Browse files

libsnapshot: Propagate the compression bit across state changes. 

This bit was getting lost because InitiateMerge() did not save the
compression bit when overwriting the update state.

Bug: N/A
Test: vts_libsnapshot_test
Test: reboot during merge phase of VABC OTA
Change-Id: I1a2219b501088de352a9c31d4b8b1a3f72d0e159
parent cf41cb7a

fs_mgr/libsnapshot/snapshot.cpp

+27 −2
Original line number Diff line number Diff line
@@ -676,6 +676,8 @@ bool SnapshotManager::InitiateMerge(uint64_t* cow_file_size) { 
        }

    }



    bool compression_enabled = false;



    uint64_t total_cow_file_size = 0;

    DmTargetSnapshot::Status initial_target_values = {};

    for (const auto& snapshot : snapshots) {
@@ -692,6 +694,8 @@ bool SnapshotManager::InitiateMerge(uint64_t* cow_file_size) { 
            return false;

        }

        total_cow_file_size += snapshot_status.cow_file_size();



        compression_enabled |= snapshot_status.compression_enabled();

    }



    if (cow_file_size) {
@@ -703,6 +707,7 @@ bool SnapshotManager::InitiateMerge(uint64_t* cow_file_size) { 
    initial_status.set_sectors_allocated(initial_target_values.sectors_allocated);

    initial_status.set_total_sectors(initial_target_values.total_sectors);

    initial_status.set_metadata_sectors(initial_target_values.metadata_sectors);

    initial_status.set_compression_enabled(compression_enabled);



    // Point of no return - mark that we're starting a merge. From now on every

    // snapshot must be a merge target.
@@ -2285,9 +2290,15 @@ SnapshotUpdateStatus SnapshotManager::ReadSnapshotUpdateStatus(LockedFile* lock) 
}



bool SnapshotManager::WriteUpdateState(LockedFile* lock, UpdateState state) {

    SnapshotUpdateStatus status = {};

    SnapshotUpdateStatus status;

    status.set_state(state);

    status.set_compression_enabled(IsCompressionEnabled());



    // If we're transitioning between two valid states (eg, we're not beginning

    // or ending an OTA), then make sure to propagate the compression bit.

    if (!(state == UpdateState::Initiated || state == UpdateState::None)) {

        SnapshotUpdateStatus old_status = ReadSnapshotUpdateStatus(lock);

        status.set_compression_enabled(old_status.compression_enabled());

    }

    return WriteSnapshotUpdateStatus(lock, status);

}
@@ -2477,6 +2488,12 @@ Return SnapshotManager::CreateUpdateSnapshots(const DeltaArchiveManifest& manife 
    auto lock = LockExclusive();

    if (!lock) return Return::Error();



    auto update_state = ReadUpdateState(lock.get());

    if (update_state != UpdateState::Initiated) {

        LOG(ERROR) << "Cannot create update snapshots in state " << update_state;

        return Return::Error();

    }



    // TODO(b/134949511): remove this check. Right now, with overlayfs mounted, the scratch

    // partition takes up a big chunk of space in super, causing COW images to be created on

    // retrofit Virtual A/B devices.
@@ -2571,6 +2588,14 @@ Return SnapshotManager::CreateUpdateSnapshots(const DeltaArchiveManifest& manife 
        return Return::Error();

    }



    SnapshotUpdateStatus status = {};

    status.set_state(update_state);

    status.set_compression_enabled(cow_creator.compression_enabled);

    if (!WriteSnapshotUpdateStatus(lock.get(), status)) {

        LOG(ERROR) << "Unable to write new update state";

        return Return::Error();

    }



    created_devices.Release();

    LOG(INFO) << "Successfully created all snapshots for target slot " << target_suffix;

fs_mgr/libsnapshot/snapshot_test.cpp

+3 −0
Original line number Diff line number Diff line
@@ -1106,6 +1106,7 @@ TEST_F(SnapshotUpdateTest, FullUpdateFlow) { 


    // Initiate the merge and wait for it to be completed.

    ASSERT_TRUE(init->InitiateMerge());

    ASSERT_EQ(init->IsSnapuserdRequired(), IsCompressionEnabled());

    ASSERT_EQ(UpdateState::MergeCompleted, init->ProcessUpdateState());



    // Check that the target partitions have the same content after the merge.
@@ -1130,6 +1131,7 @@ TEST_F(SnapshotUpdateTest, SnapshotOldPartitions) { 
    SetSize(sys_, 4_MiB);  // grows

    SetSize(vnd_, 2_MiB);  // shrinks

    // prd_b is unchanged

    ASSERT_TRUE(sm->BeginUpdate());

    ASSERT_TRUE(sm->CreateUpdateSnapshots(manifest_));

    ASSERT_EQ(4_MiB, GetSnapshotSize("sys_b").value_or(0));

}
@@ -1139,6 +1141,7 @@ TEST_F(SnapshotUpdateTest, SnapshotOldPartitions) { 
TEST_F(SnapshotUpdateTest, CowPartitionDoNotTakeOldPartitions) {

    SetSize(sys_, 2_MiB);  // shrinks

    // vnd_b and prd_b are unchanged.

    ASSERT_TRUE(sm->BeginUpdate());

    ASSERT_TRUE(sm->CreateUpdateSnapshots(manifest_));



    auto tgt = MetadataBuilder::New(*opener_, "super", 1);