From 7de21170370914419c3df47f412a89a7dac550d8 Mon Sep 17 00:00:00 2001 From: Aleksandr Shirvinskii Date: Tue, 10 Nov 2026 09:33:38 +0000 Subject: [PATCH 1/2] cvd: Fix single-instance start after stop in multi-instance groups Allow `--report_anonymous_usage_stats` and `INSTANCE_STATE_BOOT_FAILED` in `CanBypassToSingleInstance` for backwards compatibility and boot-failure retries. Update `LaunchSingleInstance` to run `run_cvd` with the instance directory as its working directory (matching `cvd_internal_start` so `secure_env` stores TPM/KeyMint state in the instance directory rather than failing in a read-only daemon CWD), and parameterize `LaunchDevice` with `instances` so `LaunchSingleInstance` pre-registers only the target instance with the operator and stops only the target instance (with a 5s launcher timeout) on boot failure. Also exclude shared `assembly_dir()` from `stop_cvd`'s fallback stop when stopping a subset of instances so running sibling instances are not disrupted. Bug: 581680503 Test: bazel test //cuttlefish/host/commands/cvd/... //cuttlefish/host/commands/stop/... Assisted-by: Jetski:Gemini --- .../host/commands/cvd/cli/commands/start.cpp | 66 ++++++++++++------- .../host/commands/cvd/cli/commands/start.h | 8 +-- .../cvd/instances/operator_client.cpp | 29 +++++--- .../commands/cvd/instances/operator_client.h | 8 +++ .../cvd/cuttlefish/host/commands/stop/main.cc | 47 +++++++++---- 5 files changed, 111 insertions(+), 47 deletions(-) diff --git a/base/cvd/cuttlefish/host/commands/cvd/cli/commands/start.cpp b/base/cvd/cuttlefish/host/commands/cvd/cli/commands/start.cpp index bd1b7a82e7a..4aea9fc938b 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/cli/commands/start.cpp +++ b/base/cvd/cuttlefish/host/commands/cvd/cli/commands/start.cpp @@ -24,6 +24,7 @@ #include #include +#include #include #include #include @@ -68,6 +69,7 @@ #include "cuttlefish/host/commands/cvd/instances/cvd_persistent_data.pb.h" #include "cuttlefish/host/commands/cvd/instances/instance_database_types.h" #include "cuttlefish/host/commands/cvd/instances/instance_manager.h" +#include "cuttlefish/host/commands/cvd/instances/local_instance.h" #include "cuttlefish/host/commands/cvd/instances/local_instance_group.h" #include "cuttlefish/host/commands/cvd/instances/operator_client.h" #include "cuttlefish/host/commands/cvd/instances/stop.h" @@ -248,11 +250,12 @@ Result SymlinkPreviousConfig(const std::string& group_home_dir) { return {}; } -Result> PreregisterGroup( - const LocalInstanceGroup& group) { +Result> PreregisterInstances( + const LocalInstanceGroup& group, + const std::vector& instances) { std::unique_ptr operator_conn = CF_EXPECT(OperatorControlConn::Create()); - CF_EXPECT(operator_conn->Preregister(group)); + CF_EXPECT(operator_conn->Preregister(group, instances)); return operator_conn; } @@ -334,7 +337,8 @@ Result> GetCvdInternalStartFlags( bool CanBypassToSingleInstance(const LocalInstance& instance, const LocalInstanceGroup& group, const std::vector& subcmd_args) { - if (instance.State() != cvd::INSTANCE_STATE_STOPPED) { + if (instance.State() != cvd::INSTANCE_STATE_STOPPED && + instance.State() != cvd::INSTANCE_STATE_BOOT_FAILED) { return false; } if (group.StartTime() == TimeStamp{}) { @@ -343,8 +347,11 @@ bool CanBypassToSingleInstance(const LocalInstance& instance, std::vector args_copy = subcmd_args; bool daemon = true; + std::string report_anonymous_usage_stats; std::vector safe_flags = { GflagsCompatFlag("daemon", daemon), + GflagsCompatFlag("report_anonymous_usage_stats", + report_anonymous_usage_stats), }; const Result res = ConsumeFlags(safe_flags, args_copy); if (!res.has_value() || !daemon || !args_copy.empty()) { @@ -552,14 +559,14 @@ std::vector CvdStartCommandHandler::CmdList() const { Result CvdStartCommandHandler::LaunchDevice( Command launch_command, LocalInstanceGroup& group, - const std::unordered_map& envs, - const CommandRequest& request) { + const std::vector& instances) { // Don't destroy the returned object until after the devices have started, it // holds a connection to the orchestrator that ensures the devices remain // pre-registered there. If the connection is lost before the devices register // themselves the pre-registration is lost and group information won't be // shown in the UI. - auto conn_res = PreregisterGroup(group); + const Result> conn_res = + PreregisterInstances(group, instances); if (!conn_res.has_value()) { LOG(ERROR) << "Failed to pre-register devices with operator, group " "information won't show in the UI: " @@ -572,14 +579,27 @@ Result CvdStartCommandHandler::LaunchDevice( GatherVmStartMetrics(group); // NOLINTNEXTLINE(misc-include-cleaner) - siginfo_t infop = CF_EXPECT(subprocess_waiter_.Wait()); + const siginfo_t infop = CF_EXPECT(subprocess_waiter_.Wait()); // NOLINTNEXTLINE(misc-include-cleaner) if (infop.si_code != CLD_EXITED || infop.si_status != EXIT_SUCCESS) { LOG(INFO) << "Device launch failed, cleaning up"; GatherVmBootFailedMetrics(group); - // run_cvd processes may be still running in background - // the order of the following operations should be kept - CF_EXPECT(CvdResetGroup(group)); + if (instances.size() < group.Instances().size()) { + std::vector ids; + ids.reserve(instances.size()); + for (const LocalInstance& instance : instances) { + ids.push_back(instance.Id()); + } + const Result stop_res = instance_manager_.StopInstanceGroup( + group, InstanceDirActionOnStop::Keep, std::chrono::seconds(5), ids); + if (!stop_res.has_value()) { + LOG(ERROR) << "Failed to stop failed instance(s): " << stop_res.error(); + } + } else { + // run_cvd processes may be still running in background + // the order of the following operations should be kept + CF_EXPECT(CvdResetGroup(group)); + } } CF_EXPECT(CheckProcessExitedNormally(infop)); GatherVmBootCompleteMetrics(group); @@ -601,8 +621,8 @@ Result CvdStartCommandHandler::LaunchDeviceInterruptible( LOG(ERROR) << "Failed to symlink the config file at system wide home: " << symlink_config_res.error(); } - Result start_res = - LaunchDevice(std::move(command), group, envs, request); + const Result start_res = + LaunchDevice(std::move(command), group, group.Instances()); if (!start_res.has_value()) { group.SetAllStates(cvd::INSTANCE_STATE_BOOT_FAILED); CF_EXPECT(instance_manager_.UpdateInstanceGroup(group)); @@ -624,12 +644,13 @@ Result CvdStartCommandHandler::LaunchSingleInstance( run_cvd_envs[kAndroidSoongHostOut] = group.HostArtifactsPath(); run_cvd_envs[kCvdMarkEnv] = "true"; - ConstructCommandParam construct_cmd_param{.bin_path = bin_path, - .home = group.HomeDir(), - .args = std::vector{}, - .envs = run_cvd_envs, - .working_dir = CurrentDirectory(), - .command_name = "run_cvd"}; + const ConstructCommandParam construct_cmd_param{ + .bin_path = bin_path, + .home = group.HomeDir(), + .args = std::vector{}, + .envs = run_cvd_envs, + .working_dir = instance.InstanceDirectory(), + .command_name = "run_cvd"}; Command command = CF_EXPECT(ConstructCommand(construct_cmd_param)); command.RedirectStdIO(Command::StdIoChannel::kStdOut, @@ -649,7 +670,7 @@ Result CvdStartCommandHandler::LaunchSingleInstance( } auto set_instance_state = [&group, &instance](cvd::InstanceState state) { - for (auto& inst : group.Instances()) { + for (LocalInstance& inst : group.Instances()) { if (inst.Id() == instance.Id()) { inst.SetState(state); break; @@ -661,9 +682,8 @@ Result CvdStartCommandHandler::LaunchSingleInstance( group.SetStartTime(CvdServerClock::now()); CF_EXPECT(instance_manager_.UpdateInstanceGroup(group)); - Result start_res = - LaunchDevice(std::move(command), group, run_cvd_envs, request); - + const Result start_res = + LaunchDevice(std::move(command), group, {instance}); if (!start_res.has_value()) { set_instance_state(cvd::INSTANCE_STATE_BOOT_FAILED); CF_EXPECT(instance_manager_.UpdateInstanceGroup(group)); diff --git a/base/cvd/cuttlefish/host/commands/cvd/cli/commands/start.h b/base/cvd/cuttlefish/host/commands/cvd/cli/commands/start.h index 8c71cd6cc5a..b8f2b229aa0 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/cli/commands/start.h +++ b/base/cvd/cuttlefish/host/commands/cvd/cli/commands/start.h @@ -18,10 +18,12 @@ #include #include +#include #include "cuttlefish/flag_parser/flag.h" #include "cuttlefish/host/commands/cvd/cli/commands/command_handler.h" #include "cuttlefish/host/commands/cvd/instances/instance_manager.h" +#include "cuttlefish/host/commands/cvd/instances/local_instance.h" #include "cuttlefish/host/commands/cvd/instances/local_instance_group.h" #include "cuttlefish/host/commands/cvd/utils/subprocess_waiter.h" #include "cuttlefish/process/command.h" @@ -48,10 +50,8 @@ class CvdStartCommandHandler : public CvdCommandHandler { LocalInstanceGroup& group, const CommandRequest& request); - Result LaunchDevice( - Command command, LocalInstanceGroup& group, - const std::unordered_map& envs, - const CommandRequest& request); + Result LaunchDevice(Command command, LocalInstanceGroup& group, + const std::vector& instances); Result LaunchDeviceInterruptible( Command command, LocalInstanceGroup& group, diff --git a/base/cvd/cuttlefish/host/commands/cvd/instances/operator_client.cpp b/base/cvd/cuttlefish/host/commands/cvd/instances/operator_client.cpp index 4e9fda8e34a..3b152bcd7da 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/instances/operator_client.cpp +++ b/base/cvd/cuttlefish/host/commands/cvd/instances/operator_client.cpp @@ -32,6 +32,7 @@ #include "cuttlefish/common/libs/fs/shared_fd.h" #include "cuttlefish/common/libs/utils/json.h" #include "cuttlefish/common/libs/utils/users.h" +#include "cuttlefish/host/commands/cvd/instances/local_instance.h" #include "cuttlefish/host/commands/cvd/instances/local_instance_group.h" #include "cuttlefish/io/write_exact.h" #include "cuttlefish/result/result.h" @@ -39,13 +40,15 @@ namespace cuttlefish { namespace { -Result BuildPreRegistrationMsg(const LocalInstanceGroup& group) { +Result BuildPreRegistrationMsg( + const LocalInstanceGroup& group, + const std::vector& instances) { Json::Value msg; msg["message_type"] = "pre-register"; msg["group_name"] = group.GroupName(); msg["owner"] = CF_EXPECT(CurrentUserName()); Json::Value devices(Json::arrayValue); - for (const auto& instance : group.Instances()) { + for (const LocalInstance& instance : instances) { Json::Value dev; dev["id"] = instance.WebRtcDeviceId(); dev["name"] = instance.Name(); @@ -78,18 +81,28 @@ Result> OperatorControlConn::Create( * Pre-registers an instance group with the operator */ Result OperatorControlConn::Preregister(const LocalInstanceGroup& group) { - Json::Value pre_registration_msg = CF_EXPECT(BuildPreRegistrationMsg(group)); + return Preregister(group, group.Instances()); +} + +Result OperatorControlConn::Preregister( + const LocalInstanceGroup& group, + const std::vector& instances) { + const Json::Value pre_registration_msg = + CF_EXPECT(BuildPreRegistrationMsg(group, instances)); CF_EXPECT(WriteExact(*conn_, pre_registration_msg.toStyledString()), "Failed to send pre-registration message to operator"); - Json::Value response = + const Json::Value response = CF_EXPECT(RecvMsg(conn_), "Error receiving pre-registration response"); std::stringstream error_messages; bool errors_found = false; - for (const auto& res : CF_EXPECT(GetArrayValues(response, {}))) { - auto id = CF_EXPECT(GetValue(res, {"id"})); - auto status = CF_EXPECT(GetValue(res, {"status"})); - auto message = CF_EXPECT(GetValue(res, {"message"})); + for (const Json::Value& res : + CF_EXPECT(GetArrayValues(response, {}))) { + const std::string id = CF_EXPECT(GetValue(res, {"id"})); + const std::string status = + CF_EXPECT(GetValue(res, {"status"})); + const std::string message = + CF_EXPECT(GetValue(res, {"message"})); if (status != "accepted") { fmt::print(error_messages, "\nid: {}, status: {}, message: {}", id, status, message); diff --git a/base/cvd/cuttlefish/host/commands/cvd/instances/operator_client.h b/base/cvd/cuttlefish/host/commands/cvd/instances/operator_client.h index cb853cdad67..2a593a1dd92 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/instances/operator_client.h +++ b/base/cvd/cuttlefish/host/commands/cvd/instances/operator_client.h @@ -18,8 +18,10 @@ #include #include +#include #include "cuttlefish/common/libs/fs/shared_fd.h" +#include "cuttlefish/host/commands/cvd/instances/local_instance.h" #include "cuttlefish/host/commands/cvd/instances/local_instance_group.h" #include "cuttlefish/result/result.h" @@ -41,6 +43,12 @@ class OperatorControlConn { */ Result Preregister(const LocalInstanceGroup& group); + /** + * Pre-registers specific instances from a group with the operator + */ + Result Preregister(const LocalInstanceGroup& group, + const std::vector& instances); + private: OperatorControlConn(SharedFD conn) : conn_(conn) {} diff --git a/base/cvd/cuttlefish/host/commands/stop/main.cc b/base/cvd/cuttlefish/host/commands/stop/main.cc index 1825319ed0a..7b7becd0b33 100644 --- a/base/cvd/cuttlefish/host/commands/stop/main.cc +++ b/base/cvd/cuttlefish/host/commands/stop/main.cc @@ -81,12 +81,16 @@ std::set FallbackDirs() { std::set DirsForInstance( const CuttlefishConfig& config, - const CuttlefishConfig::InstanceSpecific instance) { - return { - config.assembly_dir(), + const CuttlefishConfig::InstanceSpecific& instance, + const bool include_assembly_dir) { + std::set dirs = { instance.instance_dir(), instance.instance_uds_dir(), }; + if (include_assembly_dir) { + dirs.insert(config.assembly_dir()); + } + return dirs; } // Gets a set of the possible process groups of a previous launch @@ -152,11 +156,13 @@ Result CleanStopInstance( int StopInstance(const CuttlefishConfig& config, const CuttlefishConfig::InstanceSpecific& instance, - const int32_t wait_for_launcher) { - auto result = CleanStopInstance(instance, wait_for_launcher); + const int32_t wait_for_launcher, + const bool include_assembly_dir) { + const Result result = CleanStopInstance(instance, wait_for_launcher); if (!result.has_value()) { LOG(ERROR) << "Clean stop failed: " << result.error(); - return FallBackStop(DirsForInstance(config, instance)); + return FallBackStop(DirsForInstance( + config, instance, /*include_assembly_dir=*/include_assembly_dir)); } return 0; @@ -205,10 +211,12 @@ int StopCvdMain(const int32_t wait_for_launcher, const bool clear_instance_dirs, return FallBackStop(FallbackDirs()); } - std::set instance_ids(instance_nums.begin(), instance_nums.end()); + const std::set instance_ids(instance_nums.begin(), + instance_nums.end()); int exit_code = 0; - auto instances = config->Instances(); + const std::vector instances = + config->Instances(); if (!instance_ids.empty() && !instances.empty()) { unsigned first_instance_id; @@ -223,9 +231,22 @@ int StopCvdMain(const int32_t wait_for_launcher, const bool clear_instance_dirs, } } + bool stopping_all_instances = instance_ids.empty(); + if (!stopping_all_instances && !instances.empty()) { + stopping_all_instances = true; + for (const CuttlefishConfig::InstanceSpecific& instance : instances) { + unsigned id = 0; + if (!absl::SimpleAtoi(instance.id(), &id) || + instance_ids.count(id) == 0) { + stopping_all_instances = false; + break; + } + } + } + std::vector> exit_state_futures; exit_state_futures.reserve(instances.size()); - for (const auto& instance : instances) { + for (const CuttlefishConfig::InstanceSpecific& instance : instances) { unsigned id; if (!absl::SimpleAtoi(instance.id(), &id)) { LOG(ERROR) << "Failed to parse instance ID \"" << instance.id() @@ -238,9 +259,11 @@ int StopCvdMain(const int32_t wait_for_launcher, const bool clear_instance_dirs, } std::future exit_code_from_thread = std::async( std::launch::async, - [&instance, &config, &wait_for_launcher, - &clear_instance_dirs]() -> int { - int exit_status = StopInstance(*config, instance, wait_for_launcher); + [&instance, &config, &wait_for_launcher, &clear_instance_dirs, + stopping_all_instances]() -> int { + const int exit_status = + StopInstance(*config, instance, wait_for_launcher, + /*include_assembly_dir=*/stopping_all_instances); if (clear_instance_dirs && DirectoryExists(instance.instance_dir())) { LOG(INFO) << "Deleting instance dir " << instance.instance_dir(); if (!RecursivelyRemoveDirectory(instance.instance_dir()) From e54c5e1d79fc96ebd007fb9ac6f2034a83909057 Mon Sep 17 00:00:00 2001 From: Aleksandr Shirvinskii Date: Tue, 10 Nov 2026 09:34:23 +0000 Subject: [PATCH 2/2] host_orchestrator: Drop deprecated usage stats flag on cvd start Remove the deprecated `--report_anonymous_usage_stats` flag from `Instance.Start`, `Group.Start`, and `StartOptions` in Host Orchestrator. Rename `TestStopStart` to `TestGroupStopStart` without changing its behavior, and add `TestSingleInstanceStopStart` to cover stopping and starting a single secondary instance within a multi-instance group. Bug: 581680503 Test: go test ./... in frontend/src/host_orchestrator Test: bazel build //orchestration/stop_start_test:stop_start_test_test Assisted-by: Jetski:Gemini --- .../orchestration/stop_start_test/BUILD.bazel | 1 + .../stop_start_test/main_test.go | 94 ++++++++++++++++++- .../host_orchestrator/orchestrator/cvd/cvd.go | 18 ++-- 3 files changed, 100 insertions(+), 13 deletions(-) diff --git a/e2etests/orchestration/stop_start_test/BUILD.bazel b/e2etests/orchestration/stop_start_test/BUILD.bazel index 1385bc48fff..0d017a9362b 100644 --- a/e2etests/orchestration/stop_start_test/BUILD.bazel +++ b/e2etests/orchestration/stop_start_test/BUILD.bazel @@ -16,6 +16,7 @@ load("@rules_go//go:def.bzl", "go_test") go_test( name = "stop_start_test_test", + size = "large", srcs = ["main_test.go"], deps = [ "//orchestration/common", diff --git a/e2etests/orchestration/stop_start_test/main_test.go b/e2etests/orchestration/stop_start_test/main_test.go index ad1e6116858..73b06046bfd 100644 --- a/e2etests/orchestration/stop_start_test/main_test.go +++ b/e2etests/orchestration/stop_start_test/main_test.go @@ -28,7 +28,7 @@ import ( const baseURL = "http://0.0.0.0:2080" -func TestStopStart(t *testing.T) { +func TestGroupStopStart(t *testing.T) { srv := hoclient.NewHostOrchestratorClient(baseURL) t.Cleanup(func() { if err := common.CollectHOLogs(baseURL); err != nil { @@ -116,6 +116,98 @@ func TestStopStart(t *testing.T) { } } +func TestSingleInstanceStopStart(t *testing.T) { + srv := hoclient.NewHostOrchestratorClient(baseURL) + if err := srv.Reset(); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if err := common.CollectHOLogs(baseURL); err != nil { + log.Printf("failed to collect HO logs: %s", err) + } + }) + config := ` + { + "instances": [ + { + "vm": { + "crosvm": { + "enable_sandbox": false + }, + "memory_mb": 8192, + "setupwizard_mode": "OPTIONAL", + "cpus": 8 + }, + "disk": { + "default_build": "@ab/aosp-android-latest-release/aosp_cf_x86_64_only_phone-userdebug", + "download_img_zip": true + }, + "streaming": { + "device_id": "cvd-1" + } + }, + { + "vm": { + "crosvm": { + "enable_sandbox": false + }, + "memory_mb": 8192, + "setupwizard_mode": "OPTIONAL", + "cpus": 8 + }, + "disk": { + "default_build": "@ab/aosp-android-latest-release/aosp_cf_x86_64_only_phone-userdebug", + "download_img_zip": true + }, + "streaming": { + "device_id": "cvd-2" + } + } + ] + } + ` + envConfig := make(map[string]interface{}) + if err := json.Unmarshal([]byte(config), &envConfig); err != nil { + t.Fatal(err) + } + createReq := &hoapi.CreateCVDRequest{ + EnvConfig: envConfig, + } + if _, err := srv.CreateCVD(createReq, &hoclient.AccessTokenBuildAPICreds{}); err != nil { + t.Fatal(err) + } + cvds, err := srv.ListCVDs() + if err != nil { + t.Fatal(err) + } + if diff := cmp.Diff(cvdStatus(cvds), []string{"Running", "Running"}); diff != "" { + t.Fatalf("status mismatch (-want +got):\n%s", diff) + } + + secondInstance := cvds[1] + if err := srv.StopInstance(secondInstance.Group, secondInstance.Name); err != nil { + t.Fatal(err) + } + cvds, err = srv.ListCVDs() + if err != nil { + t.Fatal(err) + } + if diff := cmp.Diff(cvdStatus(cvds), []string{"Running", "Stopped"}); diff != "" { + t.Fatalf("status mismatch (-want +got):\n%s", diff) + } + + if err := srv.StartInstance(secondInstance.Group, secondInstance.Name, &hoapi.StartCVDRequest{}); err != nil { + t.Fatal(err) + } + cvds, err = srv.ListCVDs() + if err != nil { + t.Fatal(err) + } + if diff := cmp.Diff(cvdStatus(cvds), []string{"Running", "Running"}); diff != "" { + t.Fatalf("status mismatch (-want +got):\n%s", diff) + } +} + func cvdStatus(cvds []*hoapi.CVD) []string { res := make([]string, len(cvds)) for i, v := range cvds { diff --git a/frontend/src/host_orchestrator/orchestrator/cvd/cvd.go b/frontend/src/host_orchestrator/orchestrator/cvd/cvd.go index cbde1d0c4b0..0031187cb34 100644 --- a/frontend/src/host_orchestrator/orchestrator/cvd/cvd.go +++ b/frontend/src/host_orchestrator/orchestrator/cvd/cvd.go @@ -323,7 +323,7 @@ func (i *Instance) Stop() error { func (i *Instance) Start(opts StartOptions) error { args := i.selectorArgs() - args = append(args, "start", "--report_anonymous_usage_stats=y") + args = append(args, "start") args = append(args, opts.toArgs()...) _, err := i.cli.exec(CVDBin, args...) return err @@ -531,11 +531,10 @@ func (g *Group) Remove() error { } type StartOptions struct { - SnapshotPath string - KernelImage string - InitramfsImage string - BootloaderRom string - ReportUsageStats bool + SnapshotPath string + KernelImage string + InitramfsImage string + BootloaderRom string } func (o *StartOptions) toArgs() []string { @@ -552,17 +551,12 @@ func (o *StartOptions) toArgs() []string { if o.BootloaderRom != "" { args = append(args, "--bootloader", o.BootloaderRom) } - if o.ReportUsageStats { - args = append(args, "--report_anonymous_usage_stats=y") - } else { - args = append(args, "--report_anonymous_usage_stats=n") - } return args } func (g *Group) Start(opts StartOptions) error { args := g.selectorArgs() - args = append(args, "start", "--report_anonymous_usage_stats=y") + args = append(args, "start") args = append(args, opts.toArgs()...) _, err := g.cli.exec(CVDBin, args...) return err