Merge "Create restore file to track if restoring" into main

9 files changed, 38 insertions, 18 deletions

diff --git a/host/commands/assemble_cvd/assemble_cvd.cc b/host/commands/assemble_cvd/assemble_cvd.cc
index 4bc558818..5f1f3fd69 100644
--- a/host/commands/assemble_cvd/assemble_cvd.cc
+++ b/host/commands/assemble_cvd/assemble_cvd.cc
@@ -341,6 +341,12 @@ Result<const CuttlefishConfig*> InitFilesystemAndCreateConfig(
                                     default_group));
     CF_EXPECT(EnsureDirectoryExists(config.environments_uds_dir(), default_mode,
                                     default_group));
+    if (!snapshot_path.empty()) {
+      SharedFD temp = SharedFD::Creat(config.AssemblyPath("restore"), 0660);
+      if (!temp->IsOpen()) {
+        return CF_ERR("Failed to create restore file: " << temp->StrError());
+      }
+    }
 
     auto environment =
         const_cast<const CuttlefishConfig&>(config).ForDefaultEnvironment();
diff --git a/host/commands/assemble_cvd/flags.cc b/host/commands/assemble_cvd/flags.cc
index b6e42dd59..2e34ffd37 100644
--- a/host/commands/assemble_cvd/flags.cc
+++ b/host/commands/assemble_cvd/flags.cc
@@ -957,9 +957,6 @@ Result<CuttlefishConfig> InitializeCuttlefishConfiguration(
 
   tmp_config_obj.set_gem5_debug_flags(FLAGS_gem5_debug_flags);
 
-  // setting snapshot path
-  tmp_config_obj.set_snapshot_path(FLAGS_snapshot_path);
-
   // streaming, webrtc setup
   tmp_config_obj.set_webrtc_certs_dir(FLAGS_webrtc_certs_dir);
   tmp_config_obj.set_sig_server_secure(FLAGS_webrtc_sig_server_secure);
diff --git a/host/commands/logcat_receiver/main.cpp b/host/commands/logcat_receiver/main.cpp
index c562d50c6..d7c9dbc94 100644
--- a/host/commands/logcat_receiver/main.cpp
+++ b/host/commands/logcat_receiver/main.cpp
@@ -79,7 +79,7 @@ int main(int argc, char** argv) {
         << ". This is unrecoverable.";
     if (first_iter) {
       first_iter = false;
-      if ((!config->snapshot_path().empty())) {
+      if (cuttlefish::IsRestoring(*config)) {
         cuttlefish::SharedFD restore_pipe = cuttlefish::SharedFD::Open(
             instance.restore_pipe_name().c_str(), O_WRONLY);
         if (!restore_pipe->IsOpen()) {
diff --git a/host/commands/run_cvd/boot_state_machine.cc b/host/commands/run_cvd/boot_state_machine.cc
index 0e8c378e1..02a9ce842 100644
--- a/host/commands/run_cvd/boot_state_machine.cc
+++ b/host/commands/run_cvd/boot_state_machine.cc
@@ -45,7 +45,7 @@ SharedFD DaemonizeLauncher(const CuttlefishConfig& config) {
   auto instance = config.ForDefaultInstance();
   auto restore_pipe_name = instance.restore_pipe_name();
   SharedFD read_end, write_end, restore_pipe_read;
-  if (!config.snapshot_path().empty()) {
+  if (IsRestoring(config)) {
     if (Result<SharedFD> restore_pipe = SharedFD::Fifo(restore_pipe_name, 0600);
         !restore_pipe.ok()) {
       LOG(ERROR) << "Unable to create restore fifo"
@@ -65,7 +65,7 @@ SharedFD DaemonizeLauncher(const CuttlefishConfig& config) {
     // child process dies.
     write_end->Close();
     RunnerExitCodes exit_code;
-    if (!config.snapshot_path().empty()) {
+    if (IsRestoring(config)) {
       if (!restore_pipe_read->IsOpen()) {
         LOG(ERROR) << "Error opening restore pipe: "
                    << restore_pipe_read->StrError();
@@ -144,14 +144,9 @@ Result<SharedFD> ProcessLeader(
     const CuttlefishConfig& config,
     const CuttlefishConfig::InstanceSpecific& instance,
     AutoSetup<ValidateTapDevices>::Type& /* dependency */) {
-  if (!config.snapshot_path().empty()) {
-    if (Result<SharedFD> restore_adbd_pipe =
-            SharedFD::Fifo(instance.restore_adbd_pipe_name(), 0600);
-        !restore_adbd_pipe.ok()) {
-      LOG(ERROR) << "Unable to create adbd restore fifo"
-                 << restore_adbd_pipe.error().FormatForEnv();
-      return {};
-    }
+  if (IsRestoring(config)) {
+    CF_EXPECT(SharedFD::Fifo(instance.restore_adbd_pipe_name(), 0600),
+              "Unable to create adbd restore fifo");
   }
   /* These two paths result in pretty different process state, but both
    * achieve the same goal of making the current process the leader of a
diff --git a/host/commands/run_cvd/launch/open_wrt.cpp b/host/commands/run_cvd/launch/open_wrt.cpp
index 2590b3714..0ca4a9cc6 100644
--- a/host/commands/run_cvd/launch/open_wrt.cpp
+++ b/host/commands/run_cvd/launch/open_wrt.cpp
@@ -86,8 +86,8 @@ class OpenWrt : public CommandSource {
       wifi_tap = ap_cmd.AddTap(instance_.wifi_tap_name());
     }
 
-    const std::string snapshot_dir = config_.snapshot_path();
-    if (!snapshot_dir.empty()) {
+    if (IsRestoring(config_)) {
+      const std::string snapshot_dir = config_.snapshot_path();
       CF_EXPECT(ap_cmd.SetToRestoreFromSnapshot(snapshot_dir, instance_.id(),
                                                 "_openwrt"));
     }
diff --git a/host/commands/run_cvd/server_loop_impl.cpp b/host/commands/run_cvd/server_loop_impl.cpp
index 9cd677cea..2cd6240a2 100644
--- a/host/commands/run_cvd/server_loop_impl.cpp
+++ b/host/commands/run_cvd/server_loop_impl.cpp
@@ -370,6 +370,13 @@ bool ServerLoopImpl::PowerwashFiles() {
 }
 
 void ServerLoopImpl::RestartRunCvd(int notification_fd) {
+  // On device creation, if the file "restore" exists, a restore of the device
+  // occurs. This means a restart will instead perform a restore, which is
+  // undesired behavior. Always try to delete the file "restore" if a restart is
+  // requested.
+  if (IsRestoring(config_)) {
+    CHECK(RemoveFile(config_.AssemblyPath("restore")));
+  }
   auto config_path = config_.AssemblyPath("cuttlefish_config.json");
   auto followup_stdin = SharedFD::MemfdCreate("pseudo_stdin");
   WriteAll(followup_stdin, config_path + "\n");
diff --git a/host/libs/config/cuttlefish_config.cpp b/host/libs/config/cuttlefish_config.cpp
index 2bb66aa4b..fbd6e5a94 100644
--- a/host/libs/config/cuttlefish_config.cpp
+++ b/host/libs/config/cuttlefish_config.cpp
@@ -73,6 +73,9 @@ std::string DefaultEnvironmentPath(const char* environment_key,
   return StringFromEnv(environment_key, default_value) + "/" + subpath;
 }
 
+bool IsRestoring(const CuttlefishConfig& config) {
+  return FileExists(config.AssemblyPath("restore"));
+}
 ConfigFragment::~ConfigFragment() = default;
 
 static constexpr char kFragments[] = "fragments";
diff --git a/host/libs/config/cuttlefish_config.h b/host/libs/config/cuttlefish_config.h
index 23c3b12f6..17320c18a 100644
--- a/host/libs/config/cuttlefish_config.h
+++ b/host/libs/config/cuttlefish_config.h
@@ -951,6 +951,18 @@ class CuttlefishConfig {
   CuttlefishConfig& operator=(const CuttlefishConfig&) = delete;
 };
 
+// Whether the instance is restored from a snapshot. Stays true until the device
+// reboots.
+// When the device is booting, the config  init function checks if
+// "FLAGS_snapshot_path" is not empty, and if it isn't empty, a file called
+// "restore" will be created to keep track of the restore.
+// This is necessary because we don't want to
+// modify the config when the device boots, however we also want to only restore
+// once. Tracking via "restore" is necessary as a bug existed when checking if
+// "snapshot_path" existed during boot, where a restart or a powerwash of the
+// device would actually perform a restore instead of their respective actions.
+bool IsRestoring(const CuttlefishConfig&);
+
 // Vhost-user-vsock modes
 extern const char* const kVhostUserVsockModeAuto;
 extern const char* const kVhostUserVsockModeTrue;
diff --git a/host/libs/vm_manager/crosvm_manager.cpp b/host/libs/vm_manager/crosvm_manager.cpp
index 10097d396..0b2cd993a 100644
--- a/host/libs/vm_manager/crosvm_manager.cpp
+++ b/host/libs/vm_manager/crosvm_manager.cpp
@@ -413,8 +413,8 @@ Result<std::vector<MonitorCommand>> CrosvmManager::StartCommands(
   // the first invocation. If the guest requests a restart, we don't want crosvm
   // to restore again. It should reboot normally.
   std::string first_time_argument;
-  const std::string snapshot_dir_path = config.snapshot_path();
-  if (!snapshot_dir_path.empty()) {
+  if (IsRestoring(config)) {
+    const std::string snapshot_dir_path = config.snapshot_path();
     auto meta_info_json = CF_EXPECT(LoadMetaJson(snapshot_dir_path));
     const std::vector<std::string> selectors{kGuestSnapshotField,
                                              instance.id()};