Support per-instance start bypassing assembly#2617
Conversation
d8ed72c to
58c54a5
Compare
29a424c to
2d985ec
Compare
0997443 to
ef25374
Compare
066564f to
4cb3d8d
Compare
| auto config_path = | ||
| group.AssemblyDir() + "/cuttlefish_config.json"; | ||
| if (FileExists(config_path)) { | ||
| return LaunchSingleInstance(instance, group, request); |
There was a problem hiding this comment.
nit: return CF_EXPECT(LaunchSingleInstance(...))
The macro is necessary to add an additional member to the stack trace held inside the Result.
There was a problem hiding this comment.
I've updated the code to use CF_EXPECT to propagate the stack trace.
| auto config_path = | ||
| group.AssemblyDir() + "/cuttlefish_config.json"; |
There was a problem hiding this comment.
nit: the style guide writes
Use type deduction only if it makes the code clearer to readers who aren't familiar with the project, or if it makes the code safer. Do not use it merely to avoid the inconvenience of writing an explicit type.
The destructuring auto above is fair since it's the only way to write something like that, but in this case it's helpful while reading to know what the types are when possible.
There was a problem hiding this comment.
refactored and removed
| if (FileExists(config_path)) { | ||
| return LaunchSingleInstance(instance, group, request); | ||
| } else { | ||
| LOG(INFO) << "Group configuration does not exist on disk. Proceeding with normal group start."; |
There was a problem hiding this comment.
INFO is kind of high. Any launch from cvd load or cvd create --config_file will trigger this:
Maybe DEBUG?
| auto config_path = | ||
| group.AssemblyDir() + "/cuttlefish_config.json"; |
There was a problem hiding this comment.
From the perspective of cvd, whether cuttlefish_config.json exists is an implementation detail of assemble_cvd and it would be better to avoid more dependency on this fact than we already have.
Is it possible to determine the conditions for re-launching an instance through whether cvd start is being invoked on an instance state matching INSTANCE_STATE_STOPPED?
There was a problem hiding this comment.
Done. Replaced the FileExists check on the config file with instance.State() == cvd::INSTANCE_STATE_STOPPED
f239b65 to
c7ff09b
Compare
c7ff09b to
b847821
Compare
### Summary
Running cvd start typically triggers assemble_cvd, which regenerates the cuttlefish_config.json for all configured instances. Running this in a shared environment for a single instance can overwrite and corrupt the configuration of already running instances. This change allows starting a specific stopped instance using its existing assembled configuration.
### Key Changes
Assembly Bypass: Modified CvdStartCommandHandler::Handle to detect if a specific instance is targeted via selectors. Added a disk check for cuttlefish_config.json before bypassing assembly. If the configuration is missing, it correctly falls back to the normal group start path, runs assemble_cvd, and boots all instances.
LaunchSingleInstance Implementation: Added LaunchSingleInstance which directly constructs and invokes the run_cvd command for the target instance, bypassing assemble_cvd.
Environment Setup: Ensures LaunchSingleInstance sets up the exact environment variables required for run_cvd to boot the specific instance.
Database Status Update Fix: Updated LaunchSingleInstance to only update the database state for the targeted instance being started, preventing other stopped instances in the group from being incorrectly marked as active.
Bug
b/459780275
Document
go/cuttlefish-per-instance-control
Frontend pr
#2618