From 5aa05fce2825dac06f318596a8a96196c0d33615 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Sun, 22 Nov 2020 00:18:40 -0800 Subject: [PATCH] libsnapshot: Remove special-case code for first-stage init. Because init needs tighter integration with how to launch snapuserd, this code will be moved directly into init instead. Bug: 173476209 Test: manual test Change-Id: Ibce3bac9699620882eae44188d937d4900f022d3 --- fs_mgr/libsnapshot/device_info.cpp | 4 -- fs_mgr/libsnapshot/device_info.h | 1 - .../include/libsnapshot/mock_device_info.h | 1 - .../include/libsnapshot/snapshot.h | 1 - .../include/libsnapshot/snapuserd_client.h | 5 -- .../include_test/libsnapshot/test_helpers.h | 1 - fs_mgr/libsnapshot/snapshot.cpp | 50 ++----------------- fs_mgr/libsnapshot/snapshot_fuzz_utils.h | 1 - fs_mgr/libsnapshot/snapshot_test.cpp | 3 -- fs_mgr/libsnapshot/snapuserd_client.cpp | 19 ------- 10 files changed, 3 insertions(+), 83 deletions(-) diff --git a/fs_mgr/libsnapshot/device_info.cpp b/fs_mgr/libsnapshot/device_info.cpp index 87702555b..0e9010066 100644 --- a/fs_mgr/libsnapshot/device_info.cpp +++ b/fs_mgr/libsnapshot/device_info.cpp @@ -120,9 +120,5 @@ bool DeviceInfo::SetSlotAsUnbootable([[maybe_unused]] unsigned int slot) { #endif } -std::string DeviceInfo::GetSnapuserdFirstStagePidVar() const { - return kSnapuserdFirstStagePidVar; -} - } // namespace snapshot } // namespace android diff --git a/fs_mgr/libsnapshot/device_info.h b/fs_mgr/libsnapshot/device_info.h index 1f0886044..d8d3d91b0 100644 --- a/fs_mgr/libsnapshot/device_info.h +++ b/fs_mgr/libsnapshot/device_info.h @@ -39,7 +39,6 @@ class DeviceInfo final : public SnapshotManager::IDeviceInfo { bool SetBootControlMergeStatus(MergeStatus status) override; bool SetSlotAsUnbootable(unsigned int slot) override; bool IsRecovery() const override; - std::string GetSnapuserdFirstStagePidVar() const override; private: bool EnsureBootHal(); diff --git a/fs_mgr/libsnapshot/include/libsnapshot/mock_device_info.h b/fs_mgr/libsnapshot/include/libsnapshot/mock_device_info.h index d5c263d6c..ef9d64849 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/mock_device_info.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/mock_device_info.h @@ -32,7 +32,6 @@ class MockDeviceInfo : public SnapshotManager::IDeviceInfo { MOCK_METHOD(bool, SetBootControlMergeStatus, (MergeStatus status), (override)); MOCK_METHOD(bool, SetSlotAsUnbootable, (unsigned int slot), (override)); MOCK_METHOD(bool, IsRecovery, (), (const, override)); - MOCK_METHOD(std::string, GetSnapuserdFirstStagePidVar, (), (const, override)); }; } // namespace android::snapshot diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h index c06b0b4a6..f8d3cdcd2 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h @@ -107,7 +107,6 @@ class ISnapshotManager { virtual bool SetSlotAsUnbootable(unsigned int slot) = 0; virtual bool IsRecovery() const = 0; virtual bool IsTestDevice() const { return false; } - virtual std::string GetSnapuserdFirstStagePidVar() const = 0; }; virtual ~ISnapshotManager() = default; diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapuserd_client.h b/fs_mgr/libsnapshot/include/libsnapshot/snapuserd_client.h index 0e9ba9e74..2f671c255 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/snapuserd_client.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/snapuserd_client.h @@ -30,7 +30,6 @@ namespace snapshot { static constexpr uint32_t PACKET_SIZE = 512; -static constexpr char kSnapuserdSocketFirstStage[] = "snapuserd_first_stage"; static constexpr char kSnapuserdSocket[] = "snapuserd"; static constexpr char kSnapuserdFirstStagePidVar[] = "FIRST_STAGE_SNAPUSERD_PID"; @@ -38,10 +37,6 @@ static constexpr char kSnapuserdFirstStagePidVar[] = "FIRST_STAGE_SNAPUSERD_PID" // Ensure that the second-stage daemon for snapuserd is running. bool EnsureSnapuserdStarted(); -// Start the first-stage version of snapuserd, returning its pid. This is used -// by first-stage init, as well as vts_libsnapshot_test. On failure, -1 is returned. -pid_t StartFirstStageSnapuserd(); - class SnapuserdClient { private: android::base::unique_fd sockfd_; diff --git a/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h b/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h index 1d1510a43..7aef086bb 100644 --- a/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h +++ b/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h @@ -96,7 +96,6 @@ class TestDeviceInfo : public SnapshotManager::IDeviceInfo { return true; } bool IsTestDevice() const override { return true; } - std::string GetSnapuserdFirstStagePidVar() const override { return {}; } bool IsSlotUnbootable(uint32_t slot) { return unbootable_slots_.count(slot) != 0; } diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp index 57e7b8382..04c3ccce9 100644 --- a/fs_mgr/libsnapshot/snapshot.cpp +++ b/fs_mgr/libsnapshot/snapshot.cpp @@ -1402,24 +1402,6 @@ bool SnapshotManager::PerformSecondStageTransition() { LOG(ERROR) << "Could not transition all snapuserd consumers."; return false; } - - std::string pid_var = device_->GetSnapuserdFirstStagePidVar(); - if (pid_var.empty()) { - return true; - } - - int pid; - const char* pid_str = getenv(pid_var.c_str()); - if (pid_str && android::base::ParseInt(pid_str, &pid)) { - if (kill(pid, SIGTERM) < 0 && errno != ESRCH) { - LOG(ERROR) << "kill snapuserd failed"; - return false; - } - } else { - LOG(ERROR) << "Could not find or parse " << kSnapuserdFirstStagePidVar - << " for snapuserd pid"; - return false; - } return true; } @@ -1764,15 +1746,6 @@ bool SnapshotManager::MapAllPartitions(LockedFile* lock, const std::string& supe } } - if (use_first_stage_snapuserd_) { - // Remove the first-stage socket as a precaution, there is no need to - // access the daemon anymore and we'll be killing it once second-stage - // is running. - auto socket = ANDROID_SOCKET_DIR + "/"s + kSnapuserdSocketFirstStage; - snapuserd_client_ = nullptr; - unlink(socket.c_str()); - } - LOG(INFO) << "Created logical partitions with snapshot."; return true; } @@ -2419,28 +2392,11 @@ bool SnapshotManager::EnsureSnapuserdConnected() { return true; } - std::string socket; - if (use_first_stage_snapuserd_) { - auto pid = StartFirstStageSnapuserd(); - if (pid < 0) { - LOG(ERROR) << "Failed to start snapuserd"; - return false; - } - - auto pid_str = std::to_string(static_cast(pid)); - if (setenv(kSnapuserdFirstStagePidVar, pid_str.c_str(), 1) < 0) { - PLOG(ERROR) << "setenv failed storing the snapuserd pid"; - } - - socket = kSnapuserdSocketFirstStage; - } else { - if (!EnsureSnapuserdStarted()) { - return false; - } - socket = kSnapuserdSocket; + if (!use_first_stage_snapuserd_ && !EnsureSnapuserdStarted()) { + return false; } - snapuserd_client_ = SnapuserdClient::Connect(socket, 10s); + snapuserd_client_ = SnapuserdClient::Connect(kSnapuserdSocket, 10s); if (!snapuserd_client_) { LOG(ERROR) << "Unable to connect to snapuserd"; return false; diff --git a/fs_mgr/libsnapshot/snapshot_fuzz_utils.h b/fs_mgr/libsnapshot/snapshot_fuzz_utils.h index 092ee0ba2..5319e69de 100644 --- a/fs_mgr/libsnapshot/snapshot_fuzz_utils.h +++ b/fs_mgr/libsnapshot/snapshot_fuzz_utils.h @@ -124,7 +124,6 @@ class SnapshotFuzzDeviceInfo : public ISnapshotManager::IDeviceInfo { return data_->allow_set_slot_as_unbootable(); } bool IsRecovery() const override { return data_->is_recovery(); } - std::string GetSnapuserdFirstStagePidVar() const override { return {}; } void SwitchSlot() { switched_slot_ = !switched_slot_; } diff --git a/fs_mgr/libsnapshot/snapshot_test.cpp b/fs_mgr/libsnapshot/snapshot_test.cpp index 74558ab88..c6a178622 100644 --- a/fs_mgr/libsnapshot/snapshot_test.cpp +++ b/fs_mgr/libsnapshot/snapshot_test.cpp @@ -1760,9 +1760,6 @@ TEST_F(SnapshotUpdateTest, DaemonTransition) { GTEST_SKIP() << "Skipping Virtual A/B Compression test"; } - AutoKill auto_kill(StartFirstStageSnapuserd()); - ASSERT_TRUE(auto_kill.valid()); - // Ensure a connection to the second-stage daemon, but use the first-stage // code paths thereafter. ASSERT_TRUE(sm->EnsureSnapuserdConnected()); diff --git a/fs_mgr/libsnapshot/snapuserd_client.cpp b/fs_mgr/libsnapshot/snapuserd_client.cpp index a5d20611b..9ee13db7f 100644 --- a/fs_mgr/libsnapshot/snapuserd_client.cpp +++ b/fs_mgr/libsnapshot/snapuserd_client.cpp @@ -54,25 +54,6 @@ bool EnsureSnapuserdStarted() { return true; } -pid_t StartFirstStageSnapuserd() { - pid_t pid = fork(); - if (pid < 0) { - PLOG(ERROR) << "fork failed"; - return pid; - } - if (pid != 0) { - return pid; - } - - std::string arg0 = "/system/bin/snapuserd"; - std::string arg1 = kSnapuserdSocketFirstStage; - char* const argv[] = {arg0.data(), arg1.data(), nullptr}; - if (execv(arg0.c_str(), argv) < 0) { - PLOG(FATAL) << "execv failed"; - } - return pid; -} - SnapuserdClient::SnapuserdClient(android::base::unique_fd&& sockfd) : sockfd_(std::move(sockfd)) {} static inline bool IsRetryErrno() {