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

Better API for application runtime keys #1493

Merged
merged 1 commit into from Mar 11, 2024
Merged

Conversation

samoht
Copy link
Member

@samoht samoht commented Jan 12, 2024

Tentative new API based on discussions in #1483
Use: mirage/mirage-skeleton#382

There are a few more cleanup to do but the changes are actually quite simple:

  • the main function in config.ml now takes a list of runtime keys - so you do not need to register them in unikernel.ml anymore - just write normal Cmdliner terms in there
  • all the devices now takes the evaluated runtime keys as parameters - this is a bit tricky sometimes for devices with a varying number of parameters, but I think that's not too bad.

You can see the main changes in test/functoria/e2e/app/app.ml and test/functoria/e2e/app/config.ml

@reynir and @hannesm: that would be great to have your input to know if this is going in the right direction.

@hannesm
Copy link
Member

hannesm commented Feb 27, 2024

We discussed in the MirageOS meeting on Feb 26th (yesterday) that this sounds nice (and is likely the way to go). One question that was raised if we can remove the dependency that mirage depends on mirage-runtime (and functoria depends on functoria-runtime). If this is the case, the mirage utility would be independent of e.g. lwt :)

@hannesm
Copy link
Member

hannesm commented Feb 27, 2024

Related to my earlier comment, the functoria does no longer depend on functoria-runtime within this PR (opam file of functoria needs to be updated), and mirage refers in lib/mirage/mirage_runtime_arg.mli Mirage_runtime.log_threshold -- but elsewhere I already mentioned that I'm not sure about this mirage_runtime_arg module, where it is useful, and why not just pass strings through.

@hannesm
Copy link
Member

hannesm commented Feb 27, 2024

these comments shouldn't hinder a merge and release -- I have not done a review of this PR though (and won't have time soon for this).

@samoht
Copy link
Member Author

samoht commented Mar 7, 2024

I've rebased this on top of main -- the changes are now just moving the runtime args from Key_gen to the parameters of the connect and start functions. @mirage/core reviews are welcome, especially the CHANGES file :p

@samoht samoht merged commit cc20443 into mirage:main Mar 11, 2024
3 checks passed
@samoht samoht deleted the runtime-keys branch March 11, 2024 14:37
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.

None yet

2 participants