Lab 2.3: Fix It — A Good First Issue

Background

Lab 2.2 taught you the plumbing on a throwaway change. Now the change is real: you will find a genuine good first issue in the Firecracker tracker, claim it with good etiquette, reproduce it, scope it tightly, fix it, add or adjust a test, and open a PR that two maintainers can approve. The point is not just to land one fix — it is to learn a repeatable method that generalizes to every good-first-issue class: documentation, error-message clarity, and small validation gaps.

This is a fix-it lab. Because the live issue list changes daily, this lab walks one concrete, plausible example class in full — a misleading pre-boot validation error message — while teaching you to generalize. You will likely fix a different real issue; the method is identical.

Why This Lab Matters for Contributors

• A reproduction you can run is the entry ticket. Maintainers triage by "can I see it?" — an issue you can reproduce on demand is one you can credibly claim and fix.
• The discipline of scoping — changing exactly what the issue describes and nothing more — is what separates a one-round merge from a stalled PR.
• "New functionality needs an integration test" is not optional on Firecracker. Even a one-line error fix usually wants a test that pins the new message so it can't silently regress.

Prerequisites

• Lab 2.2 complete — fork, remotes, branching, sign-off, and the tools/devtool gates are second nature.
• Lab 2.1 complete — you can locate vmm_config, the API server, and the tests/ suite by role.
• gh authenticated (gh auth status) so you can list and claim issues.

gh auth status
gh issue list --repo firecracker-microvm/firecracker --label "good first issue" --state open --limit 5

Step-by-Step Tasks

Step 1: Find a Real Good First Issue

Restrict yourself, at this level, to issues labeled good first issue — curated to be scoped, self-contained, and low-risk. Combine labels to narrow:

# Browser:
#   https://github.com/firecracker-microvm/firecracker/issues?q=is%3Aopen+label%3A%22good+first+issue%22

# gh CLI — list open good first issues, newest first:
gh issue list --repo firecracker-microvm/firecracker \
  --label "good first issue" --state open --limit 30

# Narrow by type when you want a particular class:
gh issue list --repo firecracker-microvm/firecracker \
  --label "good first issue" --label "Type: Bug" --state open

# Inspect one fully, including comments and linked PRs:
gh issue view 4321 --repo firecracker-microvm/firecracker --comments

The label families you saw in the Level 2 index help you read an issue at a glance: Type: (Bug/Enhancement/Documentation), Status: (Awaiting review/Blocked/Parked), Priority:, and Kani (avoid — formal-verification territory, not Level 2).

Step 2: Qualify Before You Claim

A good Level-2 issue passes all of these:

Warning: Read the entire comment thread and the linked-PR list before you do anything. Do not open a PR and then discover two others already did. This is the same discipline as every tracker — the etiquette does not change.

Step 3: Claim It Politely

If it passes the filters and is genuinely unassigned and un-PR'd, comment your intent before you start — one short, specific sentence:

gh issue comment 4321 --repo firecracker-microvm/firecracker \
  --body "I'd like to work on this. Plan: make the \`vcpu_count\` validation error state the allowed range and the offending value, and add an integration test. Will open a PR shortly."

That comment does three things: it signals you to others, it states your scope so a maintainer can correct you before you write code, and it shows you understood the issue. Etiquette is detailed in Community Interaction.

Step 4: Reproduce

You cannot fix what you cannot see. Reproduce the reported behavior on your own build. For our worked example — a misleading error when machine-config is given an out-of-range vcpu_count — drive the API and capture the actual message:

# Start a fresh VMM (from Lab 1.3).
sudo ./build/cargo_target/x86_64-unknown-linux-musl/debug/firecracker \
  --api-sock /tmp/fc.sock &

API=/tmp/fc.sock
# Send an obviously-bad vcpu_count and read the error body verbatim.
curl -s -X PUT --unix-socket "$API" \
  --data '{"vcpu_count": 0, "mem_size_mib": 1024}' \
  http://localhost/machine-config
# -> {"fault_message":"The vCPU number is invalid!"}   <- vague: no value, no range

Write the exact current output into your reproduction notes. The bug here is not that it rejects the value — it should — but that the message is unhelpful: it names neither the offending value nor the valid range. That is a textbook good-first-issue class: an error string that does not help the user fix their input.

Tip: Generalize the reproduction step to the other classes. Docs: run the documented command and show it fails or misleads. Validation gap: send the value that is wrongly accepted and show it boots when it should be rejected. Always end Step 4 with a command anyone can paste to see the bug.

Step 5: Scope and Locate the Code

State the fix in one sentence: "Make the vcpu_count validation error report the offending value and the allowed range, and pin the new message with an integration test." Now find the code by role — never by a remembered line number:

# Where does machine-config validation live?
rg -n 'vcpu_count|VcpuConfig|MachineConfig' src/vmm/src/vmm_config/

# Find the specific error variant and its message.
rg -n 'vCPU number is invalid|InvalidVcpuCount|vcpu' src/vmm/src/vmm_config/

You will land in something like src/vmm/src/vmm_config/machine_config.rs (verify on your branch), where a validate-style function checks vcpu_count and returns an error enum whose Display produces the vague string. Read the surrounding code: what are the real bounds? Find the constant rather than hard-coding a number:

rg -n 'MAX_SUPPORTED_VCPUS|MAX_VCPU|const .*VCPU' src/vmm/src/

Step 6: Make the Fix (one logical change)

Improve the message to state the value and the constraint. The shape of the change (illustrative — match the real error type and constant names on your branch):

diff --git a/src/vmm/src/vmm_config/machine_config.rs b/src/vmm/src/vmm_config/machine_config.rs
--- a/src/vmm/src/vmm_config/machine_config.rs
+++ b/src/vmm/src/vmm_config/machine_config.rs
@@
-    #[error("The vCPU number is invalid!")]
-    InvalidVcpuCount,
+    #[error("The vCPU number is invalid: {0} is outside the supported range 1..={1}.")]
+    InvalidVcpuCount(u8, u8),
@@ fn validate(&self) -> Result<(), MachineConfigError> {
-        if self.vcpu_count == 0 || self.vcpu_count > MAX_SUPPORTED_VCPUS {
-            return Err(MachineConfigError::InvalidVcpuCount);
+        if self.vcpu_count == 0 || self.vcpu_count > MAX_SUPPORTED_VCPUS {
+            return Err(MachineConfigError::InvalidVcpuCount(
+                self.vcpu_count,
+                MAX_SUPPORTED_VCPUS,
+            ));
         }

Resist every urge to also "improve" the neighbouring mem_size_mib message or rename a field. That is a separate issue and a separate PR. One logical change.

Note on compatibility: Changing an error string is user-facing but low-risk; it does not break the API contract (the HTTP status and the fault_message shape are unchanged). Changing an error enum variant's signature is an internal change — confirm nothing outside the crate matches the variant by name in a way your edit breaks (rg -n 'InvalidVcpuCount'). Compatibility judgment grows over the curriculum; see Compatibility.

Step 7: Add or Adjust a Test

This is the step new contributors skip and reviewers always demand. Pin the new behavior so it cannot silently regress. Two levels apply:

Unit test (fast, in-crate) — assert the Display output and the validation path:

#![allow(unused)]
fn main() {
#[test]
fn test_invalid_vcpu_count_message() {
    let cfg = MachineConfig { vcpu_count: 0, mem_size_mib: 1024, ..Default::default() };
    let err = cfg.validate().unwrap_err();
    let msg = err.to_string();
    assert!(msg.contains("0"), "message should name the offending value: {msg}");
    assert!(msg.contains("supported range"), "message should state the range: {msg}");
}
}

Integration test (the suite that actually gates new functionality) — drive the API and assert the error body. Find the right pytest file by role:

rg -n 'machine.config|vcpu_count|machine_config' tests/ | head
ls tests/integration_tests/functional/ | rg -i 'machine|config|api'

Add or extend a case (illustrative; match the suite's fixtures and helpers on your branch):

def test_invalid_vcpu_count_message(uvm_plain):
    """A zero vcpu_count must be rejected with a message naming the value and range."""
    vm = uvm_plain
    vm.spawn()
    resp = vm.api.machine_config.put(vcpu_count=0, mem_size_mib=1024)  # expect failure
    assert resp.status_code == 400
    body = resp.json()["fault_message"]
    assert "0" in body and "supported range" in body, body

Run them:

# Unit tests for the crate you touched:
tools/devtool test -- --test-threads=1 -p vmm machine_config
# The integration case:
tools/devtool test -- integration_tests/functional/test_api.py -k vcpu_count

Tip: Generalize the test to the class. Docs fix: no test, but note in the PR that you ran the corrected command. Validation gap: the integration test asserts the bad config is now rejected (status 400) where before it was accepted.

Step 8: CHANGELOG, Gates, Commit, PR

Now run the Lab 2.2 pipeline for real:

# 1. CHANGELOG entry (user-facing message change -> Changed or Fixed):
#    "- Improved the machine-config vcpu_count validation error to report the
#     offending value and the supported range (#NNNNN)."

# 2. Format and gate.
tools/devtool fmt
tools/devtool checkstyle
tools/devtool checkbuild --all

# 3. Commit, signed, one logical change.
git checkout -b fix/vcpu-count-error-message
git add src/vmm/src/vmm_config/machine_config.rs CHANGELOG.md tests/
git commit -s -m "Report value and range in the vcpu_count validation error"

# 4. Push and open the PR against main, linking the issue.
git push origin fix/vcpu-count-error-message
gh pr create --repo firecracker-microvm/firecracker --base main \
  --title "Report value and range in the vcpu_count validation error" \
  --body "Closes #4321. Improves the machine-config validation error to name the offending vcpu_count and the supported range. Adds a unit test and an integration test pinning the new message."

In the PR body, link the issue (Closes #4321) so it auto-closes on merge, and fill the template honestly — including the "new functionality includes integration tests" box, which you can now check truthfully.

Step 9: Respond to Review to Merge

Expect comments — phrasing of the message, the exact range syntax, whether the test belongs in a different file. Address each by amending the relevant commit and force-pushing your branch (Lab 2.2, Step 10), reply to every thread, and keep main current underneath you. Merge needs ≥2 maintainer approvals; you do not merge your own PR. Be patient and precise.

Implementation Requirements

• A real good first issue, qualified against the Step-2 checklist, claimed with an intent comment.
• A pasteable reproduction that shows the current (wrong/misleading) behavior.
• A one-sentence scope statement and the rg commands that located the code.
• A single-logical-change fix that touches only what the issue describes.
• A test (unit and/or integration) that fails before your fix and passes after.
• A CHANGELOG entry, green tools/devtool fmt/checkstyle/checkbuild --all, a signed commit.
• A PR linking the issue, with the template fully and honestly filled.

Troubleshooting

Every good first issue already has an assignee or a linked PR

Common — they get claimed fast. Widen the net: drop the Type: filter, sort by oldest (gh issue list ... --search "sort:created-asc"), or watch the tracker for a day. Do not claim an assigned one; comment to ask if it's still active only if it's clearly stale (months idle).

I can't reproduce the reported behavior

Re-read the issue for the exact version/config. Build the same way the issue describes (tools/devtool build), match the API calls precisely, and check whether it was already fixed on main (git log --oneline -- <area>). If it's already fixed, say so on the issue — that is a useful contribution.

My fix touches more files than I expected

Stop. Either the issue is bigger than a good-first-issue (re-scope, or pick another), or you are gold-plating. Strip the change back to the one-sentence scope. File a follow-up issue for anything you noticed but weren't asked to fix.

The integration test can't find a fixture or helper

The pytest suite has its own conventions. Read a neighbouring test in the same file to copy its fixture usage (uvm_plain, microvm, the api client). rg -n 'def test_' tests/integration_tests/functional/test_api.py | head shows working examples.

clippy flags my new error variant

A tuple-variant error often triggers lints about unused fields or formatting. Make sure the #[error(...)] format string uses every field ({0}, {1}). Run tools/devtool fmt then checkstyle until clean.

Expected Output

Before the fix, the reproduction shows the vague message:

$ curl -s -X PUT --unix-socket /tmp/fc.sock --data '{"vcpu_count":0,"mem_size_mib":1024}' http://localhost/machine-config
{"fault_message":"The vCPU number is invalid!"}

After the fix and rebuild, it names the value and range:

{"fault_message":"The vCPU number is invalid: 0 is outside the supported range 1..=32."}

And both tests pass:

$ tools/devtool test -- integration_tests/functional/test_api.py -k vcpu_count
... 1 passed ...

(Exact bound 32 and message wording are version-sensitive — verify on your branch.)

Stretch Goals

1. Do a different class. Find and fix a docs good-first-issue (a stale docs/ command) and a validation-gap one (a pre-boot value wrongly accepted). Notice how Steps 4–7 stay the same.
2. Bisect a regression. If your reproduction shows behavior that changed, git bisect to find the commit that introduced it and cite it in the issue — maintainers value this.
3. Pin every error. rg -n '#\[error' src/vmm/src/vmm_config/ and check which messages already name their offending value. File issues for the ones that don't (don't fix them all in one PR).
4. Read a merged good-first-issue PR. Pick one from the closed list (gh pr list --repo firecracker-microvm/firecracker --state merged --label "good first issue") and study its commit structure, test, and CHANGELOG line.

Validation / Self-check

You are done when you can answer these without notes:

1. What four things must be true before you claim a good first issue?
2. What do you write in your claim comment, and why before you start coding?
3. Why must Step 4 (reproduce) end with a command anyone can paste?
4. Why is "one logical change" the rule, and what do you do with the unrelated nit you noticed?
5. For an error-message fix, what does the test assert, and why a test at all for a one-line change?
6. Which subsection of the CHANGELOG does a user-facing message improvement go under?
7. How many approvals merge your PR, and who performs the merge?

Next: Lab 2.4 — Review It: Spot the Flaws in a PR.