From 93faa18bce6fbdf92908a58e22dec98145f803c6 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Tue, 8 Mar 2022 23:13:47 -0800 Subject: [PATCH] libsnapshot: Add more feature flags to SnapshotMergeReport. This patch also begins reducing the complexity of SnapshotMergeStats by eliminating the indirection layer between the protobuf and SnapshotManager. Bug: 222117189 Test: statsd_testdrive Change-Id: I15d740121c381da7d8311f0cbbd0da82db877555 --- .../libsnapshot/android/snapshot/snapshot.proto | 9 +++++++++ .../include/libsnapshot/mock_snapshot.h | 1 + .../libsnapshot/mock_snapshot_merge_stats.h | 6 ++---- .../libsnapshot/include/libsnapshot/snapshot.h | 4 ++++ .../include/libsnapshot/snapshot_stats.h | 16 ++++++++-------- .../include/libsnapshot/snapshot_stub.h | 1 + fs_mgr/libsnapshot/snapshot.cpp | 17 ++++++++++++++--- fs_mgr/libsnapshot/snapshot_stats.cpp | 15 +-------------- fs_mgr/libsnapshot/snapshot_stub.cpp | 13 +++++++++---- 9 files changed, 49 insertions(+), 33 deletions(-) diff --git a/fs_mgr/libsnapshot/android/snapshot/snapshot.proto b/fs_mgr/libsnapshot/android/snapshot/snapshot.proto index 5daa84dc7..6ee8d4aa3 100644 --- a/fs_mgr/libsnapshot/android/snapshot/snapshot.proto +++ b/fs_mgr/libsnapshot/android/snapshot/snapshot.proto @@ -235,4 +235,13 @@ message SnapshotMergeReport { // The source fingerprint at the time the OTA was downloaded. string source_build_fingerprint = 10; + + // Whether this update attempt uses userspace snapshots. + bool userspace_snapshots_used = 11; + + // Whether this update attempt uses XOR compression. + bool xor_compression_used = 12; + + // Whether this update attempt used io_uring. + bool iouring_used = 13; } diff --git a/fs_mgr/libsnapshot/include/libsnapshot/mock_snapshot.h b/fs_mgr/libsnapshot/include/libsnapshot/mock_snapshot.h index ba62330f1..d458b8718 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/mock_snapshot.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/mock_snapshot.h @@ -62,6 +62,7 @@ class MockSnapshotManager : public ISnapshotManager { MOCK_METHOD(std::unique_ptr, EnsureMetadataMounted, (), (override)); MOCK_METHOD(ISnapshotMergeStats*, GetSnapshotMergeStatsInstance, (), (override)); MOCK_METHOD(std::string, ReadSourceBuildFingerprint, (), (override)); + MOCK_METHOD(void, SetMergeStatsFeatures, (ISnapshotMergeStats*), (override)); }; } // namespace android::snapshot diff --git a/fs_mgr/libsnapshot/include/libsnapshot/mock_snapshot_merge_stats.h b/fs_mgr/libsnapshot/include/libsnapshot/mock_snapshot_merge_stats.h index 3d384cc04..8280f2ea8 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/mock_snapshot_merge_stats.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/mock_snapshot_merge_stats.h @@ -28,10 +28,7 @@ class MockSnapshotMergeStats final : public ISnapshotMergeStats { virtual ~MockSnapshotMergeStats() = default; // Called when merge starts or resumes. MOCK_METHOD(bool, Start, (), (override)); - MOCK_METHOD(void, set_state, (android::snapshot::UpdateState, bool), (override)); - MOCK_METHOD(void, set_cow_file_size, (uint64_t), ()); - MOCK_METHOD(void, set_total_cow_size_bytes, (uint64_t), (override)); - MOCK_METHOD(void, set_estimated_cow_size_bytes, (uint64_t), (override)); + MOCK_METHOD(void, set_state, (android::snapshot::UpdateState), (override)); MOCK_METHOD(void, set_boot_complete_time_ms, (uint32_t), (override)); MOCK_METHOD(void, set_boot_complete_to_merge_start_time_ms, (uint32_t), (override)); MOCK_METHOD(void, set_merge_failure_code, (MergeFailureCode), (override)); @@ -45,6 +42,7 @@ class MockSnapshotMergeStats final : public ISnapshotMergeStats { MOCK_METHOD(MergeFailureCode, merge_failure_code, (), (override)); MOCK_METHOD(std::unique_ptr, Finish, (), (override)); MOCK_METHOD(bool, WriteState, (), (override)); + MOCK_METHOD(SnapshotMergeReport*, report, (), (override)); using ISnapshotMergeStats::Result; // Return nullptr if any failure. diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h index 38b47d52b..85c805d3c 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h @@ -134,6 +134,9 @@ class ISnapshotManager { // may need to be merged before wiping. virtual bool FinishedSnapshotWrites(bool wipe) = 0; + // Set feature flags on an ISnapshotMergeStats object. + virtual void SetMergeStatsFeatures(ISnapshotMergeStats* stats) = 0; + // Update an ISnapshotMergeStats object with statistics about COW usage. // This should be called before the merge begins as otherwise snapshots // may be deleted. @@ -378,6 +381,7 @@ class SnapshotManager final : public ISnapshotManager { bool MapAllSnapshots(const std::chrono::milliseconds& timeout_ms = {}) override; bool UnmapAllSnapshots() override; std::string ReadSourceBuildFingerprint() override; + void SetMergeStatsFeatures(ISnapshotMergeStats* stats) override; // We can't use WaitForFile during first-stage init, because ueventd is not // running and therefore will not automatically create symlinks. Instead, diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot_stats.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot_stats.h index 8c2fec726..8a7040035 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot_stats.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot_stats.h @@ -28,10 +28,7 @@ class ISnapshotMergeStats { virtual ~ISnapshotMergeStats() = default; // Called when merge starts or resumes. virtual bool Start() = 0; - virtual void set_state(android::snapshot::UpdateState state, bool using_compression) = 0; - virtual void set_cow_file_size(uint64_t cow_file_size) = 0; - virtual void set_total_cow_size_bytes(uint64_t bytes) = 0; - virtual void set_estimated_cow_size_bytes(uint64_t bytes) = 0; + virtual void set_state(android::snapshot::UpdateState state) = 0; virtual void set_boot_complete_time_ms(uint32_t ms) = 0; virtual void set_boot_complete_to_merge_start_time_ms(uint32_t ms) = 0; virtual void set_merge_failure_code(MergeFailureCode code) = 0; @@ -55,6 +52,9 @@ class ISnapshotMergeStats { // Return nullptr if any failure. virtual std::unique_ptr Finish() = 0; + // Return the underlying implementation. + virtual SnapshotMergeReport* report() = 0; + // Write out the current state. This should be called when data might be lost that // cannot be recovered (eg the COW sizes). virtual bool WriteState() = 0; @@ -67,11 +67,8 @@ class SnapshotMergeStats : public ISnapshotMergeStats { // ISnapshotMergeStats overrides bool Start() override; - void set_state(android::snapshot::UpdateState state, bool using_compression) override; - void set_cow_file_size(uint64_t cow_file_size) override; + void set_state(android::snapshot::UpdateState state) override; uint64_t cow_file_size() override; - void set_total_cow_size_bytes(uint64_t bytes) override; - void set_estimated_cow_size_bytes(uint64_t bytes) override; uint64_t total_cow_size_bytes() override; uint64_t estimated_cow_size_bytes() override; void set_boot_complete_time_ms(uint32_t ms) override; @@ -85,6 +82,9 @@ class SnapshotMergeStats : public ISnapshotMergeStats { std::unique_ptr Finish() override; bool WriteState() override; + // Access the underlying report before it is finished. + SnapshotMergeReport* report() override { return &report_; } + private: bool ReadState(); bool DeleteState(); diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot_stub.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot_stub.h index 318e5259d..171c7c6f9 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot_stub.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot_stub.h @@ -59,6 +59,7 @@ class SnapshotManagerStub : public ISnapshotManager { bool MapAllSnapshots(const std::chrono::milliseconds& timeout_ms) override; bool UnmapAllSnapshots() override; std::string ReadSourceBuildFingerprint() override; + void SetMergeStatsFeatures(ISnapshotMergeStats* stats) override; }; } // namespace android::snapshot diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp index d086f29a6..505ecc86d 100644 --- a/fs_mgr/libsnapshot/snapshot.cpp +++ b/fs_mgr/libsnapshot/snapshot.cpp @@ -4120,9 +4120,20 @@ void SnapshotManager::UpdateCowStats(ISnapshotMergeStats* stats) { estimated_cow_size += status.estimated_cow_size(); } - stats->set_cow_file_size(cow_file_size); - stats->set_total_cow_size_bytes(total_cow_size); - stats->set_estimated_cow_size_bytes(estimated_cow_size); + stats->report()->set_cow_file_size(cow_file_size); + stats->report()->set_total_cow_size_bytes(total_cow_size); + stats->report()->set_estimated_cow_size_bytes(estimated_cow_size); +} + +void SnapshotManager::SetMergeStatsFeatures(ISnapshotMergeStats* stats) { + auto lock = LockExclusive(); + if (!lock) return; + + SnapshotUpdateStatus update_status = ReadSnapshotUpdateStatus(lock.get()); + stats->report()->set_iouring_used(update_status.io_uring_enabled()); + stats->report()->set_userspace_snapshots_used(update_status.userspace_snapshots()); + stats->report()->set_xor_compression_used( + android::base::GetBoolProperty("ro.virtual_ab.compression.xor.enabled", false)); } bool SnapshotManager::DeleteDeviceIfExists(const std::string& name, diff --git a/fs_mgr/libsnapshot/snapshot_stats.cpp b/fs_mgr/libsnapshot/snapshot_stats.cpp index 712eafba9..9b6eb2c62 100644 --- a/fs_mgr/libsnapshot/snapshot_stats.cpp +++ b/fs_mgr/libsnapshot/snapshot_stats.cpp @@ -84,27 +84,14 @@ bool SnapshotMergeStats::Start() { return WriteState(); } -void SnapshotMergeStats::set_state(android::snapshot::UpdateState state, bool using_compression) { +void SnapshotMergeStats::set_state(android::snapshot::UpdateState state) { report_.set_state(state); - report_.set_compression_enabled(using_compression); -} - -void SnapshotMergeStats::set_cow_file_size(uint64_t cow_file_size) { - report_.set_cow_file_size(cow_file_size); } uint64_t SnapshotMergeStats::cow_file_size() { return report_.cow_file_size(); } -void SnapshotMergeStats::set_total_cow_size_bytes(uint64_t bytes) { - report_.set_total_cow_size_bytes(bytes); -} - -void SnapshotMergeStats::set_estimated_cow_size_bytes(uint64_t bytes) { - report_.set_estimated_cow_size_bytes(bytes); -} - uint64_t SnapshotMergeStats::total_cow_size_bytes() { return report_.total_cow_size_bytes(); } diff --git a/fs_mgr/libsnapshot/snapshot_stub.cpp b/fs_mgr/libsnapshot/snapshot_stub.cpp index 4af5367ac..84e2226fa 100644 --- a/fs_mgr/libsnapshot/snapshot_stub.cpp +++ b/fs_mgr/libsnapshot/snapshot_stub.cpp @@ -128,12 +128,9 @@ bool SnapshotManagerStub::UpdateUsesUserSnapshots() { class SnapshotMergeStatsStub : public ISnapshotMergeStats { bool Start() override { return false; } - void set_state(android::snapshot::UpdateState, bool) override {} - void set_cow_file_size(uint64_t) override {} + void set_state(android::snapshot::UpdateState) override {} uint64_t cow_file_size() override { return 0; } std::unique_ptr Finish() override { return nullptr; } - void set_total_cow_size_bytes(uint64_t) override {} - void set_estimated_cow_size_bytes(uint64_t) override {} uint64_t total_cow_size_bytes() override { return 0; } uint64_t estimated_cow_size_bytes() override { return 0; } void set_boot_complete_time_ms(uint32_t) override {} @@ -145,6 +142,10 @@ class SnapshotMergeStatsStub : public ISnapshotMergeStats { void set_source_build_fingerprint(const std::string&) override {} std::string source_build_fingerprint() override { return {}; } bool WriteState() override { return false; } + SnapshotMergeReport* report() override { return &report_; } + + private: + SnapshotMergeReport report_; }; ISnapshotMergeStats* SnapshotManagerStub::GetSnapshotMergeStatsInstance() { @@ -183,4 +184,8 @@ std::string SnapshotManagerStub::ReadSourceBuildFingerprint() { return {}; } +void SnapshotManagerStub::SetMergeStatsFeatures(ISnapshotMergeStats*) { + LOG(ERROR) << __FUNCTION__ << " should never be called."; +} + } // namespace android::snapshot