standalone: support guest hv interface options - #1194
Conversation
| } | ||
|
|
||
| const TYPE_NAME: &str = "guest-hyperv-interface"; | ||
| const TYPE_NAME: &str = "hyperv-guest-interface"; |
There was a problem hiding this comment.
this is incidental but the bhyve impl spells its type name as bhyve-guest-interface and the swapped order was bugging me.
| /// The hypervisor interface to present to guest OSes. | ||
| /// | ||
| /// The variants here correspond to implementations of `Enlightenment`, which | ||
| /// may influence many aspects of a VM. Most immediately, different interfaces | ||
| /// have different CPUID leaves, but can also support para-virtualized features | ||
| /// such as additional hypercalls and MSRs. | ||
| #[derive(Clone, Default, Serialize, Deserialize, Debug, PartialEq)] | ||
| #[serde(tag = "type", rename_all = "lowercase")] | ||
| pub enum HypervisorInterface { | ||
| #[default] | ||
| Bhyve, | ||
|
|
||
| HyperV { | ||
| reference_tsc: bool, | ||
| }, | ||
| } |
There was a problem hiding this comment.
you are not seeing double! propolis-standalone/src/config.rs has a copy of this structure and comment.
I've only just realized that propolis-standalone does not use propolis-config-toml, and really they kind of just accept the same toml documents (for the most part) by intentional effort rather than shared code. I'm thinking about taking a shot at unifying those, but I want to do that a bit more intentionally (I dunno what all is in the propolis-config-toml dep tree and if it would make me sad rebuilding propolis-standalone). so, a copy for now.
There was a problem hiding this comment.
I've only just realized that propolis-standalone does not use propolis-config-toml, and really they kind of just accept the same toml documents (for the most part) by intentional effort rather than shared code
😬
There was a problem hiding this comment.
I cannot recall, but is this also related to the issue where we saw standalone wanted a u16 and server a string. I just ran into that the other day while working on fixing up vsock. So, you have my +1 on eventually unifying these.
There was a problem hiding this comment.
yeah, I don't remember which field it was but I also remember something wanting an integer literal where the other wanted a string..
| // things*. | ||
| // | ||
| // That all to say: this could be a list. | ||
| pub hv_interface: Option<HypervisorInterface>, |
There was a problem hiding this comment.
Given this comment, do any of these OSes pick and choose features from multiple hypervisor interfaces? In our case it seems that we are offering a standard bhyve hypervisor with the addition of just the hyperv tsc interface, could one later also gain some feature of kvm and be a bhyve-hyperv-kvm mix? If so, do you think HypervisorInterface is expressive enough to encapsulate that, or is that a bridge we will cross later if/when we reach it?
There was a problem hiding this comment.
do any of these OSes pick and choose features from multiple hypervisor interfaces?
I think the answer is "yes but not consistently", and that more than just picking features, sometimes there are guest OS quirks based on the inferred hypervisor that come along into just features. so for example the behavior where Linux assumes that on bare x86 hardware you need some iommu features to have x2avic, but under hypervisors that are known to expose x2avic, you can just use it (if CPUID bits say it's there). that ends up (if memory serves) keyed off the last hypervisor in the list, and it's not really a "feature" in that you don't look at any hypervisor CPUID leaves other than "there's a kvm in there somewhere"
so HypervisorInterface, alone, I think is probably fine, but we find ourselves needing a list here (#1195) the container will need to change so we have something like
main.hv_interface = [
{ type = "hyperv", reference_tsc = true },
{ type = "bhyve" }
]
or.. however you spell the toml. we might(?) want to do that before turning on the TSC enlightenment for real, just so guests don't assume we actually are Hyper-V (though some guests would pick the first understood hypervisor anyway, so..)
papertigers
left a comment
There was a problem hiding this comment.
I just have one question I left, but otherwise this seems good to me!
approximately twenty billion years ago (February last year) #849 saw us grow support for presenting Hyper-V as the guest hypervisor interface. this came with plumbing it into propolis-cli with the new
--hypervflag. we since overlooked adding that plumbing into propolis-standalone, where i found myself wanting it this week!i'd fully forgotten that as-merged in 725eb8e,
well ... this does that. i'm also just deleting the
--hypervflag here since i'm assuming no one other than me was using that anyway.both
propolis-standaloneandpropolis-clinow understand stanzas likeor
the latter being somewhat less interesting today since it's just a way to write out the defaults in more words.