Skip to content
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

Change cpuType default to null #2333

Merged
merged 1 commit into from
May 18, 2024
Merged

Conversation

afbjorklund
Copy link
Member

Avoid issues with converting null strings in a map value

Change order to alphabetical, to match the "limactl info"


Helps with:

Avoid issues with converting null strings in a map value

Change order to alphabetical, to match the "limactl info"

Signed-off-by: Anders F Björklund <[email protected]>
@afbjorklund
Copy link
Member Author

afbjorklund commented May 10, 2024

Note: the -pdpe1gb is only true when running on a Darwin host, not on Linux (and looks a bit odd).

But I figured it is best to have details in the code, and avoid duplicating them here... Maybe remove it?

cpuType:
  # aarch64: "cortex-a72" # (or "host" when running on aarch64 host)
  # armv7l: "cortex-a7" # (or "host" when running on armv7l host)
  # riscv64: "rv64" # (or "host" when running on riscv64 host)
  # x86_64: "qemu64" # (or "host" when running on x86_64 host)

# 🟢 Builtin default: "qemu64" (or "host,-pdpe1gb" when running on x86_64 host)
x86_64: null
# 🟢 Builtin default: "rv64" (or "host" when running on riscv64 host)
riscv64: null
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove these lines?

Copy link
Member Author

@afbjorklund afbjorklund May 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a problem with the json, when it is trying to marshal "null" into a string value...

After changing CPUType to map[Arch]*string, it is a problem to express as jsonschema.

Also, it means that we don't have to carry the list of architectures in the "empty" yaml?

Made some similar changes to ssh.localPort and Rosetta omitempty, but will post separately

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

 images: []
-cpuType:
-  aarch64: ""
-  armv7l: ""
-  riscv64: ""
-  x86_64: ""
-ssh:
-  localPort: 0
{
   "audio" : {},
   "caCerts" : {},
   "containerd" : {},
   "cpuType" : {
      "aarch64" : "",
      "armv7l" : "",
      "riscv64" : "",
      "x86_64" : ""
   },
   "firmware" : {},
   "hostResolver" : {},
   "images" : null,
   "rosetta" : {
      "binfmt" : null,
      "enabled" : null
   },
   "ssh" : {
      "localPort" : 0
   },
   "video" : {
      "vnc" : {}
   }
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also the (text of the) lines are not removed, they are just translated into comments instead.

When uncommented, the default is reconstructed (could use "" or null for empty values)

 cpuType:
  aarch64: "cortex-a72" # (or "host" when running on aarch64 host)
  armv7l: "cortex-a7" # (or "host" when running on armv7l host)
  riscv64: "rv64" # (or "host" when running on riscv64 host)
  x86_64: "qemu64" # (or "host,-pdpe1gb" when running on x86_64 host)

Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@AkihiroSuda AkihiroSuda added this to the v0.23.0 (tentative) milestone May 18, 2024
@AkihiroSuda AkihiroSuda merged commit 9d15dba into lima-vm:master May 18, 2024
27 checks passed
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.

2 participants