vmm_cli: add declarative key=value CLI option parsing by jstarks · Pull Request #3981 · microsoft/openvmm

jstarks · 2026-07-20T16:48:28Z

The OpenVMM command line uses a compact per-argument mini-language throughout — things like --disk file:x,ro,on=nvme0 or --memory size=2G,hugepages=on. Each such argument had its own hand-written FromStr that re-implemented the same mechanics: splitting on commas, splitting on =, matching keys, rejecting unknown, duplicate, or empty options, and producing bespoke error messages. This was repetitive, easy to get subtly wrong, and inconsistent between arguments — most visibly, some booleans were bare flags while others required =on/=off, so users had to remember which was which.

This introduces vmm_cli and its companion vmm_cli_derive, a small internal framework for parsing these option strings. #[derive(KeyValueArgs)] generates the parser from an annotated struct, with attributes for renamed keys, defaults, a leading positional token, boolean flags (uniformly accepting bare presence, =on, or =off, plus tri-state Option<bool>), bracket-delimited lists, and a #[kv(flatten)] that merges a shared option struct into several parsers. A companion #[derive(KeyValueGroup)] expresses mutually-exclusive keys that resolve to a single enum variant. Reusable value types (MemorySize, BracketList, BracketRangeList) cover the common non-trivial value formats, and a conformance test pins the derive's guarantees in one place.

The modern comma-separated arguments in openvmm_entry are converted to use the derive. Public CLI types keep their existing shape: where an argument has context-dependent defaults or cross-field validation — disk placement rules, memory hugepage constraints, the vhost-user device-type discrimination — the derive produces a raw options struct and a thin hand-written wrapper applies the remaining semantics, so no downstream consumers change. Arguments built on a different grammar (colon-delimited or discriminated unions such as the serial and network backends) are left as they were.

A side benefit is that boolean options are now uniform: every flag accepts bare, =on, or =off.

The OpenVMM command line uses a compact per-argument mini-language throughout — things like `--disk file:x,ro,on=nvme0` or `--memory size=2G,hugepages=on`. Each such argument had its own hand-written `FromStr` that re-implemented the same mechanics: splitting on commas, splitting on `=`, matching keys, rejecting unknown, duplicate, or empty options, and producing bespoke error messages. This was repetitive, easy to get subtly wrong, and inconsistent between arguments — most visibly, some booleans were bare flags while others required `=on`/`=off`, so users had to remember which was which. This introduces `vmm_cli` and its companion `vmm_cli_derive`, a small internal framework for parsing these option strings. `#[derive(KeyValueArgs)]` generates the parser from an annotated struct, with attributes for renamed keys, defaults, a leading positional token, boolean flags (uniformly accepting bare presence, `=on`, or `=off`, plus tri-state `Option<bool>`), bracket-delimited lists, and a `#[kv(flatten)]` that merges a shared option struct into several parsers. A companion `#[derive(KeyValueGroup)]` expresses mutually-exclusive keys that resolve to a single enum variant. Reusable value types (`MemorySize`, `BracketList`, `BracketRangeList`) cover the common non-trivial value formats, and a conformance test pins the derive's guarantees in one place. The modern comma-separated arguments in openvmm_entry are converted to use the derive. Public CLI types keep their existing shape: where an argument has context-dependent defaults or cross-field validation — disk placement rules, memory hugepage constraints, the vhost-user device-type discrimination — the derive produces a raw options struct and a thin hand-written wrapper applies the remaining semantics, so no downstream consumers change. Arguments built on a different grammar (colon-delimited or discriminated unions such as the serial and network backends) are left as they were. A side benefit is that boolean options are now uniform: every flag accepts bare, `=on`, or `=off`.

Copilot

Pull request overview

This PR introduces a new internal CLI mini-language framework (vmm_cli + vmm_cli_derive) to declaratively parse the existing comma-separated key=value option strings used throughout OpenVMM, and migrates multiple openvmm_entry CLI argument parsers to use the derives for more uniform behavior and less duplicated parsing code.

Changes:

Added vmm_cli (runtime helpers + common value types) and vmm_cli_derive (#[derive(KeyValueArgs)] / #[derive(KeyValueGroup)]) for declarative key/value parsing.
Migrated several openvmm_entry option-string parsers (memory/numa, disk/controller options, PCIe topology options, vfio, vhost-user, etc.) to the new derives with thin validation/wrapping where needed.
Added conformance tests to pin the derive’s expected parsing semantics (unknown/duplicate handling, flags, positional head, bracket lists, flattening, groups).

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
vmm_core/vmm_cli/tests/conformance.rs	New conformance suite covering derive parsing guarantees and edge cases.
vmm_core/vmm_cli/src/lib.rs	Runtime parsing helpers (split/kv parsing, toggles, bracketed lists, `MemorySize`).
vmm_core/vmm_cli/Cargo.toml	New crate definition for `vmm_cli`.
vmm_core/vmm_cli_derive/src/lib.rs	Proc-macro derives implementing `KeyValueArgs` and `KeyValueGroup`.
vmm_core/vmm_cli_derive/Cargo.toml	New proc-macro crate definition for `vmm_cli_derive`.
openvmm/openvmm_entry/src/lib.rs	Adapts config-building to updated parsed CLI types (`MemorySize`, bracket ranges, etc.).
openvmm/openvmm_entry/src/cli_args.rs	Converts multiple `FromStr` parsers to the derive-based approach + validation wrappers.
openvmm/openvmm_entry/Cargo.toml	Adds dependency on `vmm_cli`.
Cargo.toml	Adds workspace path deps for `vmm_cli` and `vmm_cli_derive`.
Cargo.lock	Locks the newly added crates.

Copilot

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.

github-actions · 2026-07-21T10:38:49Z

1 Petri tests failed (1 unstable)

Copilot

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.

github-actions · 2026-07-21T13:49:29Z

1 Petri tests failed (1 unstable)

smalis-msft

I'm surprised this isn't something you can do with just clap derives, but very cool

Copilot

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.

github-actions · 2026-07-22T09:57:56Z

2 Petri tests failed (2 unstable)

Copilot

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

openvmm/openvmm_entry/src/lib.rs:1929

Avoid panicking on user-provided CLI input: expect("NUMA memory size was validated") can still panic if validation is bypassed due to a future refactor or bug. Since this closure already returns anyhow::Result, propagate an error instead.

                                    mem_size: n
                                        .memory
                                        .size
                                        .expect("NUMA memory size was validated")
                                        .0,

Copilot

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

openvmm/openvmm_entry/src/lib.rs:1929

Using expect("NUMA memory size was validated") can still panic on malformed/unexpected CLI input (or future refactors that bypass the earlier validation). Since this path already returns anyhow::Result, prefer propagating an error instead of panicking.

                                    mem_size: n
                                        .memory
                                        .size
                                        .expect("NUMA memory size was validated")
                                        .0,

+    for field in &named.named {
+        let fname = field.ident.as_ref().unwrap();
+        let ty = &field.ty;
+        let fa = parse_field_attr(field)?;
+        let key = fa.key.clone().unwrap_or_else(|| fname.to_string());
+


github-actions · 2026-07-22T14:59:50Z

2 Petri tests failed (1 unstable)

smalis-msft

I'm surprised this isn't something you can do with just clap derives, but very co

Copilot

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

openvmm/openvmm_entry/src/lib.rs:1929

Using .expect("NUMA memory size was validated") will panic on an invariant violation and turn a CLI parse/validation issue into a crash. Since this is ultimately driven by user input, prefer propagating an error here instead of panicking.

                                    mem_size: n
                                        .memory
                                        .size
                                        .expect("NUMA memory size was validated")
                                        .0,

openvmm/openvmm_entry/src/cli_args.rs:1498

parse_memory_config previously accepted the special VMGS_DEFAULT token via parse_memory(). The new bare-shortcut path uses vmm_cli::MemorySize parsing directly, which will now reject --memory VMGS_DEFAULT (a behavior change). If you want to preserve the old acceptance without teaching MemorySize about VMGS_DEFAULT, wrap parse_memory(s)? into MemorySize here.

    // Bare shortcut: `--memory 64G` sets only the size.
    let memory = if !s.contains('=') && !s.contains(',') {
        MemoryCli {
            size: Some(s.parse::<vmm_cli::MemorySize>()?),
            ..Default::default()

+    let mut accum_fields = Vec::new();
+    let mut accept_arms = Vec::new();
+    let mut flatten_accepts = Vec::new();
+    let mut append_keys = Vec::new();
+    let mut finish_fields = Vec::new();
+    let mut positional: Option<(syn::Ident, Type, String)> = None;
+
+    for field in &named.named {
+        let fname = field.ident.as_ref().unwrap();
+        let ty = &field.ty;
+        let fa = parse_field_attr(field)?;
+        validate_field_attr(field, &fa)?;
+        let key = fa.key.clone().unwrap_or_else(|| fname.to_string());
+
+        if fa.positional {


github-actions · 2026-07-22T16:26:33Z

2 Petri tests failed (1 unstable)

Copilot AI review requested due to automatic review settings July 20, 2026 16:48

Copilot started reviewing on behalf of jstarks July 20, 2026 16:49 View session

Copilot AI reviewed Jul 20, 2026

View reviewed changes

Comment thread vmm_core/vmm_cli_derive/src/lib.rs

Comment thread vmm_core/vmm_cli_derive/src/lib.rs

Comment thread openvmm/openvmm_entry/src/cli_args.rs

Comment thread vmm_core/vmm_cli_derive/src/lib.rs

feedback

f58d370

Copilot AI review requested due to automatic review settings July 21, 2026 09:25

jstarks marked this pull request as ready for review July 21, 2026 09:25

jstarks requested a review from a team as a code owner July 21, 2026 09:25

Copilot started reviewing on behalf of jstarks July 21, 2026 09:25 View session

Copilot AI reviewed Jul 21, 2026

View reviewed changes

Comment thread vmm_core/vmm_cli_derive/src/lib.rs

Comment thread openvmm/openvmm_entry/src/cli_args.rs Outdated

Comment thread openvmm/openvmm_entry/src/cli_args.rs

feedback

b22ba7a

Copilot AI review requested due to automatic review settings July 21, 2026 12:03

github-actions Bot added the Guide label Jul 21, 2026

Copilot started reviewing on behalf of jstarks July 21, 2026 12:04 View session

Copilot AI reviewed Jul 21, 2026

View reviewed changes

Comment thread vmm_core/vmm_cli/src/lib.rs Outdated

Comment thread openvmm/openvmm_entry/src/lib.rs Outdated

Comment thread vmm_core/vmm_cli/src/lib.rs Outdated

smalis-msft reviewed Jul 21, 2026

View reviewed changes

jstarks added 2 commits July 22, 2026 00:43

feedback

9353a13

feedback

1621ca7

Copilot AI review requested due to automatic review settings July 22, 2026 08:38

Copilot started reviewing on behalf of jstarks July 22, 2026 08:39 View session

Copilot AI reviewed Jul 22, 2026

View reviewed changes

Comment thread openvmm/openvmm_entry/src/lib.rs

Comment thread vmm_core/vmm_cli/src/lib.rs Outdated

Comment thread vmm_core/vmm_cli_derive/src/lib.rs

feedback

b955132

Copilot AI review requested due to automatic review settings July 22, 2026 13:55

Copilot started reviewing on behalf of jstarks July 22, 2026 13:55 View session

Copilot AI reviewed Jul 22, 2026

View reviewed changes

Comment thread vmm_core/vmm_cli/src/private.rs

Comment thread vmm_core/vmm_cli/src/lib.rs

feedback

0d9085a

Copilot AI review requested due to automatic review settings July 22, 2026 14:49

Copilot started reviewing on behalf of jstarks July 22, 2026 14:50 View session

Copilot AI reviewed Jul 22, 2026

View reviewed changes

Comment thread vmm_core/vmm_cli_derive/src/lib.rs

Comment on lines +149 to +154

for field in &named.named {

let fname = field.ident.as_ref().unwrap();

let ty = &field.ty;

let fa = parse_field_attr(field)?;

let key = fa.key.clone().unwrap_or_else(|| fname.to_string());

smalis-msft approved these changes Jul 22, 2026

View reviewed changes

feedback

d11e485

Copilot AI review requested due to automatic review settings July 22, 2026 15:11

Copilot started reviewing on behalf of jstarks July 22, 2026 15:12 View session

smalis-msft approved these changes Jul 22, 2026

View reviewed changes

jstarks enabled auto-merge (squash) July 22, 2026 15:15

Copilot AI reviewed Jul 22, 2026

View reviewed changes

jstarks merged commit 51f982e into microsoft:main Jul 22, 2026
68 of 69 checks passed

jstarks deleted the vmm_cli branch July 22, 2026 16:10

Uh oh!

Conversation

jstarks commented Jul 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jul 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jul 21, 2026

Uh oh!

smalis-msft left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jul 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

github-actions Bot commented Jul 22, 2026

Uh oh!

smalis-msft left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

github-actions Bot commented Jul 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants