flowey: introduce `run-igvm` command #830

justus-camp-microsoft · 2025-02-08T00:26:53Z

The intention of this change is to speed up the inner dev-loop and streamline some things that are currently a little annoying. Namely, this change enables flowey to do what would normally happen when you source build_support/setup_windows_cross.sh and then run openvmm in the same shell.

Trailing args are passed to openvmm - the igvm arg is automatically filled in by the recipe you select.

Example usage:

cargo xflowey run-igvm x64 -- --hv --vtl2 --com3 console -m 4GB --com1 none --vmbus-com1-serial term=wt --vmbus-com2-serial term=wt --net uh:consomme --vmbus-redirect --no-alias-map

jstarks · 2025-02-08T00:32:16Z

flowey/flowey_lib_hvlite/src/_jobs/local_build_igvm.rs

@@ -45,6 +45,7 @@ pub struct Customizations {
 flowey_request! {
    pub struct Params {
        pub artifact_dir: ReadVar<PathBuf>,
+        pub bin_path: Option<WriteVar<PathBuf>>,


Seems like this should just take the place of done, and the existing caller(s) can just throw the resulting buffer away.

I agree, but in implementing it looks to me like this would take a good amount of re-factoring to actually do. You/Daniel might know of a better solution, but to me it seems that to do it this way I would need to do some node restructuring as variables are only available at the NodeCtx layer, which makes doing it this way problematic as we directly dep_on this node in build_igvm.rs which only has access to a PipelineJobCtx.

I'd rather not do the node restructuring as part of this PR unless we feel that it's necessary to be ok with merging this.

jstarks · 2025-02-08T00:35:54Z

flowey/flowey_lib_hvlite/src/init_cross_build.rs

@@ -103,7 +103,83 @@ impl FlowNode for Node {
                    // allowing for compilation of Windows applications from Linux.
                    // For now, just silently continue regardless.
                    // TODO: Detect (and potentially install) these dependencies
-                    (FlowPlatform::Linux(_), target_lexicon::OperatingSystem::Windows) => {}
+                    (FlowPlatform::Linux(_), target_lexicon::OperatingSystem::Windows) => {
+                        let sh = xshell::Shell::new()?;


Can we not just use build_windows_cross, perhaps refactoring it to be able to echo env vars to set? I know we like to do everything in Rust around here, but I don't want to maintain two different ways to do this, and I think the existing script-based approach is more portable to other projects.

For once, I might actually agree with leveraging the existing bash script.

It has a very narrow scope (i.e: only runs on Linux, used to set various env vars, isn't coupled to any other scripts, doesn't export and functions, etc...), and having a single source of truth for this gnarly logic is better than having two separate ones for manual vs. automated use-cases.

+1 to having flowey simply invoke the script with an extra parameter that has it print a list of newline-delimited env var=key lines that flowey can then parse.

I went ahead and added a --print-only option to the script and am now invoking it to set the env vars instead.

jstarks · 2025-02-08T00:36:29Z

flowey/flowey_lib_hvlite/src/init_cross_build.rs

-                    (FlowPlatform::Linux(_), target_lexicon::OperatingSystem::Windows) => {}
+                    (FlowPlatform::Linux(_), target_lexicon::OperatingSystem::Windows) => {
+                        let sh = xshell::Shell::new()?;
+                        let workspace_root = xshell::cmd!(


Flowey already has a mechanism for this.

i.e: take a dep on git_checkout_hvlite_repo

in general, you really can't assume anything about the working directly flowey is running. today, it happens to be in <openvmm-repo>/flowey-out, but if we ever wanted to move it to, say, .cache/flowey_openvmm/flowey-out, this code that implicitly assumes you're running somewhere in the openvmm repo path would fail.

Took a dep on git_checkout_openvmm_repo but wrote some ugly code in the process. Left a comment about a lifetime issue but the control flow of this is also pretty ugly so open to suggestions in restructuring.

jstarks · 2025-02-08T00:37:49Z

flowey/flowey_lib_hvlite/src/init_cross_build.rs

+                        let windows_cross_link = "lld-link-14";
+                        let dll_tool = "llvm-dlltool-14";
+
+                        let vswhere = xshell::cmd!(sh, "wslpath 'C:\\Program Files (x86)\\Microsoft Visual Studio\\Installer\\vswhere.exe'").read()?;


Can we skip all this when the appropriate env vars are already set? Some of these steps take a little time, thanks to vswhere.exe being dog slow for whatever reason.

note that we can also sidestep vswhere by leaning on something like https://github.com/nabijaczleweli/vswhom.rs or https://crates.io/crates/thound (or even jonathan blow's OG C script). I bet it'd be faster to compile + run any of these alternatives than it would be to launch + scrape the output of vswhere, hah

Yes, there are faster ways to do this. E.g., cc-rs knows how to do this quickly as well.

It's tempting to try to use cc-rs or a similar crate instead of the script. But these crates all need to be built targetting Windows, the very thing we can't do until we set up cross compilation.

But yeah, I will probably submit a PR that replaces some of the script's logic with faster logic that assumes a little more about the layout of a VS installation. If it's fast enough, I may just move the detection logic into the compiler wrapper scripts themselves.

Have we given this feedback to the visual studio folks? Lots of workarounds seem to exist (based on this conversation), for the tool that is trying to be the single source of truth for navigating visual studio installs.

Just to circle back to this - do you want me write some logic to check for the variables in flowey/in the script to sidestep this or are you planning on tackling it in a separate PR? Happy to tack it on here, just don't want to duplicate work

mattkur · 2025-02-10T00:54:36Z

I quite appreciate work that goes to making the dev experience better, so thank you for taking the time to make this change! A question for you: what parts of this do you think we can leverage to make it easier to run vmm_tests locally? That's a multi-step experience today, and prone to silent failures & not testing what you think you actually test.

Finally, and perhaps this is bike-shedding, but I'm not sure I fully understand the name. I suppose this is an analog to build-igvm, so we should think of this helper as solely helping to run openhcl cases? Do you also have plans to make this work for HyperV based VMs as well?

Forgive the scope creep implied in these questions. I am not suggesting that you should address, or even have answers to, the questions above. These are the sorts of questions that your PR raised in my mind.

daprilik · 2025-02-10T18:22:45Z

Regarding naming - In the context of the OpenVMM repo, I think its quite reasonable to have a run-igvm to compliment build-igvm, in much the same way stock cargo has cargo build and cargo run. As long as we clearly document its semantics, as well as its inherent limitations (wrt. the sorts of IGVM files OpenVMM can run, and its poor performance, relative to Hyper-V), I think the name is solid.

Regarding Hyper-V compat - I agree that it would be cool if there was a magic flag like --on-hyperv to also run IGVM files on Hyper-V... but given the radically different configuration mechanisms for OpenVMM vs. Hyper-V, one would need to essentially write a "VMM Abstraction Layer" in order to have a sane unified CLI interface.

I'll note that Trevor and John have been building up some very interesting primitives for launching Hyper-V VMs from our Rust infrastructure, so it seems plausible that maybe at some point in the future, we could look into leveraging those primitives to implement that sort of unified, VMM-agnostic helper CLI tool.

Regarding vmm_tests - folks are actively working on fleshing out the petri-based test artifact resolver logic to enable a more streamlined test execution experience, where petri integrates directly with flowey in order to automatically build/download test artifacts seamlessly, as part of standard cargo nextest user experience. i.e: instead of returning an error saying "please run this command to build this test artifact", the test infra would have a mechanism of more seamlessly declaring all necessary dependencies up-front, and then having a "one-click" command to build/download all those dependencies.

The alternative approach, where flowey first pre-downloads a bunch of test artifacts and then invokes cargo nextest, is not particularly viable for local development, as without some kind of "dynamic" understanding of what test artifacts are going to be required to run the tiny subset of tests the user is interested in, flowey would need to build/download all possible test dependencies that might be used, in order to guarantee a similar "one click" experience for users. This might be viable today, given our relatively small amount of dependencies... but its not an approach that will scale as we continue to invest in and expand our test infrastructure here.

By leveraging our existing cargo nextest-based test infrastructure as the entry-point to running vmm_tests, we avoid needing to build / teach folks about a new, OpenHCL-specific API for running tests. Instead, folks can rely on their existing knowledge of how Rust test infrastructure works, as well as referencing standard documentation on https://nexte.st/ for configuring / filtering what test are run.

Also, I'll note that I've actually played around with a flowey-frontend to VMM tests in the past, via my cargo xjob experiments. I quickly ran into all the problems I've outlined above (wrt. needing to build/download "the world" in order to run a single test), and can attest that - while it worked - it wasn't pleasant having to waste so much time waiting for a bunch of useless artifacts to get procured...

justus-camp-microsoft · 2025-02-10T18:48:35Z

Daniel beat me to the punch here - the initial motivation for this actually was for running vmm_tests but after thinking it through I ran into the issues he talked about. You don't actually get much of a speed increase if the flowey action has to rebuild everything all the time so I decided to just go ahead with run-igvm for speeding up the manual testing/code iteration workflow.

I thought of some sort of in-between where you specify which vmm_tests suite you want to run through an arg and can avoid this issue, but I think it's a bit of a brittle approach and isn't very maintainable.

As for Hyper-V, I frankly don't know what it would take to get that working so I'll defer to Daniel's opinion on it.

mattkur · 2025-02-10T20:30:41Z

Great, thanks both for bringing me along in understanding.

justus-camp-microsoft · 2025-02-10T22:42:06Z

flowey/flowey_lib_hvlite/src/init_cross_build.rs

+            let (env_write_var, env_read_var) = if let Some(var) = runtime_env_for_cross {
+                (Some(var.1), Some(var.0))
+            } else {
+                (None, None)
+            };


This is some copilot suggested code to fix a lifetime issue. Is there a better way to get this to compile?

justus-camp-microsoft added 4 commits February 3, 2025 10:06

something sort of close to working

df0ea77

Merge branch 'main' into run_igvm

951547e

working I think?

2c40ede

changes from a quick once-over of the diff

d2b1b8f

justus-camp-microsoft requested review from a team as code owners February 8, 2025 00:26

jstarks reviewed Feb 8, 2025

View reviewed changes

clippy

f522008

jstarks reviewed Feb 8, 2025

View reviewed changes

leverage existing script and add print parameter

122bb18

take a dep on git_checkout_openvmm_repo

157bc0b

justus-camp-microsoft commented Feb 10, 2025

View reviewed changes

justus-camp-microsoft changed the title ~~wip: flowey: introduce run-igvm command~~ flowey: introduce run-igvm command Feb 10, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

flowey: introduce `run-igvm` command #830

flowey: introduce `run-igvm` command #830

justus-camp-microsoft commented Feb 8, 2025 •

edited

Loading

jstarks Feb 8, 2025

justus-camp-microsoft Feb 10, 2025 •

edited

Loading

jstarks Feb 8, 2025

daprilik Feb 8, 2025

justus-camp-microsoft Feb 10, 2025

jstarks Feb 8, 2025

daprilik Feb 8, 2025

justus-camp-microsoft Feb 10, 2025

jstarks Feb 8, 2025

daprilik Feb 8, 2025

jstarks Feb 8, 2025

mattkur Feb 10, 2025

justus-camp-microsoft Feb 10, 2025

mattkur commented Feb 10, 2025

daprilik commented Feb 10, 2025 •

edited

Loading

justus-camp-microsoft commented Feb 10, 2025 •

edited

Loading

mattkur commented Feb 10, 2025 •

edited

Loading

justus-camp-microsoft Feb 10, 2025

flowey: introduce run-igvm command #830

Are you sure you want to change the base?

flowey: introduce run-igvm command #830

Conversation

justus-camp-microsoft commented Feb 8, 2025 • edited Loading

Choose a reason for hiding this comment

justus-camp-microsoft Feb 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattkur commented Feb 10, 2025

daprilik commented Feb 10, 2025 • edited Loading

justus-camp-microsoft commented Feb 10, 2025 • edited Loading

mattkur commented Feb 10, 2025 • edited Loading

Choose a reason for hiding this comment

flowey: introduce `run-igvm` command #830

flowey: introduce `run-igvm` command #830

justus-camp-microsoft commented Feb 8, 2025 •

edited

Loading

justus-camp-microsoft Feb 10, 2025 •

edited

Loading

daprilik commented Feb 10, 2025 •

edited

Loading

justus-camp-microsoft commented Feb 10, 2025 •

edited

Loading

mattkur commented Feb 10, 2025 •

edited

Loading