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

cloud-hypervisor: add platformOEMStrings and extraPlatformOpts. #336

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Ramblurr
Copy link

The cloud-hypervisor command line interface unfortunately doesn't support
multiple instances of the same arg with a different value, so we have to resort
to these extra module options rather than using extraArgs.

To make matters even worse, the --platform argument (of which there can be
only one), is overloaded with different types of sub-args that also need to be
provided multiple times.

This commit allows the operator to add oem strings (for example to pass systemd
credentials), as well as raw platform options as needed.

The cloud-hypervisor command line interface unfortunately doesn't support
multiple instances of the same arg with a different value, so we have to resort
to these extra module options rather than using extraArgs.

To make matters even worse, the `--platform` argument (of which there can be
only one), is overloaded with different types of sub-args that also need to be
provided multiple times.

This commit allows the operator to add oem strings (for example to pass systemd
credentials), as well as raw platform options as needed.
Copy link

nix-ci-app bot commented Feb 18, 2025

NixCI is ready to run on this PR.
Maintainer: Comment nix-ci run to run now.

Ramblurr added a commit to Ramblurr/microvm.nix that referenced this pull request Feb 18, 2025
This commit implements `microvm.credentialFiles` a mechanism for passing
credentials into guest vms from the host.

Currently only support for qemu is implemented as I want to test the waters to
see if you're interested in this feature, Astro.

In addition to qmeu cloud-hypervisor can be supported via smbios. But it depends
on [this feature being added](cloud-hypervisor/cloud-hypervisor#6951 (comment)), and also astro#336 being merged to microvm.nix

cloud-hypervisor could be supported immediately, but then the secrets would be
visible in the ps output.

A cursory code search shows that the following additional hypervisors could be
supported:

- crosvm: via fw_cfg, or smbios
- alioth: via fw_cfg
- stratovirt: via fw_cfg (maybe smbios)

kvmtool and firecracker both seem like they cannot be supported.

Related:
- astro#259
- astro#52
@@ -483,6 +483,30 @@ in
'';
};

cloud-hypervisor.platformOEMStrings = mkOption {
Copy link
Owner

Choose a reason for hiding this comment

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

Instead of creating another option, you could just use map (lib.removePrefix "oem_strings=") (builtins.filter (lib.hasPrefix "oem_strings=") config.cloud-hypervisor.extraPlatformOpts)) to get these.

Copy link
Author

Choose a reason for hiding this comment

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

That wouldn't quite parse it correctly because there is the square bracket list syntax too. And we have to combine it with the vsock OEM string.

But we could parse them out and then rebuild the oem_strings arg, should I so that?

It's a shame that chv has such a clunky interface.

Copy link
Owner

Choose a reason for hiding this comment

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

You're the one with the new use-case here :-)

If you agree that it is worthwhile working around chv's clunky interface, so please go ahead!

If you prefer separate options, I'm going to accept them.

[ "io.systemd.credential:APIKEY=supersecret" ]
'';
};
cloud-hypervisor.extraPlatformOpts = mkOption {
Copy link
Owner

@astro astro Feb 18, 2025

Choose a reason for hiding this comment

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

I like cloud-hypervisor myself, but one of the main features of this Flake is choice with a single change to config.microvm.hypervisor. At least qemu has similar parameters but I really would like to see this researched for all seven supported VMMs. Oh, --platform is chv-specific!

I wonder if this too should be filtered from config.cloud-hypervisor.extraArgs, thereby removing that limitation for us?

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