Support cvd start --daemon=false

Implements foreground mode for `cvd start`. When `--daemon=false` is requested,
it continues to launch the device via `cvd_internal_start --daemon=true` but
subsequently attaches the log monitor view (`cvd monitor`). Upon user
interruption (Ctrl-C), it gracefully shuts down the instance group.

Assisted-by: Jetski:Gemini-Next
Bug: b/519304405
TAG=agy
CONV=34ddc7b8-a695-4937-be99-b796087dd987

Author: Databean

base/cvd/cuttlefish/host/commands/cvd/cli/commands/BUILD.bazel

@@ -405,6 +405,7 @@ cf_cc_library(
         "//cuttlefish/host/commands/cvd/cli:utils",
         "//cuttlefish/host/commands/cvd/cli/commands:command_handler",
         "//cuttlefish/host/commands/cvd/cli/commands:host_tool_target",
+        "//cuttlefish/host/commands/cvd/cli/commands/monitor",
         "//cuttlefish/host/commands/cvd/cli/selector",
         "//cuttlefish/host/commands/cvd/fetch:substitute",
         "//cuttlefish/host/commands/cvd/instances",

base/cvd/cuttlefish/host/commands/cvd/cli/commands/monitor/monitor.cc

@@ -18,8 +18,9 @@
 
 #include <errno.h>
 #include <fcntl.h>
-#include <poll.h>
+#include <sys/eventfd.h>
 #include <sys/inotify.h>
+#include <sys/poll.h>
 #include <sys/types.h>
 #include <unistd.h>
 
@@ -28,6 +29,7 @@
 #include <memory>
 #include <string>
 #include <thread>
+#include <vector>
 
 #include "absl/strings/str_cat.h"
 
@@ -40,16 +42,8 @@
 #include "cuttlefish/result/result.h"
 
 namespace cuttlefish {
-
 namespace {
 
-void ClearLastNLines(int n) {
-  if (n > 0) {
-    // Move cursor up N lines and clear to end of screen
-    std::cout << AnsiCursorUp(n) << kAnsiClearScreenAfterCursor << std::flush;
-  }
-}
-
 void UpdateFileAndWatch(const SharedFD& inotify_fd, const std::string& path,
                         SharedFD& fd, int& watch) {
   if (!fd->IsOpen()) {
@@ -62,7 +56,17 @@ void UpdateFileAndWatch(const SharedFD& inotify_fd, const std::string& path,
 
 }  // namespace
 
+void ClearLastNLines(int n) {
+  if (n > 0) {
+    // Move cursor up N lines and clear to end of screen
+    std::cout << AnsiCursorUp(n) << kAnsiClearScreenAfterCursor << std::flush;
+  }
+}
 Result<void> MonitorLogs(const LocalInstance& instance) {
+  return MonitorLogs(instance, SharedFD());
+}
+
+Result<void> MonitorLogs(const LocalInstance& instance, SharedFD stop_eventfd) {
   CF_EXPECT(isatty(0), "The monitor command requires an interactive terminal.");
 
   const std::string kernel_log =
@@ -122,19 +126,34 @@ Result<void> MonitorLogs(const LocalInstance& instance) {
     }
     last_draw_time = std::chrono::steady_clock::now();
 
+    // Setup poll_fds
+    std::vector<PollSharedFd> poll_fds = {
+        PollSharedFd{.fd = inotify_fd, .events = POLLIN, .revents = 0},
+    };
+    if (stop_eventfd->IsOpen()) {
+      poll_fds.push_back(
+          PollSharedFd{.fd = stop_eventfd, .events = POLLIN, .revents = 0});
+    }
+
     // Block until file changes occur. If any watch failed or is missing, use
     // a 1-second fallback timeout to awake and retry.
-    // NOLINTNEXTLINE(misc-include-cleaner)
-    PollSharedFd poll_fd = {inotify_fd, POLLIN, 0};
     int timeout_ms = -1;
     if (dir_watch == -1 || kernel_watch == -1 || launcher_watch == -1 ||
         logcat_watch == -1) {
       timeout_ms = 1000;
     }
 
-    // NOLINTNEXTLINE(misc-include-cleaner)
-    if (SharedFD::Poll(&poll_fd, 1, timeout_ms) > 0 &&
-        (poll_fd.revents & POLLIN)) {
+    int poll_res = SharedFD::Poll(poll_fds, timeout_ms);
+
+    if (stop_eventfd->IsOpen() && (poll_fds[1].revents & POLLIN)) {
+      // Stop requested via eventfd
+      eventfd_t val;
+      stop_eventfd->EventfdRead(&val);
+      ClearLastNLines(total_lines_drawn);
+      return {};
+    }
+
+    if (poll_res > 0 && (poll_fds[0].revents & POLLIN)) {
       // Exhaustively drain all available events from the non-blocking
       // descriptor to coalesce rapid file modifications.
       char buf[4096]

base/cvd/cuttlefish/host/commands/cvd/cli/commands/monitor/monitor.h

@@ -16,11 +16,18 @@
 
 #pragma once
 
+#include "cuttlefish/common/libs/fs/shared_fd.h"
 #include "cuttlefish/host/commands/cvd/instances/local_instance.h"
 #include "cuttlefish/result/result.h"
 
 namespace cuttlefish {
 
+void ClearLastNLines(int n);
+
 Result<void> MonitorLogs(const LocalInstance& instance);
 
+// The monitor will stop and return if `stop_eventfd` becomes readable (receives
+// an event).
+Result<void> MonitorLogs(const LocalInstance& instance, SharedFD stop_eventfd);
+
 }  // namespace cuttlefish

base/cvd/cuttlefish/host/commands/cvd/cli/commands/start.cpp

@@ -24,6 +24,8 @@
 #include <unistd.h>
 
 #include <algorithm>
+#include <chrono>
+#include <cstdio>
 #include <cstdlib>
 #include <cstring>
 #include <iostream>
@@ -32,26 +34,29 @@
 #include <optional>
 #include <set>
 #include <string>
+#include <thread>
 #include <utility>
 #include <vector>
 
-#include <fmt/core.h>
-#include <fmt/format.h>
 #include "absl/log/log.h"
 #include "absl/strings/str_join.h"
 #include "absl/strings/str_split.h"
+#include "fmt/core.h"
+#include "fmt/format.h"
 
+#include "cuttlefish/common/libs/fs/shared_buf.h"
 #include "cuttlefish/common/libs/fs/shared_fd.h"
 #include "cuttlefish/common/libs/utils/contains.h"
 #include "cuttlefish/common/libs/utils/files.h"
-#include "cuttlefish/flag_parser/flag.h"
-#include "cuttlefish/flag_parser/gflags_compat.h"
 #include "cuttlefish/common/libs/utils/json.h"
 #include "cuttlefish/common/libs/utils/subprocess.h"
 #include "cuttlefish/common/libs/utils/users.h"
+#include "cuttlefish/flag_parser/flag.h"
+#include "cuttlefish/flag_parser/gflags_compat.h"
 #include "cuttlefish/host/commands/cvd/cli/command_request.h"
 #include "cuttlefish/host/commands/cvd/cli/commands/command_handler.h"
 #include "cuttlefish/host/commands/cvd/cli/commands/host_tool_target.h"
+#include "cuttlefish/host/commands/cvd/cli/commands/monitor/monitor.h"
 #include "cuttlefish/host/commands/cvd/cli/help_format.h"
 #include "cuttlefish/host/commands/cvd/cli/selector/selector.h"
 #include "cuttlefish/host/commands/cvd/cli/types.h"
@@ -279,7 +284,8 @@ Result<Command> ConstructCvdNonHelpCommand(const std::string& bin_file,
                                            const LocalInstanceGroup& group,
                                            const cvd_common::Args& args,
                                            const cvd_common::Envs& envs,
-                                           const CommandRequest& request) {
+                                           const CommandRequest& request,
+                                           SharedFD output_fd = SharedFD()) {
   auto bin_path = group.HostArtifactsPath();
   CF_EXPECTF(PotentiallyHostArtifactsPath(bin_path),
              "ANDROID_HOST_OUT, \"{}\" is not a tool directory", bin_path);
@@ -292,10 +298,17 @@ Result<Command> ConstructCvdNonHelpCommand(const std::string& bin_file,
                                             .working_dir = CurrentDirectory(),
                                             .command_name = bin_file};
   Command non_help_command = CF_EXPECT(ConstructCommand(construct_cmd_param));
-  // Print everything to stderr, cvd needs to print JSON to stdout which
-  // would be unparseable with the subcommand's output.
-  non_help_command.RedirectStdIO(Subprocess::StdIOChannel::kStdOut,
-                                 Subprocess::StdIOChannel::kStdErr);
+  if (output_fd->IsOpen()) {
+    non_help_command.RedirectStdIO(Subprocess::StdIOChannel::kStdOut,
+                                   output_fd);
+    non_help_command.RedirectStdIO(Subprocess::StdIOChannel::kStdErr,
+                                   output_fd);
+  } else {
+    // Print everything to stderr, cvd needs to print JSON to stdout which
+    // would be unparseable with the subcommand's output.
+    non_help_command.RedirectStdIO(Subprocess::StdIOChannel::kStdOut,
+                                   Subprocess::StdIOChannel::kStdErr);
+  }
   return non_help_command;
 }
 
@@ -317,18 +330,6 @@ CvdStartCommandHandler::CvdStartCommandHandler(
     InstanceManager& instance_manager)
     : instance_manager_(instance_manager) {}
 
-static Result<void> ConsumeDaemonModeFlag(cvd_common::Args& args) {
-  bool daemon = true;
-  Flag flag =
-      GflagsCompatFlag("daemon", daemon)
-          .AddValidator([&daemon]() -> Result<void> {
-            CF_EXPECT(!!daemon, "`cvd start` must always run in daemon mode.");
-            return {};
-          });
-  CF_EXPECT(ConsumeFlags({flag}, args));
-  return {};
-}
-
 static bool HasUnsafeFlagsForBypass(const std::vector<std::string>& args) {
   std::vector<std::string> args_copy = args;
   bool daemon = true;
@@ -370,8 +371,18 @@ Result<void> CvdStartCommandHandler::Handle(const CommandRequest& request) {
     }
   }
 
-  CF_EXPECT(ConsumeDaemonModeFlag(subcmd_args));
+  CF_EXPECT(ConsumeFlags(BuildOwnFlags(), subcmd_args));
+  bool is_daemon = own_flags_.daemon.value_or(true);
   subcmd_args.push_back("--daemon=true");
+  if (own_flags_.report_anonymous_usage_stats) {
+    subcmd_args.push_back("--report_anonymous_usage_stats=" +
+                          *own_flags_.report_anonymous_usage_stats);
+  } else if (!is_daemon) {
+    // If the user did not specify the anonymous usage stats flag, default it
+    // to 'n' to ensure that cvd_internal_start does not block waiting for
+    // input on stdin.
+    subcmd_args.push_back("--report_anonymous_usage_stats=n");
+  }
 
   LocalInstanceGroup group =
       CF_EXPECT(selector::SelectGroup(instance_manager_, request),
@@ -388,46 +399,109 @@ Result<void> CvdStartCommandHandler::Handle(const CommandRequest& request) {
   CF_EXPECT(UpdateEnvs(envs, group));
   const auto bin = CF_EXPECT(FindStartBin(group.HostArtifactsPath()));
 
-  CF_EXPECT(ConsumeFlags(BuildOwnFlags(), subcmd_args));
-
   CF_EXPECT(HostPackageSubstitution(group.HostArtifactsPath(),
                                     own_flags_.host_substitutions));
 
-  Command command = CF_EXPECT(
-      ConstructCvdNonHelpCommand(bin, group, subcmd_args, envs, request));
-
-  // The instance database needs to be updated if an interrupt is received.
-  auto handle_res = PushInterruptListener([this, &group](int signal) {
-    LOG(WARNING) << strsignal(signal) << " signal received, cleanning up";
-    auto interrupt_res = subprocess_waiter_.Interrupt();
-    if (!interrupt_res.ok()) {
-      LOG(ERROR) << "Failed to stop subprocesses: " << interrupt_res.error();
-      LOG(ERROR) << "Devices may still be executing in the background, run "
-                    "`cvd reset` to ensure a clean state";
-    }
-
-    group.SetAllStates(cvd::INSTANCE_STATE_CANCELLED);
-    auto update_res = instance_manager_.UpdateInstanceGroup(group);
-    if (!update_res.ok()) {
-      LOG(ERROR) << "Failed to update group status: " << update_res.error();
-    }
-    // It's technically possible for the group's state to be set to
-    // "running" before abort has a chance to run, but that can only happen
-    // if the instances are indeed running, so it's OK.
+  SharedFD memfd;
+  SharedFD stop_eventfd;
+  if (!is_daemon) {
+    memfd = SharedFD::MemfdCreate("cvd_internal_start_output");
+    CF_EXPECT(memfd->IsOpen(), "Failed to create memfd for subprocess output: "
+                                   << memfd->StrError());
+
+    stop_eventfd = SharedFD::Event();
+    CF_EXPECT(stop_eventfd->IsOpen(),
+              "Failed to create eventfd for stopping monitor: "
+                  << stop_eventfd->StrError());
+  }
 
-    std::abort();
-  });
-  auto listener_handle = CF_EXPECT(std::move(handle_res));
+  Command command = CF_EXPECT(ConstructCvdNonHelpCommand(
+      bin, group, subcmd_args, envs, request, memfd));
+
+  Result<std::unique_ptr<InterruptListenerHandle>> handle_res =
+      PushInterruptListener([this, &group, stop_eventfd](int signal) {
+        LOG(WARNING) << strsignal(signal) << " signal received, cleanning up";
+        if (stop_eventfd->IsOpen()) {
+          stop_eventfd->EventfdWrite(1);
+        }
+        Result<void> interrupt_res = subprocess_waiter_.Interrupt();
+        if (!interrupt_res.ok()) {
+          LOG(ERROR) << "Failed to stop subprocesses: "
+                     << interrupt_res.error();
+          LOG(ERROR) << "Devices may still be executing in the background, "
+                        "run `cvd reset` to ensure a clean state";
+        }
+
+        group.SetAllStates(cvd::INSTANCE_STATE_CANCELLED);
+        Result<void> update_res = instance_manager_.UpdateInstanceGroup(group);
+        if (!update_res.ok()) {
+          LOG(ERROR) << "Failed to update group status: " << update_res.error();
+        }
+        // It's technically possible for the group's state to be set to
+        // "running" before abort has a chance to run, but that can only happen
+        // if the instances are indeed running, so it's OK.
+
+        std::abort();
+      });
+
+  std::unique_ptr<InterruptListenerHandle> listener_handle =
+      CF_EXPECT(std::move(handle_res));
   group.SetAllStates(cvd::INSTANCE_STATE_STARTING);
   group.SetStartTime(CvdServerClock::now());
   CF_EXPECT(instance_manager_.UpdateInstanceGroup(group));
-  CF_EXPECT(
-      LaunchDeviceInterruptible(std::move(command), group, envs, request));
+
+  std::thread monitor_thread;
+  Result<void> monitor_res;
+
+  if (!is_daemon) {
+    monitor_thread = std::thread([&group, stop_eventfd, &monitor_res]() {
+      const LocalInstance& first_instance = *group.Instances().begin();
+      monitor_res = MonitorLogs(first_instance, stop_eventfd);
+    });
+  }
+
+  Result<void> launch_res =
+      LaunchDeviceInterruptible(std::move(command), group, envs, request);
+
+  if (!launch_res.ok()) {
+    if (!is_daemon) {
+      if (stop_eventfd->IsOpen()) {
+        stop_eventfd->EventfdWrite(1);
+      }
+      if (monitor_thread.joinable()) {
+        monitor_thread.join();
+      }
+      memfd->LSeek(0, SEEK_SET);
+      std::string full_output;
+      ReadAll(memfd, &full_output);
+      std::cout << full_output << std::flush;
+    }
+    return launch_res;
+  }
+
   group.SetAllStates(cvd::INSTANCE_STATE_RUNNING);
   CF_EXPECT(instance_manager_.UpdateInstanceGroup(group));
+
+  if (!is_daemon) {
+    listener_handle.reset();
+    std::unique_ptr<InterruptListenerHandle> stop_listener =
+        CF_EXPECT(PushInterruptListener(
+            [stop_eventfd](int) { stop_eventfd->EventfdWrite(1); }));
+
+    if (monitor_thread.joinable()) {
+      monitor_thread.join();
+    }
+    stop_listener.reset();
+
+    LOG(INFO) << "Stopping device...";
+    CF_EXPECT(instance_manager_.StopInstanceGroup(
+        group, std::chrono::seconds(5), InstanceDirActionOnStop::Keep, {}));
+    LOG(INFO) << "Device stopped.";
+    return monitor_res;
+  }
+
   listener_handle.reset();
 
-  // show user a more succinct output
   if (isatty(0)) {
     std::vector<std::string> instance_names;
     for (const auto& instance : group.Instances()) {
@@ -609,7 +683,15 @@ std::vector<Flag> CvdStartCommandHandler::BuildOwnFlags() {
                     "cuttlefish-base package. The special value \"all\" causes "
                     "it to replace everything it can, while with an empty it "
                     "will replace what the android build specifies in the "
-                    "'debian_substitution_marker' file.")};
+                    "'debian_substitution_marker' file."),
+          GflagsCompatFlag("daemon", own_flags_.daemon)
+              .Help("Run the start command in the background as a daemon. "
+                    "If false, the command runs in the foreground and monitors "
+                    "logs."),
+          GflagsCompatFlag("report_anonymous_usage_stats",
+                           own_flags_.report_anonymous_usage_stats)
+              .Help("Report anonymous usage stats. In foreground mode, "
+                    "defaults to 'n' if not specified.")};
 }
 
 std::unique_ptr<CvdCommandHandler> NewCvdStartCommandHandler(