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

runtime-rs: ch: Unbreak CH driver #8803

Conversation

jodh-intel
Copy link
Contributor

Unfortunately, PR #6876 broke running runtime-rs with Cloud Hypervisor so add minimal implementations of the new APIs to allow CH to be used with runtime-rs.

Full implementations will be added later on issues #8800, #8801, and #8802.

@katacontainersbot katacontainersbot added the size/small Small and simple task label Jan 11, 2024
}

pub(crate) fn resize_memory(&self, _new_mem_mb: u32) -> Result<(u32, MemoryConfig)> {
todo!()
// No resize currently - see https://github.com/kata-containers/kata-containers/issues/8801
Copy link
Member

Choose a reason for hiding this comment

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

@jodh-intel, I think it's worth adding a message here saying that the resize is not currently implemented.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I've also converted the 1st commit to return an error as that doesn't break the default use-case but does provide a GH issue URL too.

@jodh-intel jodh-intel force-pushed the issues-8784-runtime-rs-ch-rm-todo-to-unbreak branch from 2e01ca4 to 5d93a69 Compare January 11, 2024 14:10
Remove the `todo!()` macro which would cause a runtime crash and replace
with a implementation that returns an error as a stop-gap until kata-containers#8800 is
implemented.

Fixes: kata-containers#8785.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
Replace the `todo!()` calls with a minimal NOP implementation to return
the CH driver to working order since the `todo!()`'s forcibly crash the
driver at runtime. Full implementations for these APIs will be added on
issues kata-containers#8800, kata-containers#8801, and kata-containers#8802.

Fixes: kata-containers#8784.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
@jodh-intel jodh-intel force-pushed the issues-8784-runtime-rs-ch-rm-todo-to-unbreak branch from 5d93a69 to 29e0de4 Compare January 11, 2024 14:11
Copy link
Member

@fidencio fidencio left a comment

Choose a reason for hiding this comment

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

lgtm, thanks @jodh-intel!

@jodh-intel jodh-intel added safe-to-test Add to PR after manually reviewing to allow certain extra checks to run ok-to-test labels Jan 11, 2024
@jodh-intel
Copy link
Contributor Author

/test

Copy link
Contributor

@cmaf cmaf left a comment

Choose a reason for hiding this comment

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

thanks @jodh-intel

@fidencio fidencio merged commit a606401 into kata-containers:main Jan 11, 2024
177 of 186 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test safe-to-test Add to PR after manually reviewing to allow certain extra checks to run size/small Small and simple task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants