Implement and export CpuId, MsrList and Msrs as FamStructWrappers

[acatangiu]

Description of changes

kvm_cpuid2, kvm_msr_list and kvm_msrs each contain a flexible array member and are forcing users of them to work with unsafe code.
Implement and export CpuId, MsrList and Msrs as safe wrappers over them.

[sameo]

Although I'm not a big fan of mixing autogenerated code with manually crafted one, I still prefer to have all the structure definitions (And I consider the FAM implementations to be a structure definition) in the same place.
So, approving that one...

[andreeaflorescu]

Let's put merging this PR a bit on hold. I am mainly concerned about the dependency on vmm-sys-util. During the rust-vmm meetup we discussed with CrosVM about consuming rust-vmm crates and kvm-bindings was one of the low hanging fruits. As they have a different process for consuming crates, we should double check with them if they still want to use this crate and if adding a vmm-sys-util dependency is fine with them.

CC: @dgreid @zachreizner

[acatangiu]

Let's put merging this PR a bit on hold. I am mainly concerned about the dependency on vmm-sys-util. During the rust-vmm meetup we discussed with CrosVM about consuming rust-vmm crates and kvm-bindings was one of the low hanging fruits. As they have a different process for consuming crates, we should double check with them if they still want to use this crate and if adding a vmm-sys-util dependency is fine with them.

@andreeaflorescu I don't think this is a good principle to apply to decisions regarding the rust-vmm crates design/features. I believe we should strive to do what's best for the crate and the rust-vmm project in general, even when that might make things harder for a particular consumer because of their internal/private process of consuming crates. We should aim for maximizing the collective gain.

Regarding CrosVM in particular, I believe that vmm-sys-util is also a low hanging fruit for an useful crate they can consume from rust-vmm.

CC: @dgreid @zachreizner

[alexandruag]

I think we shouldn't dismiss dependency management so easily. Besides, it seems straightforward to add a feature to gate the inclusion of the FAM wrappers (and the vmm-sys-util dependency). How about we do just that for now?

[andreeaflorescu]

Is the plan to update the PR and make the dependency optional?

[acatangiu]

Is the plan to update the PR and make the dependency optional?

That’s a good idea to solve the dependency problem if there ever was one, still waiting on feedback.

[jiangliu]

Could we make the feature as default?

[acatangiu]

@andreeaflorescu @jiangliu @sameo I've updated the PR to condition the safe-wrappers and the vmm-sys-util dependency on a crate feature.

The safe-wrappers feature is enabled by default. Any consumers of kvm-bindings who do not want the wrappers or do not want the vmm-sys-util dependency can opt-out and disable both.

[alexandruag]

From what I've seen so far, features should be opt-in unless there's a good reason to do otherwise. This follows the principle of least astonishment, as it prevents users from unknowingly pulling unnecessary code/dependencies either directly or via transitive dependencies.

@jiangliu, I was just wondering, does having this feature on by default enable any use case that becomes difficult or inconvenient otherwise?

[acatangiu]

@alexandruag I was not aware of the principle of least astonishment.
I did, however, look at the top downloaded crates on crates.io and saw that most come with a default list of features. Some examples from the top 10 most downloaded crates:

• quote
• syn
• regex

Therefore, I'm not sure what is the canonic path we should take. I made a judgement-call of including the safe wrappers by default, but I don't see that as a blocker, both ways work and I can update the PR to not have them by default.

@aghecenco @andreeaflorescu @sameo @jiangliu what is your view (tagging the code-owners of this crate)?

[alexandruag]

There's no absolute rule here, but I think making people "opt-in" into things they want/need to use instead of surprising them with a default configuration of features is a very sound design principle, especially because it means the least amount of additions are present by default (in most cases).

To give an example, let's say crate A has some features which are enabled by default, and crate B uses crate A without needing any of the features, but doesn't disable any of them either. If I would like to use crate B in my project, I kinda have to deal with all those dependencies/code coming from A via features, even though they're not necessary.

It would be very interesting to know if there's an use case that's impacted by having features off by default.

[jiangliu]

I changed my mind:)
Letting users to explicitly opt-in features may be a good choice for rust-vmm because we want to control all external dependencies.

[acatangiu]

@andreeaflorescu @alexandruag @jiangliu I've removed the feature from the default list of features. It is now opt-in.

[andreeaflorescu]

We should also update the readme to specify that we now have a new feature and describe why/how to use it.

[acatangiu]

Addressed all comments, please re-review.

I will add a CHANGELOG entry in a following PR where I'll also do the version bump.

[acatangiu]

@sameo @jiangliu I've updated the Readme at @andreeaflorescu's suggestion. Please take another look.