-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improvements to Starnix VM handling #5334
base: master
Are you sure you want to change the base?
Conversation
This allows for multiple starnix VM pools to exist simultaneously without conflict, and avoids any unintential global state.
- Extraneous references to the `ffx log` pipe are closed, allowing the EOF from the subprocess to be propagated properly. - The SSH bridge into the Starnix sshd is now properly shut down when the instance is closed, avoiding a zombie process. - Some of the starnix code has been updated to be in line with a refactor that took place in other VM types.
They are very noisy and can cause important crash information to run off the end of the buffer; this is a temporary fix to avoid that.
6362cb7
to
d93d0f5
Compare
if err != nil { | ||
return nil, fmt.Errorf("failed to make ffx isolation dir: %w", err) | ||
} | ||
log.Logf(0, "initialized vm pool with ffx dir: %v", ffxDir) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: how about putting this under an if env.Debug
? (I do sometimes refer to this log string in order to run ffx log on the side, but it would be okay to have it only in debug mode.)
@@ -137,6 +161,12 @@ func (pool *Pool) Create(workdir string, index int) (vmimpl.Instance, error) { | |||
return inst, nil | |||
} | |||
|
|||
func (pool *Pool) Close() error { | |||
log.Logf(0, "shutting down vm pool with tempdir %v...", pool.ffxDir) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likewise, maybe print this only in debug mode.
if err != nil { | ||
return nil, err | ||
} | ||
inst.sshPrivKey = string(bytes.Trim(privkey, "\"\n")) | ||
|
||
// Copy auto-detected product bundle path from in-tree ffx to isolated ffx. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, glad this worked!
@@ -187,8 +215,9 @@ func (inst *instance) Close() error { | |||
} | |||
|
|||
func (inst *instance) startFuchsiaVM() error { | |||
err := inst.ffx("emu", "start", "--headless", "--name", inst.name, "--net", "user") | |||
if err != nil { | |||
inst.runFfx(1*time.Minute, "config", "get", "product.path") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: any reason the timeout is different from the same command in pool.Create()
?
No description provided.