Support for serializing/deserializing KVM state

[andreeaflorescu]

This issue can be used for discussing the approach we take when serializing kvm-bindings.

Some other discussion points here: #7

[andreeaflorescu]

@alexandruag can you talk about the proxy solution you had in mind for handling serialization?

[sboeuf]

@andreeaflorescu

I don't think that the cost is so high TBH, but then it's probably better to discuss it on the actual proposal :)) I have a few concerns with adding serialize support directly on the bindings. Over time it can add a non-negligible size overhead.

This can be simply tackled through a Rust feature gate. This way, any project which does not expect to serialize these structures does not have to pay the price.

Then the second thing is that bindings in their current form (with the padding definition and FAM structs) is not exactly the "KVM state" in a sens.

The point for a consumer of this crate is to go entirely through it, so that it does not have to manage any of the "real" structures.
What's your proposal otherwise?

Globally, I'm not a fan of growing more and more crates. I'd be in favor of actually merging kvm-bindings with kvm-ioctls to simplify things. In my opinion, rust-vmm is not here to solve the problems from every possible projects, but instead is dedicated for VMM use cases. In that context, I think we should be opinionated and take some decisions fitting these use cases. That's why having the kvm structures deriving Versionize sounds reasonable to me, as it fits well for Firecracker, Cloud-Hypervisor and Alibaba's project.

[andreeaflorescu]

Globally, I'm not a fan of growing more and more crates. I'd be in favor of actually merging kvm-bindings with kvm-ioctls to simplify things.

This can have an easy fix by creating a kvm workspace with both kvm-bindings and kvm-ioctls in there. In any case, kvm-ioctls only supports one version of kvm-bindings, so having them both in the same repository makes sense to me.

In my opinion, rust-vmm is not here to solve the problems from every possible projects, but instead is dedicated for VMM use cases. In that context, I think we should be opinionated and take some decisions fitting these use cases. That's why having the kvm structures deriving Versionize sounds reasonable to me, as it fits well for Firecracker, Cloud-Hypervisor and Alibaba's project.

I don't think that separating state from the serialization of the state is fixing problems for every possible project. Again, I think it is better to look at how the proxy would work in practice before we go on the path with having serialization directly on structures.

[sboeuf]

This can have an easy fix by creating a kvm workspace with both kvm-bindings and kvm-ioctls in there. In any case, kvm-ioctls only supports one version of kvm-bindings, so having them both in the same repository makes sense to me.

Cool :)

I don't think that separating state from the serialization of the state is fixing problems for every possible project.

In a way it is because looking only at the current consumers, they all expect serialization and to rely on versionize. So we're creating more work for the beauty of having things properly separated, and I think that's great, but I think that can still come as a second step. For now, I think it's more important to show that all these consumers can share the same crate.

Again, I think it is better to look at how the proxy would work in practice before we go on the path with having serialization directly on structures.

Problem with this approach is that everything takes time. I'm sure @alexandruag will come up with a nice idea, but how long will it take until we can get this merged and actually used. It's okay if everything's not perfect at first, the improvement with this proxy crate can come later, but again the most important is that all project can share the kvm-bindings crate as soon as possible.

[andreeaflorescu]

This can have an easy fix by creating a kvm workspace with both kvm-bindings and kvm-ioctls in there. In any case, kvm-ioctls only supports one version of kvm-bindings, so having them both in the same repository makes sense to me.

Cool :)

I just created an issue to track this: rust-vmm/community#98
Lemme know what you think and I'll also try to ping a few more people to see if we can make this change happen :D

[sboeuf]

@andreeaflorescu @alexandruag
I opened #25 to close the gap between Firecracker fork of the kvm-bindings and the upstream version from rust-vmm.
This way, based on the timeline @alexandruag will provide about getting the proxy crate ready, we can simply start merging #25 and introduce @alexandruag's code later.

One more thing, (when) are you planning to push versionize to rust-vmm? This is the blocker to get anything like kvm-bindings or the proxy crate to support serialization and versioning.

[alexandruag]

Hi everyone! Sorry for being late in the day to the discussion. I also support the separation point of view, and the thing I'm working on should be ready for review (or I'll be convinced it doesn't work :D) by Monday. Let's have a look then and see what everybody thinks. Does that sound good?

Regarding the last question from Sebastien, I consider versionize, like serde, to be a very general purpose tool (in fact it would be super cool to have a single crate someday for serialization with and without version information), and I would not predicate using/maintaining it on being part of rust-vmm.

[sboeuf]

Hi everyone! Sorry for being late in the day to the discussion. I also support the separation point of view, and the thing I'm working on should be ready for review (or I'll be convinced it doesn't work :D) by Monday. Let's have a look then and see what everybody thinks. Does that sound good?

Sounds good!

Regarding the last question from Sebastien, I consider versionize, like serde, to be a very general purpose tool (in fact it would be super cool to have a single crate someday for serialization with and without version information), and I would not predicate using/maintaining it on being part of rust-vmm.

I think until the crate stays under the firecracker organization, it's gonna be hard to get kvm-bindings to rely on it. @jiangliu what do you think?

[alexandruag]

Hi all! I've completed a prototype for the versionize-by-proxy approach. It's available as a PR (so it's easier to comment etc.) in a Firecracker fork for now. Here's a quick rundown of the three commits in there:

• The first adds a new crate (actual name TBD), which only contains the definitions and logic required for the bindings that need to implement Versionize. It has no features (we only care about the latest bindings and always use FamStructWrapper). There's a file that contains the bindings definitions, and another with the autogenerated layout tests from upstream. I've only added the x86 module, but I'm happy to do the same for aarch64 if the approach makes sense.

• The second commit implements the conversion logic (upstream bindings <-> versionized bindings) for those structures that are part of the state objects. It uses bitwise conversion (which is the simplest/quickest approach), enabled by the fact that our versionized structures are identical to their upstream counterparts (approaches where they are different are possible, but not required here). There's also a function which helps convert FamStructWrappers.

• The third commit showcases how Firecracker can switch back to consuming upstream kvm-bindings and kvm-ioctls by using the adapters from the new crate in its VCPU/VM state objects.

I like the approach where the conversion is done using a separate crate because it completely separates any serialization-related concerns from the main bindings crate. In particular, it prevents feature creep (library crates that depend on bindings - i.e. something like cpuid in the future - would most likely have to have additional features themselves to control the way the import the bindings). It's interesting to discuss the implications of each approach, but for now I am looking forward to any first impressions related to the above.

In terms of initial measurements, the release Firecracker binary size is increased by ~50K, but only ~4K when building with codegen-units = 1 (which also reduces the overall binary size further). I've done some quick tests and the duration of a simple VCPU+VM state save+restore is increased by a negligible amount or up to several microseconds (will double check this as well if this path makes sense).

[acatangiu]

Hi all, I've looked at the versionize-by-proxy approach and it looks good, cool engineering, but I might be missing some info to understand the big picture and why it is necessary (at least in the case of kvm-bindings).

The PR brings ~3k new lines to maintain with the sole purpose to shadow/redefine existing structs and define subtle mechanisms with arguably brittle safety to transform between the kvm-bindings original and proxy types.

From my (limited) point of view the two options and their trade-offs for kvm-bindings are:

1. No proxy - using versionize directly on kvm-bindings with a feature gate.
   • Introduces specialized feature that needs to be cascaded up to stateless crates like kvm-ioctls
2. Proxy
   • introduces non-negligible-sized versioned-kvm-bindings shadow/proxy crate to maintain
   • increases complexity
   • [needs benchmark to confirm, but] possibly adds size and latency

• Common
  • Need to maintain and test multiple flavors - both versioned and un-versioned.

The same question should be asked for all crates that define any data required to be serialized/versioned. For example. in high-level crates that implement mostly functionality and very little state the data above might be seen differently.

From my understanding Firecracker, Cloud-Hypervisor and Alibaba's project all want versioned serialization and they're all interested in/committed to using versionize. What are the gains from doing proxy-serialization of kvm-bindings?

While this PoC is great, I want to understand the trade-offs we're considering here and what is the ultimate goal we're pursuing.

[sboeuf]

I'm not gonna repeat what @acatangiu said, cause I agree with everything :)
The code looks good, but do we really need this additional layer given that every project is planning on using Versionize through the bindings? And I'm slightly concerned about potential overheads too.

/cc @rbradford

[rbradford]

@alexandruag It's very cool what you've demonstrated from a technical perspective

I think we should be put the serialization/versionization inside kvm-bindings as all current users want it. If a future, hypothetical user doesn't want it then they can turn it off, fork or regenerate the bindings. The added cost of maintaining the extra crate and size/complexity impact is not worthwhile for a hypothetical situation.

[sandreim]

The PoC looks good @alexandruag . I understand the point you want to make by keeping the bindings/ioctls crates clean to work around "feature creep". So I am thinking about what is gained and what is lost for both rust-vmm and it's users if we choose to use the proxy or not. With everything else being fine I am not sure that adding extra complexity/maintenance requirements is the right thing to do this early.

Looking at the comments of other people it looks like they want to keep things simple and incrementally evolve the current implementation by working backwards from future usecases/issues as they come up.

[alexandruag]

Hi folks! Apologies for the delayed reply, and thanks for looking at the proposal. We're all looking at some options & next steps and will circle back to the thread.

[andreeaflorescu]

Taking into consideration the discussions on this thread, it looks that folks are inclined towards an approach that fixes the problem for the immediately interested users as opposed to considering the additional steps to achieve separation of concerns. With this in mind, we would like to propose the following course of action and focus on two directions aiming at solving the needs of current customers (Alibaba, CH, Firecracker) in the short term, while finding the best approach for the rust-vmm project in the long run.

1. Short-term:
   1. Have the versionize support added directly on top kvm-bindings.
   2. The advantages of this solution is that it has the least potential overhead for the current use cases.
   3. There a few changes needed before merging the PR, but we should raise them in the PR directly.
2. Long-term:
   1. We’re looking for a path towards separating the serialization logic from the autogenerated bindings.
   2. This approach can start from the proxy proposal, and we can iterate on it (for example to have a clearer picture about the overhead involved).
   3. To get this started, we’d like to know what are people’s major concerns with the alternative approach, and if there are any blockers in terms of deploying it to production.

From a separation perspective, kvm-bindings is what we consider a low level API, while serialization is an application specific concern. I believe this is not so easy to see due to the current dynamics of rust-vmm, as the active community consists mainly of developers from Firecracker, Cloud Hypervisor, and Alibaba, products that all plan to use a similar form of serialization.

Are people onboard with the general idea and the course of action?

[jiangliu]

Hi everyone! Sorry for being late in the day to the discussion. I also support the separation point of view, and the thing I'm working on should be ready for review (or I'll be convinced it doesn't work :D) by Monday. Let's have a look then and see what everybody thinks. Does that sound good?

Sounds good!

Regarding the last question from Sebastien, I consider versionize, like serde, to be a very general purpose tool (in fact it would be super cool to have a single crate someday for serialization with and without version information), and I would not predicate using/maintaining it on being part of rust-vmm.

I think until the crate stays under the firecracker organization, it's gonna be hard to get kvm-bindings to rely on it. @jiangliu what do you think?

Apology for the delay:( And eventually I have time to go over the versionize and the "Dummy PR for proxy bindings" PR.

Last year we have tried a similar way as the proxy binding solution as a generic way to support live-upgrade. We have encountered several challenges.

1. Rust doesn't ensure data struct layout for repr(rust) structs. For kvm-binding, this may not be an issue because all structs are repc(C).
2. It's very challenge to keep in sync between the main object structs and the proxy state structs.
3. It's very hard to serialize/deserialize Arc objects, though this is a special case for live-upgrade.
4. We adopted a lockstep upgrading policy to avoid support of multiple versions.

Now we are planning to refactor based on the versionize crate due to the capability to support multiple versions of data structs. And we hope we could add a feature to the versionize crate to enable/disable generating versionized serde. But that way, we could avoid the cost to maintain separate proxy crates.

FYI:
https://github.com/cloud-hypervisor/vmm-serde/blob/v1/impl/src/lib.rs
https://github.com/cloud-hypervisor/vmm-serde/blob/v1/impl/src/export_as_pub.rs

[alexandruag]

Hi Gerry! Like you mention, for kvm-bindings the repr(C) and other guarantees regading the structs enable more translation approaches. Less restrictive state objects (which are not under the direct control of the application) are usually a lot less verbose, and that becomes an important simplifying assumption in itself. It looks like keeping things in sync becomes much easier with certain testing approaches, which can often be autogenerated. I think it's best to avoid conflating live data (i.e. Arcs) and state, and this is another reason why the two should be cleanly separated.

Having versionize generate serde output would be really cool and we've chatted about it before, but it seemed like quite the undertaking. Maybe it makes sense to get in touch with the serde folks to see what their thoughts are on versioned serialization and whether there's some common ground going forward.

[sboeuf]

Taking into consideration the discussions on this thread, it looks that folks are inclined towards an approach that fixes the problem for the immediately interested users as opposed to considering the additional steps to achieve separation of concerns. With this in mind, we would like to propose the following course of action and focus on two directions aiming at solving the needs of current customers (Alibaba, CH, Firecracker) in the short term, while finding the best approach for the rust-vmm project in the long run.

Sounds good!

1. Short-term:
   Have the versionize support added directly on top kvm-bindings.
   The advantages of this solution is that it has the least potential overhead for the current use cases.
   There a few changes needed before merging the PR, but we should raise them in the PR directly.

We talked about adding rust feature to gate Versionize being derived or not.

2. Long-term:
   We’re looking for a path towards separating the serialization logic from the autogenerated bindings.
   This approach can start from the proxy proposal, and we can iterate on it (for example to have a clearer picture about the overhead involved).
   To get this started, we’d like to know what are people’s major concerns with the alternative approach, and if there are any blockers in terms of deploying it to production.

Based on @alexandruag's PR and based on his feedback the major concerns are:

• Increased complexity
• Increased maintenance
• Performance slow down
• Increased footprint

From a separation perspective, kvm-bindings is what we consider a low level API, while serialization is an application specific concern. I believe this is not so easy to see due to the current dynamics of rust-vmm, as the active community consists mainly of developers from Firecracker, Cloud Hypervisor, and Alibaba, products that all plan to use a similar form of serialization.

I totally get it, but the point is to be pragmatic by solving real problems first. It's very hard and time consuming to try to handle potential users and use cases that might never happen.

Are people onboard with the general idea and the course of action?

Yes :)

[roypat]

Serialization support via an optional serde feature was implemented in #101