-
Notifications
You must be signed in to change notification settings - Fork 183
netkvm: add netkvm_poll_mode test case #4353
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Test result: |
653d703 to
9b32bdb
Compare
|
@leidwang Hi Leidong, please help review this poll mode patch. I have completed it now. |
qemu/tests/netkvm_poll_mode.py
Outdated
|
|
||
| :param test: QEMU test object | ||
| :param params: Dictionary with the test parameters | ||
| :param env: Dictionary with test environmen. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| :param env: Dictionary with test environmen. | |
| :param env: Dictionary with test environment. |
qemu/tests/netkvm_poll_mode.py
Outdated
| mapping_output = re.findall(keyword, result) | ||
| if not mapping_output: | ||
| test.error("Can't get %s from traceview", keyword) | ||
| return mapping_output |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to return any value for run()
qemu/tests/cfg/netkvm_poll_mode.cfg
Outdated
| setup_rss_queues = '"*NumRssQueues" %s' | ||
| check_rss_queues = "*NumRssQueues" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rss_queue_name = "*NumRssQueues"
Please update these lines as we discussed.
| queues = ${smp} | ||
| variants: | ||
| - rss_16: | ||
| rss_queues = 16 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rss_queue_value = 1
| restore_image_after_testing = yes | ||
| # this case only tested on prewhql255 or higher version | ||
| cdroms += " virtio" | ||
| required_virtio_win_prewhql = [0.1.255, ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we also need add virtio-win version here
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
WalkthroughAdds a new test configuration Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Areas to review closely:
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (2)
qemu/tests/netkvm_poll_mode.py (2)
17-17: Fix the typo in the docstring (past review comment not addressed).The docstring has a typo: "environmen." should be "environment." This was previously flagged but remains unfixed.
Apply this diff:
- :param env: Dictionary with test environmen. + :param env: Dictionary with test environment.
33-40: Remove the return statement (past review comment not addressed).The
run()function should not return a value, as mentioned in the previous review. In the avocado-vt test framework, test functions communicate results through test.fail(), test.error(), or test.log methods, not return values.Apply this diff:
mapping_output = re.findall(keyword, result) if not mapping_output: test.error("Can't get %s from traceview", keyword) - return mapping_output + test.log.info("Found keyword matches: %s", mapping_output)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
qemu/tests/cfg/netkvm_poll_mode.cfg(1 hunks)qemu/tests/netkvm_poll_mode.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-30T07:12:07.731Z
Learnt from: JinLiul
Repo: autotest/tp-qemu PR: 4362
File: qemu/tests/cfg/sev_direct_kernel_boot.cfg:18-18
Timestamp: 2025-09-30T07:12:07.731Z
Learning: In QEMU SEV testing configurations, the vm_sev_kernel_hashes property works with both SEV and SNP variants as it was moved to common SevCommonProperties in recent QEMU patches, so it can be declared at the top level to apply to both variants.
Applied to files:
qemu/tests/cfg/netkvm_poll_mode.cfg
🔇 Additional comments (4)
qemu/tests/cfg/netkvm_poll_mode.cfg (3)
1-11: LGTM! Version requirement and OS restrictions look correct.The configuration properly restricts the test to Windows 11 and Windows Server 2025 with virtio-net devices, and includes the virtio-win version requirement (prewhql 0.1.255 or higher) as mentioned in the past review feedback.
12-17: Parameter definitions look correct.The RSS and poll mode parameter definitions are properly structured. The quoted format for parameter names (e.g.,
"*RSS","*NumRssQueues") appears intentional for Windows netkvm registry/command usage, and the template with%splaceholder on line 14 correctly supports dynamic RSS queue values.
18-38: Test variants are well-structured.The variants properly define multiple test scenarios covering different combinations of SMP counts, queue counts, and RSS queue settings. The nested variant structure (queue4_smp4 with rss_16/rss_2 sub-variants) and the keyword expectations (enabled/disabled) align well with the test logic in the Python module.
qemu/tests/netkvm_poll_mode.py (1)
19-21: VM setup looks correct.
9b32bdb to
2e2969f
Compare
Poll mode is a must-have feature for Windows Server 2025 and Windows 11. Signed-off-by: wji <[email protected]>
2e2969f to
f5712ab
Compare
Poll mode is a must-have feature for Windows Server 2025 and Windows 11.
ID: 2267
Signed-off-by: Wenkang Ji [email protected]
Summary by CodeRabbit