This repository was archived by the owner on Nov 6, 2024. It is now read-only.
Add versioning to all bindings and conditionally compiled serde support to x86 bindings relevant to Firecracker snapshotting #14
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 6 commits
December 19, 2019 16:28
Signed-off-by: Alexandra Iordache <[email protected]>
...for snapshotting, which can be directly interpreted by serde with no additional work. Added TODOs for remaining relevant structs (& unions) that need a bit of handicraft. Added ser/deser unit tests using serde_json as serializer. Signed-off-by: Alexandra Iordache <[email protected]>
Signed-off-by: Alexandra Iordache <[email protected]>
Add dummy serialization support for kvm_msrs and kvm_cpuid2. These will be properly (de)serialized through their corresponding FamStructWrappers (Cpuid and Msrs). Signed-off-by: Alexandra Iordache <[email protected]>
Add serialization support for: * kvm_ioapic_state__bindgen_ty_1 - needed for kvm_ioapic, later needed for kvm_irqchip * kvm_irqchip__bindgen_ty_1 - needed for kvm_irqchip For each union, the field that takes up the most space is serialized. Signed-off-by: Alexandra Iordache <[email protected]>
Composed of architecture, kernel version and crate version (all strings). Up to the consumer to decide how to handle it. Signed-off-by: Alexandra Iordache <[email protected]>
alxiord
changed the title
jiangliu
reviewed
Dec 30, 2019
| @@ -22,3 +22,51 @@ pub mod bindings { | |||
| #[cfg(all(not(feature = "kvm-v4_14_0"), not(feature = "kvm-v4_20_0")))] | |||
| pub use super::bindings_v4_20_0::*; | |||
| } | |||
|
|
|||
| #[cfg_attr(feature = "with-serde", derive(Deserialize, Serialize))] | |||
There was a problem hiding this comment.
We have a proposal for a new crate at:
rust-vmm/community#76
which aims to build a common serde framework for snapshot/live migration/live upgrading.
It helps to simplify the code as
#[derive(Deserialize, Serialize)]
There was a problem hiding this comment.
Thanks! 👍 I think I don't fully understand what the new crate wants to achieve, I added a couple of questions to start from in the PR.
bjzhjing
added a commit
to bjzhjing/cloud-hypervisor
that referenced
this pull request
Jan 8, 2020
kvm migration implementation dependent on the following open PRs: rust-vmm/kvm-bindings#14 rust-vmm/vmm-sys-util#66 It requires to change the following dependencies URL to internal kvm-bindings kvm-ioctl vmm-sys-util Signed-off-by: Cathy Zhang <[email protected]>
yisun-git
pushed a commit
to sameo/cloud-hypervisor-1
that referenced
this pull request
Jan 15, 2020
kvm migration implementation dependent on the following open PRs: rust-vmm/kvm-bindings#14 rust-vmm/vmm-sys-util#66 It requires to change the following dependencies URL to internal kvm-bindings kvm-ioctl vmm-sys-util Signed-off-by: Cathy Zhang <[email protected]> Signed-off-by: Yi Sun <[email protected]>
|
Closing in favor of https://crates.io/crates/versionize |
epilys
pushed a commit
to epilys/kvm-bindings
that referenced
this pull request
Oct 28, 2024
They were unexported by commit 9527998. Fixes: rust-vmm#14 Signed-off-by: Samuel Ortiz <[email protected]>
epilys
pushed a commit
to epilys/kvm-bindings
that referenced
this pull request
Oct 28, 2024
The cargo tests run outside of the crate scope and allow us to verify that our public structures are properly exported. Fixes: rust-vmm#14 Signed-off-by: Samuel Ortiz <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #5
This PR adds
serdecompatibility to the most restricted set of bindings relevant to Firecracker snapshotting (firecracker-microvm/firecracker#1184), following the requirements laid out in this doc.It also introduces a static version variable, available on all platforms, composed of the architecture, kernel version and crate version. This will be relevant when deserializing bindings across different environments.
Regarding
serdesupport, the following features stand out:serdesupport is conditionally compiled and gated by thewith-serdefeature, in line with fam wrappers: add conditional serde compatibility vmm-sys-util#66.Serialization code will need to be manually added for each new:
This is a key difference from my previous proposal [RFC] Add serde [de]serialization for KVM bindings #4 and @jiangliu's current proposal RFC: enhance kvm-binding to optionally support serde #7. The reasoning is that deserialization introduces a new attack surface, so we should be as restrictive and attentive as possible. I am no longer of the opinion that serialization support should be easy to add automatically, with minimal interference from the developer. On the contrary, I'm now a firm believer in careful inspection of every new item that makes its way into a secure VMM's memory space.
Not all the bindings are serializable. Only the bare minimum required to hold the guest's state will be migrated / included in the snapshot, so why make them all serializable?
Bindings are not "byte-dump"-serialized anymore (as in [RFC] Add serde [de]serialization for KVM bindings #4 and RFC: enhance kvm-binding to optionally support serde #7). This is another item on which I've changed my mind. A corrupted byte-dump snapshot will be harder to detect and reject than a "well-typed" one. Furthermore, unlike a "well-typed", fully
serde-compatible format, a byte dump will be useless to inspect in 3rd party tools and painful to debug. The current solution allows the data to be dumped to JSON, for example, with theserde-jsoncrate, and inspected as text at struct field granularity. That's just not possible with a byte dump, and valuable for debugging.