Merge changes Ibeffa4a3,Ibce3bac9 am: d9a6144435
Original change: https://android-review.googlesource.com/c/platform/system/core/+/1507551
Change-Id: I360484df0d65534d64415083b390f8b7bcc5c1d2
diff --git a/fs_mgr/libsnapshot/device_info.cpp b/fs_mgr/libsnapshot/device_info.cpp
index 8770255..0e90100 100644
--- a/fs_mgr/libsnapshot/device_info.cpp
+++ b/fs_mgr/libsnapshot/device_info.cpp
@@ -120,9 +120,5 @@
#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 1f08860..d8d3d91 100644
--- a/fs_mgr/libsnapshot/device_info.h
+++ b/fs_mgr/libsnapshot/device_info.h
@@ -39,7 +39,6 @@
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 d5c263d..ef9d648 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 @@
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 c06b0b4..f8d3cdc 100644
--- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
+++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
@@ -107,7 +107,6 @@
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 0e9ba9e..aa9ba6e 100644
--- a/fs_mgr/libsnapshot/include/libsnapshot/snapuserd_client.h
+++ b/fs_mgr/libsnapshot/include/libsnapshot/snapuserd_client.h
@@ -30,7 +30,6 @@
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 @@
// 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_;
@@ -75,6 +70,11 @@
// Wait for snapuserd to disassociate with a dm-user control device. This
// must ONLY be called if the control device has already been deleted.
bool WaitForDeviceDelete(const std::string& control_device);
+
+ // Detach snapuserd. This shuts down the listener socket, and will cause
+ // snapuserd to gracefully exit once all handler threads have terminated.
+ // This should only be used on first-stage instances of snapuserd.
+ bool DetachSnapuserd();
};
} // namespace snapshot
diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapuserd_server.h b/fs_mgr/libsnapshot/include/libsnapshot/snapuserd_server.h
index cadfd71..1491aac 100644
--- a/fs_mgr/libsnapshot/include/libsnapshot/snapuserd_server.h
+++ b/fs_mgr/libsnapshot/include/libsnapshot/snapuserd_server.h
@@ -41,6 +41,7 @@
QUERY,
STOP,
DELETE,
+ DETACH,
INVALID,
};
@@ -106,6 +107,7 @@
bool IsTerminating() { return terminating_; }
void RunThread(std::shared_ptr<DmUserHandler> handler);
+ void JoinAllThreads();
// Find a DmUserHandler within a lock.
HandlerList::iterator FindHandler(std::lock_guard<std::mutex>* proof_of_lock,
diff --git a/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h b/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h
index 1d1510a..7aef086 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 @@
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 57e7b83..04c3ccc 100644
--- a/fs_mgr/libsnapshot/snapshot.cpp
+++ b/fs_mgr/libsnapshot/snapshot.cpp
@@ -1402,24 +1402,6 @@
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 @@
}
}
- 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 @@
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<int>(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 092ee0b..5319e69 100644
--- a/fs_mgr/libsnapshot/snapshot_fuzz_utils.h
+++ b/fs_mgr/libsnapshot/snapshot_fuzz_utils.h
@@ -124,7 +124,6 @@
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 74558ab..c6a1786 100644
--- a/fs_mgr/libsnapshot/snapshot_test.cpp
+++ b/fs_mgr/libsnapshot/snapshot_test.cpp
@@ -1760,9 +1760,6 @@
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 a5d2061..7282bff 100644
--- a/fs_mgr/libsnapshot/snapuserd_client.cpp
+++ b/fs_mgr/libsnapshot/snapuserd_client.cpp
@@ -54,25 +54,6 @@
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() {
@@ -229,5 +210,13 @@
return num_sectors;
}
+bool SnapuserdClient::DetachSnapuserd() {
+ if (!Sendmsg("detach")) {
+ LOG(ERROR) << "Failed to detach snapuserd.";
+ return false;
+ }
+ return true;
+}
+
} // namespace snapshot
} // namespace android
diff --git a/fs_mgr/libsnapshot/snapuserd_server.cpp b/fs_mgr/libsnapshot/snapuserd_server.cpp
index 9d57ab0..7a5cead 100644
--- a/fs_mgr/libsnapshot/snapuserd_server.cpp
+++ b/fs_mgr/libsnapshot/snapuserd_server.cpp
@@ -38,6 +38,7 @@
if (input == "stop") return DaemonOperations::STOP;
if (input == "query") return DaemonOperations::QUERY;
if (input == "delete") return DaemonOperations::DELETE;
+ if (input == "detach") return DaemonOperations::DETACH;
return DaemonOperations::INVALID;
}
@@ -72,19 +73,7 @@
void SnapuserdServer::ShutdownThreads() {
StopThreads();
-
- // Acquire the thread list within the lock.
- std::vector<std::shared_ptr<DmUserHandler>> dm_users;
- {
- std::lock_guard<std::mutex> guard(lock_);
- dm_users = std::move(dm_users_);
- }
-
- for (auto& client : dm_users) {
- auto& th = client->thread();
-
- if (th.joinable()) th.join();
- }
+ JoinAllThreads();
}
const std::string& DmUserHandler::GetMiscName() const {
@@ -214,6 +203,10 @@
}
return Sendmsg(fd, "success");
}
+ case DaemonOperations::DETACH: {
+ terminating_ = true;
+ return Sendmsg(fd, "success");
+ }
default: {
LOG(ERROR) << "Received unknown message type from client";
Sendmsg(fd, "fail");
@@ -234,7 +227,7 @@
LOG(INFO) << "Exiting thread for handler: " << handler->GetMiscName();
- // If the main thread called /emoveHandler, the handler was already removed
+ // If the main thread called RemoveHandler, the handler was already removed
// from within the lock, and calling RemoveHandler again has no effect.
RemoveHandler(handler->GetMiscName(), false);
}
@@ -286,9 +279,26 @@
}
}
}
+
+ JoinAllThreads();
return true;
}
+void SnapuserdServer::JoinAllThreads() {
+ // Acquire the thread list within the lock.
+ std::vector<std::shared_ptr<DmUserHandler>> dm_users;
+ {
+ std::lock_guard<std::mutex> guard(lock_);
+ dm_users = std::move(dm_users_);
+ }
+
+ for (auto& client : dm_users) {
+ auto& th = client->thread();
+
+ if (th.joinable()) th.join();
+ }
+}
+
void SnapuserdServer::AddWatchedFd(android::base::borrowed_fd fd) {
struct pollfd p = {};
p.fd = fd.get();