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

feat: add support for Firecracker v1+ #507

Merged
merged 1 commit into from
Aug 19, 2022
Merged

feat: add support for Firecracker v1+ #507

merged 1 commit into from
Aug 19, 2022

Conversation

richardcase
Copy link
Member

What this PR does / why we need it:

A few changes to support Firecracker v1+. We are sticking
with the v1 MMDS initially and will look to upgrade to the v2 MMDS in
the future.

To use this version with macvtap we will need to use our fork with the
1.1 changes.

NOTE: incorrect serialization caused the issues with MMDS not working.
Strangely Firecracker didn't complain about the invalid json config.
Time and stress wasted on what was essentially a typo. ffs.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #390

Special notes for your reviewer:

Checklist:

  • squashed commits into logical changes

Comment on lines +180 to +187
const (
// InstanceStateNotStarted the instance hasn't started running yet.
InstanceStateNotStarted InstanceState = "Not started"
// InstanceStateRunning the instance is running.
InstanceStateRunning InstanceState = "Running"
// InstanceStatePaused the instance is currently paused.
InstanceStatePaused InstanceState = "Paused"
)
Copy link
Member

Choose a reason for hiding this comment

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

lol how many ways of noting status do we have now? i think i count 3

Copy link
Member Author

Choose a reason for hiding this comment

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

I know 🤣 I just moved this down to be with their type declaration. We should pickup that bug again thats around the status.

MicroVM driver.
| Flintlock | Firecracker |
| ----------------- | -------------------------------- |
| v0.3.0 | Official v1.0+ or v1.0.0-macvtap |
Copy link
Member

Choose a reason for hiding this comment

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

have a line for 0.2.0 just for completeness?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added...good spot.

Callisto13
Callisto13 previously approved these changes Aug 19, 2022
A small number of changes to support Firecracker v1+. We are sticking
with the v1 MMDS initially andf will look to upgrade to the v2 MMDS in
the future.

To use this version with macvtap we will need to use our fork with the
1.1 changes.

NOTE: incorrect serialization caused the issues with MMDS not working.
Strangely Firecracker didn't complain about the invalid json config.
Time and stress wasted on wht was essentially a type.

Signed-off-by: Richard Case <richard.case@outlook.com>
@richardcase richardcase merged commit ca7e46b into liquidmetal-dev:main Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix flintlock to work with firecracker v1.0.0
2 participants