Merge changes I903438b2,Ic7e8351a

* changes:
  Cvd server supports in-place updates
  Save in-memory data to a memory file

5 files changed, 242 insertions, 51 deletions

diff --git a/host/commands/cvd/main.cc b/host/commands/cvd/main.cc
index 9d7a975d3..532541fb8 100644
--- a/host/commands/cvd/main.cc
+++ b/host/commands/cvd/main.cc
@@ -23,7 +23,6 @@
 
 #include <android-base/file.h>
 #include <android-base/logging.h>
-#include <android-base/result.h>
 
 #include "common/libs/fs/shared_buf.h"
 #include "common/libs/fs/shared_fd.h"
@@ -32,11 +31,13 @@
 #include "common/libs/utils/json.h"
 #include "common/libs/utils/result.h"
 #include "common/libs/utils/shared_fd_flag.h"
+#include "common/libs/utils/tee_logging.h"
 #include "host/commands/cvd/client.h"
 #include "host/commands/cvd/common_utils.h"
 #include "host/commands/cvd/fetch/fetch_cvd.h"
 #include "host/commands/cvd/frontline_parser.h"
 #include "host/commands/cvd/handle_reset.h"
+#include "host/commands/cvd/logger.h"
 #include "host/commands/cvd/reset_client_utils.h"
 #include "host/commands/cvd/server.h"
 #include "host/commands/cvd/server_constants.h"
@@ -69,21 +70,53 @@ bool IsServerModeExpected(const std::string& exec_file) {
   return exec_file == kServerExecPath;
 }
 
-Result<void> RunServer(const SharedFD& internal_server_fd,
-                       const SharedFD& carryover_client_fd) {
-  if (!internal_server_fd->IsOpen()) {
+struct RunServerParam {
+  SharedFD internal_server_fd;
+  SharedFD carryover_client_fd;
+  std::optional<SharedFD> memory_carryover_fd;
+  /**
+   * Cvd server usually prints out in the client's stream. However,
+   * after Exec(), the client stdout and stderr becomes unreachable by
+   * LOG(ERROR), etc.
+   *
+   * Thus, in that case, the client fd is passed to print Exec() log
+   * on it.
+   *
+   */
+  SharedFD carryover_stderr_fd;
+};
+Result<void> RunServer(const RunServerParam& fds) {
+  if (!fds.internal_server_fd->IsOpen()) {
     return CF_ERR(
         "Expected to be in server mode, but didn't get a server "
         "fd: "
-        << internal_server_fd->StrError());
+        << fds.internal_server_fd->StrError());
+  }
+  std::unique_ptr<ServerLogger> server_logger =
+      std::make_unique<ServerLogger>();
+  CF_EXPECT(server_logger != nullptr, "ServerLogger memory allocation failed.");
+
+  std::unique_ptr<ServerLogger::ScopedLogger> scoped_logger;
+  if (fds.carryover_stderr_fd->IsOpen()) {
+    scoped_logger = std::make_unique<ServerLogger::ScopedLogger>(
+        std::move(server_logger->LogThreadToFd(fds.carryover_stderr_fd)));
   }
-  CF_EXPECT(CvdServerMain(internal_server_fd, carryover_client_fd));
+  if (fds.memory_carryover_fd && !(*fds.memory_carryover_fd)->IsOpen()) {
+    LOG(ERROR) << "Memory carryover file is supposed to be open but is not.";
+  }
+  CF_EXPECT(CvdServerMain({.internal_server_fd = fds.internal_server_fd,
+                           .carryover_client_fd = fds.carryover_client_fd,
+                           .memory_carryover_fd = fds.memory_carryover_fd,
+                           .server_logger = std::move(server_logger),
+                           .scoped_logger = std::move(scoped_logger)}));
   return {};
 }
 
 struct ParseResult {
-  SharedFD internal_server_fd_;
-  SharedFD carryover_client_fd_;
+  SharedFD internal_server_fd;
+  SharedFD carryover_client_fd;
+  std::optional<SharedFD> memory_carryover_fd;
+  SharedFD carryover_stderr_fd;
 };
 
 Result<ParseResult> ParseIfServer(std::vector<std::string>& all_args) {
@@ -93,9 +126,23 @@ Result<ParseResult> ParseIfServer(std::vector<std::string>& all_args) {
   SharedFD carryover_client_fd;
   flags.emplace_back(
       SharedFDFlag("INTERNAL_carryover_client_fd", carryover_client_fd));
-
+  SharedFD carryover_stderr_fd;
+  flags.emplace_back(
+      SharedFDFlag("INTERNAL_carryover_stderr_fd", carryover_stderr_fd));
+  SharedFD memory_carryover_fd;
+  flags.emplace_back(
+      SharedFDFlag("INTERNAL_memory_carryover_fd", memory_carryover_fd));
   CF_EXPECT(ParseFlags(flags, all_args));
-  ParseResult result = {internal_server_fd, carryover_client_fd};
+  std::optional<SharedFD> memory_carryover_fd_opt;
+  if (memory_carryover_fd->IsOpen()) {
+    memory_carryover_fd_opt = std::move(memory_carryover_fd);
+  }
+  ParseResult result = {
+      .internal_server_fd = internal_server_fd,
+      .carryover_client_fd = carryover_client_fd,
+      .memory_carryover_fd = memory_carryover_fd_opt,
+      .carryover_stderr_fd = carryover_stderr_fd,
+  };
   return {result};
 }
 
@@ -195,9 +242,12 @@ Result<void> CvdMain(int argc, char** argv, char** envp) {
   }
 
   if (IsServerModeExpected(all_args[0])) {
-    auto [internal_server_fd, carryover_client_fd] =
-        CF_EXPECT(ParseIfServer(all_args));
-    return RunServer(internal_server_fd, carryover_client_fd);
+    auto parsed_fds = CF_EXPECT(ParseIfServer(all_args));
+
+    return RunServer({.internal_server_fd = parsed_fds.internal_server_fd,
+                      .carryover_client_fd = parsed_fds.carryover_client_fd,
+                      .memory_carryover_fd = parsed_fds.memory_carryover_fd,
+                      .carryover_stderr_fd = parsed_fds.carryover_stderr_fd});
   }
 
   CF_EXPECT_EQ(android::base::Basename(all_args[0]), "cvd");
diff --git a/host/commands/cvd/server.cc b/host/commands/cvd/server.cc
index 407801323..d2cea1ac3 100644
--- a/host/commands/cvd/server.cc
+++ b/host/commands/cvd/server.cc
@@ -189,25 +189,56 @@ void CvdServer::Join() {
   }
 }
 
-Result<void> CvdServer::Exec(SharedFD new_exe, SharedFD client_fd) {
+Result<void> CvdServer::Exec(const ExecParam& exec_param) {
   CF_EXPECT(server_fd_->IsOpen(), "Server not running");
   Stop();
   android::base::unique_fd server_dup{server_fd_->UNMANAGED_Dup()};
   CF_EXPECT(server_dup.get() >= 0, "dup: \"" << server_fd_->StrError() << "\"");
-  android::base::unique_fd client_dup{client_fd->UNMANAGED_Dup()};
+  android::base::unique_fd client_dup{
+      exec_param.carryover_client_fd->UNMANAGED_Dup()};
   CF_EXPECT(client_dup.get() >= 0, "dup: \"" << server_fd_->StrError() << "\"");
-  std::vector<std::string> argv_str = {
+  android::base::unique_fd client_stderr_dup{
+      exec_param.client_stderr_fd->UNMANAGED_Dup()};
+  CF_EXPECT(client_stderr_dup.get() >= 0,
+            "dup: \"" << exec_param.client_stderr_fd->StrError() << "\"");
+  cvd_common::Args argv_str = {
       kServerExecPath,
       "-INTERNAL_server_fd=" + std::to_string(server_dup.get()),
       "-INTERNAL_carryover_client_fd=" + std::to_string(client_dup.get()),
+      "-INTERNAL_carryover_stderr_fd=" +
+          std::to_string(client_stderr_dup.get()),
   };
+
+  int in_memory_dup = -1;
+  ScopeGuard exit_action([&in_memory_dup]() {
+    if (in_memory_dup >= 0) {
+      if (close(in_memory_dup) != 0) {
+        LOG(ERROR) << "Failed to close file " << in_memory_dup;
+      }
+    }
+  });
+  if (exec_param.in_memory_data_fd) {
+    in_memory_dup = exec_param.in_memory_data_fd.value()->UNMANAGED_Dup();
+    CF_EXPECT(
+        in_memory_dup >= 0,
+        "dup: \"" << exec_param.in_memory_data_fd.value()->StrError() << "\"");
+    argv_str.push_back("-INTERNAL_memory_carryover_fd=" +
+                       std::to_string(in_memory_dup));
+  }
+
   std::vector<char*> argv_cstr;
   for (const auto& argv : argv_str) {
     argv_cstr.emplace_back(strdup(argv.c_str()));
   }
   argv_cstr.emplace_back(nullptr);
-  android::base::unique_fd new_exe_dup{new_exe->UNMANAGED_Dup()};
-  CF_EXPECT(new_exe_dup.get() >= 0, "dup: \"" << new_exe->StrError() << "\"");
+  android::base::unique_fd new_exe_dup{exec_param.new_exe->UNMANAGED_Dup()};
+  CF_EXPECT(new_exe_dup.get() >= 0,
+            "dup: \"" << exec_param.new_exe->StrError() << "\"");
+
+  if (exec_param.verbose) {
+    LOG(ERROR) << "Server Exec'ing: " << android::base::Join(argv_str, " ");
+  }
+
   fexecve(new_exe_dup.get(), argv_cstr.data(), environ);
   for (const auto& argv : argv_cstr) {
     free(argv);
@@ -241,18 +272,20 @@ Result<void> CvdServer::StartServer(SharedFD server_fd) {
   return {};
 }
 
-Result<void> CvdServer::AcceptCarryoverClient(SharedFD client) {
-  cvd::Response success_message;
-  success_message.mutable_status()->set_code(cvd::Status::OK);
-  success_message.mutable_command_response();
-  CF_EXPECT(SendResponse(client, success_message));
-
+Result<void> CvdServer::AcceptCarryoverClient(
+    SharedFD client,
+    // the passed ScopedLogger should be destroyed on return of this function.
+    std::unique_ptr<ServerLogger::ScopedLogger>) {
   auto self_cb = [this](EpollEvent ev) -> Result<void> {
     CF_EXPECT(HandleMessage(ev));
     return {};
   };
   CF_EXPECT(epoll_pool_.Register(client, EPOLLIN, self_cb));
 
+  cvd::Response success_message;
+  success_message.mutable_status()->set_code(cvd::Status::OK);
+  success_message.mutable_command_response();
+  CF_EXPECT(SendResponse(client, success_message));
   return {};
 }
 
@@ -430,25 +463,35 @@ Result<cvd::Response> CvdServer::HandleRequest(RequestWithStdio orig_request,
   return response;
 }
 
-static fruit::Component<> ServerComponent() {
+Result<void> CvdServer::InstanceDbFromJson(const std::string& json_string) {
+  const uid_t uid = getuid();
+  auto json = CF_EXPECT(ParseJson(json_string));
+  CF_EXPECT(instance_manager_.LoadFromJson(uid, json));
+  return {};
+}
+
+static fruit::Component<> ServerComponent(ServerLogger* server_logger) {
   return fruit::createComponent()
       .addMultibinding<CvdServer, CvdServer>()
+      .bindInstance(*server_logger)
       .install(BuildApiModule)
       .install(EpollLoopComponent)
       .install(HostToolTargetManagerComponent)
       .install(OperationToBinsMapComponent);
 }
 
-Result<int> CvdServerMain(SharedFD server_fd, SharedFD carryover_client) {
+Result<int> CvdServerMain(ServerMainParam&& fds) {
   LOG(INFO) << "Starting server";
 
   CF_EXPECT(daemon(0, 0) != -1, strerror(errno));
 
   signal(SIGPIPE, SIG_IGN);
 
+  SharedFD server_fd = std::move(fds.internal_server_fd);
   CF_EXPECT(server_fd->IsOpen(), "Did not receive a valid cvd_server fd");
 
-  fruit::Injector<> injector(ServerComponent);
+  std::unique_ptr<ServerLogger> server_logger = std::move(fds.server_logger);
+  fruit::Injector<> injector(ServerComponent, server_logger.get());
 
   for (auto& late_injected : injector.getMultibindings<LateInjected>()) {
     CF_EXPECT(late_injected->LateInject(injector));
@@ -458,15 +501,44 @@ Result<int> CvdServerMain(SharedFD server_fd, SharedFD carryover_client) {
   CF_EXPECT(server_bindings.size() == 1,
             "Expected 1 server binding, got " << server_bindings.size());
   auto& server = *(server_bindings[0]);
+
+  std::optional<SharedFD> memory_carryover_fd =
+      std::move(fds.memory_carryover_fd);
+  if (memory_carryover_fd) {
+    const std::string json_string =
+        CF_EXPECT(ReadAllFromMemFd(*memory_carryover_fd));
+    CF_EXPECT(server.InstanceDbFromJson(json_string),
+              "Failed to load from: " << json_string);
+  }
+
   server.StartServer(server_fd);
 
+  SharedFD carryover_client = std::move(fds.carryover_client_fd);
+  // The carryover_client wouldn't be available after AcceptCarryoverClient()
   if (carryover_client->IsOpen()) {
-    CF_EXPECT(server.AcceptCarryoverClient(carryover_client));
+    // release scoped_logger for this thread inside AcceptCarryoverClient()
+    CF_EXPECT(server.AcceptCarryoverClient(carryover_client,
+                                           std::move(fds.scoped_logger)));
+  } else {
+    // release scoped_logger now and delete the object
+    fds.scoped_logger.reset();
   }
-
   server.Join();
 
   return 0;
 }
 
+Result<std::string> ReadAllFromMemFd(const SharedFD& mem_fd) {
+  const auto n_message_size = mem_fd->LSeek(0, SEEK_END);
+  CF_EXPECT_NE(n_message_size, -1, "LSeek on the memory file failed.");
+  std::vector<char> buffer(n_message_size);
+  CF_EXPECT_EQ(mem_fd->LSeek(0, SEEK_SET), 0, mem_fd->StrError());
+  auto n_read = ReadExact(mem_fd, buffer.data(), n_message_size);
+  CF_EXPECT(n_read == n_message_size,
+            "Expected to read " << n_message_size << " bytes but actually read "
+                                << n_read << " bytes.");
+  std::string message(buffer.begin(), buffer.end());
+  return message;
+}
+
 }  // namespace cuttlefish
diff --git a/host/commands/cvd/server.h b/host/commands/cvd/server.h
index de7ca02b3..fa375c26a 100644
--- a/host/commands/cvd/server.h
+++ b/host/commands/cvd/server.h
@@ -42,17 +42,46 @@
 
 namespace cuttlefish {
 
+struct ServerMainParam {
+  SharedFD internal_server_fd;
+  SharedFD carryover_client_fd;
+  std::optional<SharedFD> memory_carryover_fd;
+  std::unique_ptr<ServerLogger> server_logger;
+  /* scoped logger that carries the stderr of the carried-over
+   * client. The client may have called "cvd restart-server."
+   *
+   * The scoped_logger should expire just after AcceptCarryoverClient()
+   */
+  std::unique_ptr<ServerLogger::ScopedLogger> scoped_logger;
+};
+Result<int> CvdServerMain(ServerMainParam&& fds);
+
 class CvdServer {
+  // for server_logger_.
+  // server_logger_ shouldn't be exposed to anything but CvdServerMain()
+  friend Result<int> CvdServerMain(ServerMainParam&& fds);
+
  public:
   INJECT(CvdServer(BuildApi&, EpollPool&, InstanceManager&,
                    HostToolTargetManager&, ServerLogger&));
   ~CvdServer();
 
   Result<void> StartServer(SharedFD server);
-  Result<void> Exec(SharedFD new_exe, SharedFD client);
-  Result<void> AcceptCarryoverClient(SharedFD client);
+  struct ExecParam {
+    SharedFD new_exe;
+    SharedFD carryover_client_fd;  // the client that called cvd restart-server
+    std::optional<SharedFD>
+        in_memory_data_fd;  // fd to carry over in-memory data
+    SharedFD client_stderr_fd;
+    bool verbose;
+  };
+  Result<void> Exec(const ExecParam&);
+  Result<void> AcceptCarryoverClient(
+      SharedFD client,
+      std::unique_ptr<ServerLogger::ScopedLogger> scoped_logger);
   void Stop();
   void Join();
+  Result<void> InstanceDbFromJson(const std::string& json_string);
 
  private:
   struct OngoingRequest {
@@ -91,6 +120,7 @@ Result<CvdServerHandler*> RequestHandler(
     const RequestWithStdio& request,
     const std::vector<CvdServerHandler*>& handlers);
 
-Result<int> CvdServerMain(SharedFD server_fd, SharedFD carryover_client);
+// Read all contents from the file
+Result<std::string> ReadAllFromMemFd(const SharedFD& mem_fd);
 
 }  // namespace cuttlefish
diff --git a/host/commands/cvd/server_command/restart.cpp b/host/commands/cvd/server_command/restart.cpp
index 4c005095e..3116be3b8 100644
--- a/host/commands/cvd/server_command/restart.cpp
+++ b/host/commands/cvd/server_command/restart.cpp
@@ -18,18 +18,19 @@
 
 #include <sys/types.h>
 
+#include <cstdio>
 #include <iostream>
 
 #include <android-base/file.h>
-#include <android-base/strings.h>
 #include <fruit/fruit.h>
 
-#include "cvd_server.pb.h"
-
 #include "common/libs/fs/shared_buf.h"
 #include "common/libs/fs/shared_fd.h"
+#include "common/libs/utils/contains.h"
 #include "common/libs/utils/result.h"
-#include "host/commands/cvd/flag.h"
+#include "cvd_server.pb.h"
+#include "host/commands/cvd/common_utils.h"
+#include "host/commands/cvd/epoll_loop.h"
 #include "host/commands/cvd/frontline_parser.h"
 #include "host/commands/cvd/instance_manager.h"
 #include "host/commands/cvd/server_command/components.h"
@@ -42,6 +43,7 @@ namespace {
 
 constexpr char kRestartServerHelpMessage[] =
     R"(Cuttlefish Virtual Device (CVD) CLI.
+
 usage: cvd restart-server <common args> <mode> <mode args>
 
 Common Args:
@@ -101,11 +103,7 @@ class CvdRestartHandler : public CvdServerHandler {
 
   Result<cvd::Response> Handle(const RequestWithStdio& request) override {
     CF_EXPECT(CanHandle(request));
-    CF_EXPECT(request.Credentials() != std::nullopt);
-    const uid_t uid = request.Credentials()->uid;
     cvd::Response response;
-    response.mutable_shutdown_response();
-
     if (request.Message().has_shutdown_request()) {
       response.mutable_shutdown_response();
     } else {
@@ -130,17 +128,25 @@ class CvdRestartHandler : public CvdServerHandler {
       return response;
     }
 
-    if (instance_manager_.HasInstanceGroups(uid)) {
-      response.mutable_status()->set_code(cvd::Status::FAILED_PRECONDITION);
-      response.mutable_status()->set_message(
-          "Cannot restart cvd_server while devices are being tracked. "
-          "Try `cvd kill-server`.");
-      return response;
-    }
-
+    // On CF_ERR, the locks will be released automatically
     WriteAll(request.Out(), "Stopping the cvd_server.\n");
     server_.Stop();
 
+    CF_EXPECT(request.Credentials() != std::nullopt);
+    const uid_t client_uid = request.Credentials()->uid;
+    auto json_string =
+        CF_EXPECT(SerializedInstanceDatabaseToString(client_uid));
+    std::optional<SharedFD> mem_fd;
+    if (instance_manager_.HasInstanceGroups(client_uid)) {
+      mem_fd = CF_EXPECT(CreateMemFileWithSerializedDb(json_string));
+      CF_EXPECT(mem_fd != std::nullopt && (*mem_fd)->IsOpen(),
+                "mem file not open?");
+    }
+
+    if (parsed.verbose && mem_fd) {
+      PrintFileLink(request.Err(), *mem_fd);
+    }
+
     const std::string subcmd = parsed.subcmd.value_or("reuse-server");
     SharedFD new_exe;
     CF_EXPECT(Contains(supported_modes_, subcmd),
@@ -153,9 +159,14 @@ class CvdRestartHandler : public CvdServerHandler {
     } else if (subcmd == "reuse-server") {
       new_exe = CF_EXPECT(NewExecFromPath(request, kServerExecPath));
     } else {
-      return CF_ERR("Unrecognized command line");
+      return CF_ERR("unsupported subcommand");
     }
-    CF_EXPECT(server_.Exec(new_exe, request.Client()));
+
+    CF_EXPECT(server_.Exec({.new_exe = new_exe,
+                            .carryover_client_fd = request.Client(),
+                            .client_stderr_fd = request.Err(),
+                            .in_memory_data_fd = mem_fd,
+                            .verbose = parsed.verbose}));
     return CF_ERR("Should be unreachable");
   }
 
@@ -227,6 +238,35 @@ class CvdRestartHandler : public CvdServerHandler {
     return new_exe;
   }
 
+  Result<std::string> SerializedInstanceDatabaseToString(
+      const uid_t client_uid) {
+    auto db_json = CF_EXPECT(instance_manager_.Serialize(client_uid),
+                             "Failed to serialized instance database");
+    return db_json.toStyledString();
+  }
+
+  Result<SharedFD> CreateMemFileWithSerializedDb(
+      const std::string& json_string) {
+    const std::string mem_file_name = "cvd_server_" + std::to_string(getpid());
+    auto mem_fd = SharedFD::MemfdCreateWithData(mem_file_name, json_string);
+    CF_EXPECT(mem_fd->IsOpen(),
+              "MemfdCreateWithData failed: " << mem_fd->StrError());
+    return mem_fd;
+  }
+
+  void PrintFileLink(const SharedFD& fd_stream, const SharedFD& mem_fd) const {
+    auto link_target_result = mem_fd->ProcFdLinkTarget();
+    if (!link_target_result.ok()) {
+      WriteAll(fd_stream,
+               "Failed to resolve the link target for the memory file.\n");
+      return;
+    }
+    std::string message("The link target for the memory file is ");
+    message.append(*link_target_result).append("\n");
+    WriteAll(fd_stream, message);
+    return;
+  }
+
   BuildApi& build_api_;
   std::vector<std::string> supported_modes_;
   FlagCollection flags_;
diff --git a/host/commands/cvd/server_command/start.cpp b/host/commands/cvd/server_command/start.cpp
index b530ac420..91c14b076 100644
--- a/host/commands/cvd/server_command/start.cpp
+++ b/host/commands/cvd/server_command/start.cpp
@@ -678,7 +678,6 @@ Result<cvd::Response> CvdStartCommandHandler::Handle(
     LOG(ERROR) << "AcloudCompatActions() failed"
                << " but continue as they are minor errors.";
   }
-  LOG(ERROR) << "Daemon mode is " << (is_daemon ? "set" : "unset");
   return is_daemon ? HandleDaemon(group_creation_info, uid)
                    : HandleNoDaemon(group_creation_info, uid);
 }