Accurate versions in SMBIOS tables #701
Comments
|
If you're going to include the commit hash it might also help to include the commit depth (as close to a monotonic revision number as you can get with git) like we do for Helios components. And, I guess, also the branch name, etc. |
|
Since Propolis itself is not published to crates.io, we don't really increment the crate version number very often --- it will almost always be 0.1.0. I think the Git metadata is much likelier to be useful. We may also want to include the bhyve API version in a system version string? |
|
One point worth noting: If we put version data about propolis and/or bhyve in the SMBIOS tables, it will potentially become out of date when the instance is live-migrated. |
Currently, the SMBIOS system (type 1) table has a hard-coded version field. It would be nice if this was instead populated with details about the Propolis version, as described in issue #701. This branch adds a `build.rs`` script that uses [the `vergen` crate][1] to emit information about the Git revision that Propolis was build from. Now, we can generate a version string that describes the git branch, commit hash, and commit depth, as described in [this comment][2]. This is generated in a `propolis::version()` function, which also includes the Bhyve API version (detected at runtime). This results in version strings like: ``` Propolis v0.1.0-658 (0d8efa1) eliza/somebios-version, <unknown Bhyve API version> ``` (on a Linux machine; the Bhyve version would be present on illumos) In addition to populating the SMBIOS system version, this commit also sets the Clap version for CLI commands, so that the `--version` flag will print out the same value that's set in SMBIOS. [1]: https://docs.rs/vergen [2]: #701 (comment)
|
Regarding the OVMF version, since Propolis just gets the bootrom from a path to an arbitrary file in the TOML config, it doesn't have an easy way to know what the OVMF version is. Probably, we want to make the reported BIOS version configurable in the config file, and ensure that it's set to whatever OVMF version Propolis gets? |
I don't think it's a bad idea, but rather one of limited utility, at least for now. I'd rather not clutter up the build until we have a clear reason.
At some point, I imagine we'll have some sort of selectable bootrom profiles (complete with version metadata), but until then, setting it via the config is probably the right path. (And I wouldn't even bother with wiring it up in standalone) |
I don't believe this would actually be breaking. Once |
|
I missed the discussion here when I wrote up this comment in #702.
While not breaking, this feels like it'd still be misleading to guests. Putting it in here suggests that customers should use it and key off of it semantically. So the fact that it can't be updated at run-time across a migration to reflect a change is a strong reason for me that this shouldn't be reflecting something that's non-static. In the case of OVMF, I assume that the OVMF version migrates with the guest so that would be consistent? But I don't see how that can be the case at all for Propolis. So I think the question we should be asking is what is how does a customer use this usefully and how do we ensure it reflects the system as a whole that they are using versus a point in time snapshot of the virutalization impl. |
Presently, the BIOS version string in Propolis' SMBIOS tables is hardcoded to a default value. It would be nice to instead use the OVMF version for the BIOS version string. Because Propolis' understanding of bootroms is just a path on the filesystem to some kind of file, it's not aware of the OVMF version, or, indeed, that the bootrom even *is* OVMF (and it could conceivably be anything). Therefore, the bootrom version must be provided externally, such as by the Oxide control plane in the case of `propolis-server`, or by the user when running standalone. This PR adds a config field `bootrom_version` to the TOML config files for `propolis-server` and `propolis-standalone` which can be used to provide a value for the bootrom version string. If the version string is not provided, Propolis will continue to use the current default values. I considered changing the config format to move the `bootrom` path field and `bootrom_version` string field into a `bootrom` table, as in: ```toml [bootrom] path = "/path/to/OVMF_CODE.fd" version = "edk2-stable202402" ``` However, this would break existing configs, and I don't think it's so much nicer than ```toml bootrom = "/path/to/OVMF_CODE.fd" bootrom_version = "edk2-stable202402" ```` to justify breakage. I'm happy to change the format if others disagree. Along with #702, this branch implements the changes described in #701.
Presently, the BIOS version string in Propolis' SMBIOS tables is hardcoded to a default value. It would be nice to instead use the OVMF version for the BIOS version string. Because Propolis' understanding of bootroms is just a path on the filesystem to some kind of file, it's not aware of the OVMF version, or, indeed, that the bootrom even *is* OVMF (and it could conceivably be anything). Therefore, the bootrom version must be provided externally, such as by the Oxide control plane in the case of `propolis-server`, or by the user when running standalone. This PR adds a config field `bootrom_version` to the TOML config files for `propolis-server` and `propolis-standalone` which can be used to provide a value for the bootrom version string. If the version string is not provided, Propolis will continue to use the current default values. I considered changing the config format to move the `bootrom` path field and `bootrom_version` string field into a `bootrom` table, as in: ```toml [bootrom] path = "/path/to/OVMF_CODE.fd" version = "edk2-stable202402" ``` However, this would break existing configs, and I don't think it's so much nicer than ```toml bootrom = "/path/to/OVMF_CODE.fd" bootrom_version = "edk2-stable202402" ```` to justify breakage. I'm happy to change the format if others disagree. Along with #702, this branch implements the changes described in #701.
The version information in the BIOS (type 0) and System (type 1) tables are currently hardcoded. We could instead set them to the OVMF and propolis versions (/commit hash?), respectively.
The text was updated successfully, but these errors were encountered: