diff options
author | Frederick Mayle <fmayle@google.com> | 2024-04-15 17:17:14 -0700 |
---|---|---|
committer | Frederick Mayle <fmayle@google.com> | 2024-04-15 17:51:36 -0700 |
commit | 2d738def6e16486053cf2602832e1ceabaf6af42 (patch) | |
tree | b2bd4573eca1d3019d44453ae149c5bb5a84f1ce | |
parent | 2776ccfa6c9b9a045d1e9389cd439ed7f8cd583c (diff) | |
download | cuttlefish-2d738def6e16486053cf2602832e1ceabaf6af42.tar.gz |
fix adb proxy when powerwash is used on a restored device
`IsRestoring` was recently introduced to detect if the new device is for
a restore. If the device is reboot'd or powerwash'd, it correctly
returns `false`. However, the adb vsock proxy was still using its own
config option, `sock_vsock_proxy_wait_adbd_start`, that continues
returning `true` even after powerwash and so the adb proxy was
incorrectly waiting for a signal from some restore-only code. (this
isn't an issue for `adb reboot` because it doesn't restart the vsock
proxy process)
To fix, this commit deletes `sock_vsock_proxy_wait_adbd_start` and
instead uses `IsRestoring` directly.
Bug: 331741524
Test: m && (stop_cvd; launch_cvd --noresume --enable-sandbox=false --daemon --secure_hals=keymint,oemlock,guest_keymint_insecure && snapshot_util_cvd --subcmd=snapshot_take --auto_suspend --force --snapshot_path=$HOME/cf-snapshot && stop_cvd && launch_cvd --snapshot_path=$HOME/cf-snapshot) && powerwash_cvd
Change-Id: I493741948c2a3e87d41551820aa808c33d6cafc3
-rw-r--r-- | host/commands/assemble_cvd/flags.cc | 11 | ||||
-rw-r--r-- | host/libs/config/adb/adb.h | 1 | ||||
-rw-r--r-- | host/libs/config/adb/launch.cpp | 13 | ||||
-rw-r--r-- | host/libs/config/cuttlefish_config.h | 9 | ||||
-rw-r--r-- | host/libs/config/cuttlefish_config_instance.cpp | 11 |
5 files changed, 9 insertions, 36 deletions
diff --git a/host/commands/assemble_cvd/flags.cc b/host/commands/assemble_cvd/flags.cc index 476830515..c1e8b8767 100644 --- a/host/commands/assemble_cvd/flags.cc +++ b/host/commands/assemble_cvd/flags.cc @@ -914,13 +914,6 @@ Result<CuttlefishConfig> InitializeCuttlefishConfiguration( snapshot_path + "/assembly/cuttlefish_config.json"; tmp_config_obj.LoadFromFile(snapshot_path_config.c_str()); tmp_config_obj.set_snapshot_path(snapshot_path); - auto instance_nums = - CF_EXPECT(InstanceNumsCalculator().FromGlobalGflags().Calculate()); - - for (const auto& num : instance_nums) { - auto instance = tmp_config_obj.ForInstance(num); - instance.set_sock_vsock_proxy_wait_adbd_start(false); - } return tmp_config_obj; } @@ -1168,8 +1161,6 @@ Result<CuttlefishConfig> InitializeCuttlefishConfiguration( CF_EXPECT(GET_FLAG_BOOL_VALUE(crosvm_use_rng)); std::vector<bool> use_pmem_vec = CF_EXPECT(GET_FLAG_BOOL_VALUE(use_pmem)); const bool restore_from_snapshot = !std::string(FLAGS_snapshot_path).empty(); - std::vector<bool> sock_vsock_proxy_wait_adbd_vec(instance_nums.size(), - !restore_from_snapshot); std::vector<std::string> device_external_network_vec = CF_EXPECT(GET_FLAG_STR_VALUE(device_external_network)); @@ -1305,8 +1296,6 @@ Result<CuttlefishConfig> InitializeCuttlefishConfiguration( instance.set_crosvm_use_balloon(use_balloon_vec[instance_index]); instance.set_crosvm_use_rng(use_rng_vec[instance_index]); instance.set_use_pmem(use_pmem_vec[instance_index]); - instance.set_sock_vsock_proxy_wait_adbd_start( - sock_vsock_proxy_wait_adbd_vec[instance_index]); instance.set_bootconfig_supported(guest_configs[instance_index].bootconfig_supported); instance.set_filename_encryption_mode( guest_configs[instance_index].hctr2_supported ? "hctr2" : "cts"); diff --git a/host/libs/config/adb/adb.h b/host/libs/config/adb/adb.h index 08b15496d..b0566395f 100644 --- a/host/libs/config/adb/adb.h +++ b/host/libs/config/adb/adb.h @@ -57,6 +57,7 @@ AdbConfigFlagComponent(); fruit::Component<fruit::Required<AdbConfig>, AdbConfigFragment> AdbConfigFragmentComponent(); fruit::Component<fruit::Required<KernelLogPipeProvider, const AdbConfig, + const CuttlefishConfig, const CuttlefishConfig::InstanceSpecific>> LaunchAdbComponent(); diff --git a/host/libs/config/adb/launch.cpp b/host/libs/config/adb/launch.cpp index c5343af63..e5d78dc3f 100644 --- a/host/libs/config/adb/launch.cpp +++ b/host/libs/config/adb/launch.cpp @@ -121,9 +121,11 @@ class AdbConnector : public CommandSource { class SocketVsockProxy : public CommandSource, public KernelLogPipeConsumer { public: INJECT(SocketVsockProxy(const AdbHelper& helper, + const CuttlefishConfig& cuttlefish_config, const CuttlefishConfig::InstanceSpecific& instance, KernelLogPipeProvider& log_pipe_provider)) : helper_(helper), + cuttlefish_config_(cuttlefish_config), instance_(instance), log_pipe_provider_(log_pipe_provider) {} @@ -145,11 +147,10 @@ class SocketVsockProxy : public CommandSource, public KernelLogPipeConsumer { adb_tunnel.AddParameter("--start_event_id=", monitor::Event::AdbdStarted); adb_tunnel.AddParameter("--stop_event_id=", monitor::Event::FastbootStarted); - /* fmayle@ found out that when cuttlefish starts from the saved snapshot - * that was saved after ADBD start event, the socket_vsock_proxy must not - * wait for the AdbdStarted event. - */ - if (!instance_.sock_vsock_proxy_wait_adbd_start()) { + // We assume that snapshots are always taken after ADBD has started. That + // means the start event will never come for a restored device, so we pass + // a flag to the proxy to allow it to alter its behavior. + if (IsRestoring(cuttlefish_config_)) { adb_tunnel.AddParameter("--restore=true"); } adb_tunnel.AddParameter("--server_type=tcp"); @@ -210,6 +211,7 @@ class SocketVsockProxy : public CommandSource, public KernelLogPipeConsumer { } const AdbHelper& helper_; + const CuttlefishConfig& cuttlefish_config_; const CuttlefishConfig::InstanceSpecific& instance_; KernelLogPipeProvider& log_pipe_provider_; SharedFD kernel_log_pipe_; @@ -218,6 +220,7 @@ class SocketVsockProxy : public CommandSource, public KernelLogPipeConsumer { } // namespace fruit::Component<fruit::Required<KernelLogPipeProvider, const AdbConfig, + const CuttlefishConfig, const CuttlefishConfig::InstanceSpecific>> LaunchAdbComponent() { return fruit::createComponent() diff --git a/host/libs/config/cuttlefish_config.h b/host/libs/config/cuttlefish_config.h index b7d95064c..d63a9e3fe 100644 --- a/host/libs/config/cuttlefish_config.h +++ b/host/libs/config/cuttlefish_config.h @@ -500,14 +500,6 @@ class CuttlefishConfig { bool crosvm_use_balloon() const; bool crosvm_use_rng() const; bool use_pmem() const; - /* fmayle@ found out that when cuttlefish starts from the saved snapshot - * that was saved after ADBD start event, the socket_vsock_proxy must not - * wait for the AdbdStarted event. - * - * This instance-specific configuration tells the host sock_vsock_proxy - * not to wait for the adbd start event. - */ - bool sock_vsock_proxy_wait_adbd_start() const; // Wifi MAC address inside the guest int wifi_mac_prefix() const; @@ -736,7 +728,6 @@ class CuttlefishConfig { void set_crosvm_use_balloon(const bool use_balloon); void set_crosvm_use_rng(const bool use_rng); void set_use_pmem(const bool use_pmem); - void set_sock_vsock_proxy_wait_adbd_start(const bool); // Wifi MAC address inside the guest void set_wifi_mac_prefix(const int wifi_mac_prefix); // Gnss grpc proxy server port inside the host diff --git a/host/libs/config/cuttlefish_config_instance.cpp b/host/libs/config/cuttlefish_config_instance.cpp index 023cc9830..335c0621a 100644 --- a/host/libs/config/cuttlefish_config_instance.cpp +++ b/host/libs/config/cuttlefish_config_instance.cpp @@ -1695,17 +1695,6 @@ bool CuttlefishConfig::InstanceSpecific::use_pmem() const { return (*Dictionary())[kCrosvmUsePmem].asBool(); } -static constexpr char kSockVsockWaitAdbdStart[] = - "sock_vsock_proxy_wait_adbd_start"; -void CuttlefishConfig::MutableInstanceSpecific:: - set_sock_vsock_proxy_wait_adbd_start(const bool wait_adbd_start) { - (*Dictionary())[kSockVsockWaitAdbdStart] = wait_adbd_start; -} -bool CuttlefishConfig::InstanceSpecific::sock_vsock_proxy_wait_adbd_start() - const { - return (*Dictionary())[kSockVsockWaitAdbdStart].asBool(); -} - std::string CuttlefishConfig::InstanceSpecific::touch_socket_path( int touch_dev_idx) const { return PerInstanceInternalUdsPath( |