-
Notifications
You must be signed in to change notification settings - Fork 699
Don't read VMOpts.VZ struct from cidata directly #3992
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
base: master
Are you sure you want to change the base?
Conversation
Still need to fix VMOpts.VZ in the templates:
It is a bit over-engineered at the moment, since the docker templates don't set VMOpts. |
6dd9aad
to
8c936aa
Compare
@@ -118,7 +118,7 @@ func setupEnv(instConfigEnv map[string]string, propagateProxyEnv bool, slirpGate | |||
return env, nil | |||
} | |||
|
|||
func templateArgs(ctx context.Context, bootScripts bool, instDir, name string, instConfig *limatype.LimaYAML, udpDNSLocalPort, tcpDNSLocalPort, vsockPort int, virtioPort string) (*TemplateArgs, error) { | |||
func templateArgs(ctx context.Context, bootScripts bool, instDir, name string, instConfig *limatype.LimaYAML, udpDNSLocalPort, tcpDNSLocalPort, vsockPort int, virtioPort string, rosettaEnabled, rosettaBinFmt bool) (*TemplateArgs, error) { |
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.
Will this be changed again when #3899 comes in the future?
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.
Possibly, but not until the deprecated fields are gone from the top
rosettaEnabled := limaDriver.Info().Features.RosettaEnabled | ||
rosettaBinFmt := limaDriver.Info().Features.RosettaBinFmt | ||
// Disable Rosetta in Plain mode | ||
if *inst.Config.Plain { |
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.
If the hostagent could query the plain mode config to the driver, wouldn't the hostagent be able to pass them to the functions that process the template without touching the contents of the config?
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.
Yes, it would be possible to move the logic from the hostagent to the guest and the boot scripts and have them read $LIMA_CIDATA_PLAIN instead (if one is willing to make them more "smart" like that)
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.
Also, should the hostagent be able to receive the template string used for lima.env
from the driver?
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.
The lima.env is not handled by the driver, it is hostagent that creates the cidata (which includes lima.env)
This is supposed to be provided by the driver, and passed to the hostagent as features. The cidata and hostagent doesn't know VZ. Note: this also excludes the VMOpts.VZ from default and override, similar to the change made to VMOpts.QEMU already. It is unknown. Signed-off-by: Anders F Björklund <[email protected]>
8c936aa
to
e2f09da
Compare
This is supposed to be provided by the driver, and passed to the
hostagent as features. The cidata and hostagent doesn't know VZ.
Note: this also excludes the VMOpts.VZ from default and override,
similar to the change made to VMOpts.QEMU already. It is unknown.