pvpanic: Skip QMP event check for i440fx machine type

[heywji]

Added skip_qmp_check parameter to differentiate between i440fx (no QMP event) and Q35 (with QMP event).

ID: 4592

Signed-off-by: Wenkang Ji [email protected]

Summary by CodeRabbit

• Tests
  • Added configuration option to skip QMP panic event checks for specific guest types, while preserving existing validation behavior when the flag is not enabled.

[coderabbitai]

Walkthrough

The changes introduce a new conditional configuration flag skip_qmp_check to the pvpanic test suite. This flag allows bypassing the QMP panic event check for specific guest types (i440fx) while maintaining existing behavior for other configurations.

Changes

Cohort / File(s)	Summary
Configuration Update qemu/tests/cfg/pvpanic_event_check.cfg	Added skip_qmp_check = yes flag under the i440fx section to conditionally bypass QMP checks for that guest type
Conditional Logic qemu/tests/pvpanic.py	Introduced skip_qmp_check parameter extracted from config; wrapped existing QMP event check in conditional branch to skip when flag is enabled

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

The changes are minimal and follow a straightforward pattern: a single configuration flag with corresponding conditional logic. No complex control flow, error handling modifications, or multi-file interdependencies are present. Review focuses on verifying the conditional branch correctly implements the skip behavior and the config flag integrates properly.

Poem

🐰 A panic check that skips with grace,
The i440fx finds its special place,
With one small flag, so clean and bright,
The QMP dance pauses from sight,
Config and code in harmony play,
Tests run smoother, hip-hop hooray! 🎉

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The PR title "pvpanic: Skip QMP event check for i440fx machine type" accurately describes the primary objective and changes in the pull request. The changeset introduces a skip_qmp_check parameter in the configuration file and implements conditional logic in the test code to bypass QMP event validation when running on the i440fx machine type. The title is concise, clear, and specific enough that a teammate scanning history would immediately understand the purpose of this change without needing to read the full changeset details.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

qemu/tests/pvpanic.py (1)

176-184: Consider making the log message dynamic and verify the skip behavior is documented.

The conditional logic is sound and preserves original behavior when skip_qmp_check is False. However:

1. The log message on line 177 hardcodes "i440fx machine type", but skip_qmp_check is a generic flag that could theoretically be used for other machine types. Consider making it dynamic.
2. When skipping the QMP check, the test essentially passes after triggering the crash without verifying any panic-related behavior. Ensure this is the expected behavior for i440fx and is properly documented.

Apply this diff to make the log message more generic:

-        test.log.info("Skipping QMP event check for i440fx machine type")
+        test.log.info("Skipping QMP event check as per configuration")

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 02fbc5f and f93961d.

📒 Files selected for processing (2)

• qemu/tests/cfg/pvpanic_event_check.cfg (1 hunks)
• qemu/tests/pvpanic.py (2 hunks)

🔇 Additional comments (2)

qemu/tests/pvpanic.py (1)

136-136: LGTM!

The parameter retrieval is correct, with an appropriate default value of "no".

qemu/tests/cfg/pvpanic_event_check.cfg (1)

16-16: LGTM!

The configuration change correctly adds the skip_qmp_check flag for the i440fx machine type, while Q35 retains the default behavior (checking for QMP events).