Skip to content

Conversation

afbjorklund
Copy link
Member

@afbjorklund afbjorklund commented Aug 23, 2025

The format of the VMOpts is known only to the driver, everyone
else will see a basic map[string]any or something similar to it.

Note: the string is the driver type, the any is the actual Opts

Converting from the abstract format to the actual format is done
using YAML, just like it was before when the format was known.

Issue #3506

Note: the use of QEMUOpts is not addressed here, only VMOpts.

There needs to be a follow up, to move the struct into the driver.

Same for VZ.

Depends:

@afbjorklund
Copy link
Member Author

@unsuman here is a way to convert the VMType into an abstract "struct"

-type VMOpts struct {
-       QEMU QEMUOpts `yaml:"qemu,omitempty" json:"qemu,omitempty"`
-}
+type VMOpts map[VMType]any

Then converting/casting is done, by marshalling to YAML and unmarshalling

@unsuman
Copy link
Contributor

unsuman commented Aug 23, 2025

@unsuman here is a way to convert the VMType into an abstract "struct"

-type VMOpts struct {
-       QEMU QEMUOpts `yaml:"qemu,omitempty" json:"qemu,omitempty"`
-}
+type VMOpts map[VMType]any

Then converting/casting is done, by marshalling to YAML and unmarshalling

In this way, the drivers can provide their specific settings without changes needed in the main codebase. Thanks, this looks clean and adaptable!

@jandubois
Copy link
Member

jandubois commented Aug 23, 2025

I just remembered that the Kubernetes community has been going back and forth about whether user data strings should be encoded as maps, or as list with a name field. I think it has currently settled on preferring lists over maps.

If we were following this convention (and I'm not sure we should, but wanted to at least bring it up), then vmOpts would look like this instead:

vmOpts:
- name: qemu
  value:
    qemuOption1: ...
- name: vz
  value:
    vzOption1: ...

Personally I'm not fond of this because it makes jq/yq expressions more complicated, but I do understand the argument that this makes it clearer which keys are struct fields, and which are user supplied list entries.

Update

This rule maintains the invariant that all JSON/YAML keys are fields in API objects. The only exceptions are pure maps in the API (currently, labels, selectors, annotations, data), as opposed to sets of subobjects.

https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#lists-of-named-subobjects-preferred-over-maps

@afbjorklund
Copy link
Member Author

afbjorklund commented Aug 24, 2025

I have only seen that (list) as a workaround for ordered maps.
But then it is usually one map per value, and not extra fields like "name"/"value" (or key/value?)

vmOpts: !!omap
- qemu:
    # Minimum version of QEMU required to create an instance of this template.
    # Will be ignored if the vmType is not "qemu"
    # 🟢 Builtin default: not set
    minimumVersion: null
    # Specify desired QEMU CPU type for each arch.
    # You can see what options are available for host emulation with: `qemu-system-$(arch) -cpu help`.
    # Setting of instructions is supported like this: "qemu64,+ssse3".
    # 🟢 Builtin default: hard-coded arch map with type (see the output of `limactl info | jq .defaultTemplate.cpuType`)
    cpuType:
    #   aarch64: "max" # (or "host" when running on aarch64 host)
    #   armv7l:  "max" # (or "host" when running on armv7l host)
    #   riscv64: "max" # (or "host" when running on riscv64 host)
    #   x86_64:  "max" # (or "host" when running on x86_64 host; additional options are appended on Intel Mac)
- vz:
    rosetta:
      # Enable Rosetta inside the VM; needs `vmType: vz`
      # Hint: try `softwareupdate --install-rosetta` if Lima gets stuck at `Installing rosetta...`
      # 🟢 Builtin default: false
      enabled: null
      # Register rosetta to /proc/sys/fs/binfmt_misc
      # 🟢 Builtin default: false
      binfmt: null
- docker:
    runtime: runq

The benefit of not doing something like this here, is that it is backwards-compatible to not do it...
The struct and the map[string] serializes in the same way.


I am glad we are not using kyaml here...

I don't think we need the ordered maps?

Preferrably the vmOpts should look like any other field in the lima.yaml, even if it is now "extendable".
We do need to have a callback to document and validate which fields are available, but that's separate.

@afbjorklund afbjorklund marked this pull request as ready for review August 24, 2025 07:08
@afbjorklund
Copy link
Member Author

afbjorklund commented Aug 24, 2025

One annoying quirk of go-yaml is that 1 unmarshals as uint64(1) which is not very intuitive...

map[string]any{"bar": string("two"), "foo": uint64(1)}

So I changed the unittest to use a static YAML/JSON string, instead of comparing with the any.

`{"foo":1,"bar":"two"}`

@jandubois
Copy link
Member

I don't think we need the ordered maps?

I don't think it is about ordering, it is about the API promise.

As I wrote above, I'm not a bug fan of this convention for CLI usability reasons, and I'm fine to use the map with the driver name as the key. It just should be an intentional decision we make.

@jandubois
Copy link
Member

One annoying quirk of go-yaml is that 1 unmarshals as uint64(1) which is not very intuitive...

When unmarshaling into any it picks the largest type that can hold the value, which is uint64. It would be int64 if it was -1.

It is kind of arbitrary which rules you pick. YAML originates from untyped languages (Perl) were type was inferred from the value and could change (transmogrify) at runtime as needed, which is not a good fit for statically typed languages. Unmarshaling into any is not a strongly typed concept. 😄 It emulates untyped features using reflection.

@AkihiroSuda AkihiroSuda added this to the v2.0.0 milestone Aug 27, 2025
@afbjorklund
Copy link
Member Author

afbjorklund commented Sep 7, 2025

Needs to convert VMOpts.VZ as well, and move the validation (and the struct?) to the driver.

It might not be possible to move the QEMUOpts and VZOpts yet, but they should be "gone"

type QEMUOpts struct {
        MinimumVersion *string `yaml:"minimumVersion,omitempty" json:"minimumVersion,omitempty" jsonschema:"nullable"`
        CPUType        CPUType `yaml:"cpuType,omitempty" json:"cpuType,omitempty" jsonschema:"nullable"`
}

type CPUType = map[Arch]string

type VZOpts struct {
        Rosetta Rosetta `yaml:"rosetta,omitempty" json:"rosetta,omitempty"`
}

type Rosetta struct {
        Enabled *bool `yaml:"enabled,omitempty" json:"enabled,omitempty" jsonschema:"nullable"`
        BinFmt  *bool `yaml:"binfmt,omitempty" json:"binfmt,omitempty" jsonschema:"nullable"`
}

That is, they can stay as deprecated. But eventually the value is known only to the driver.

We will probably have to give up on the jsonschema for Opts, and leave that to driver Validate.

@afbjorklund afbjorklund changed the title Convert limayaml.VMType to an abstract map Convert limayaml.VMOpts to an abstract map Sep 7, 2025
@afbjorklund
Copy link
Member Author

afbjorklund commented Sep 7, 2025

It is kind of arbitrary which rules you pick.

We will go with what "the library" is using...

And provide an entry point, for drivers to use:

func Convert(x, y any, comment string) error

Then Lima can switch libraries, in a new SDK.

@afbjorklund afbjorklund changed the title Convert limayaml.VMOpts to an abstract map Convert limatype.VMOpts to an abstract map Sep 7, 2025
@afbjorklund afbjorklund force-pushed the vmopts-qemu branch 2 times, most recently from fdebf9c to cc3c2f2 Compare September 13, 2025 18:04
@afbjorklund afbjorklund marked this pull request as draft September 19, 2025 05:36
The format of the VMOpts is known only to the driver, everyone
else will see a basic map[string]any or something similar to it.

Converting from the abstract format to the actual format is done
using YAML, just like it was before when the format was known.

Signed-off-by: Anders F Björklund <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants