-
Notifications
You must be signed in to change notification settings - Fork 182
qemu_guest_agent: Add test for QGA install without virtio-serial #4403
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?
qemu_guest_agent: Add test for QGA install without virtio-serial #4403
Conversation
WalkthroughAdds a Windows-specific test variant Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
@leidwang please help to review when you're free |
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: 0
🧹 Nitpick comments (2)
qemu/tests/qemu_guest_agent.py (2)
451-512:gagent_check_install_no_devicecleanly covers the no-serial install scenarioThis flow (ensure VM, open session, uninstall if present, resolve MSI path via
get_qga_pkg_pathwhen available, install usinggagent_install_cmd, then assert service status) matches the intent of validating QGA install without a virtio-serial device and relies only on guest-side checks, notself.gagent. The extraget_qga_pkg_pathcall duplicates some work done in the Windowssetuppath but is harmless and keeps the test self-contained.
4681-4682: Windows setup consistently avoidsgagent_createforgagent_serial_type = "none"Mirroring the base class, this guard prevents trying to create a QemuAgent backed by a non-existent serial device in the
no_serialvariant, without impacting existing virtio/ISA flows. If you touch this again, a small helper likeif params.get("gagent_serial_type") != "none":shared between Linux and Windows setups could reduce duplication.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
qemu/tests/cfg/qemu_guest_agent.cfg(2 hunks)qemu/tests/qemu_guest_agent.py(4 hunks)
🔇 Additional comments (4)
qemu/tests/cfg/qemu_guest_agent.cfg (2)
669-672: New Windowsgagent_install_no_devicevariant wiring looks correctThe combination of
only Windows,only no_serial, andgagent_check_type = install_no_devicecleanly targets the new Python handler without affecting existing variants.
696-699:no_serialserial variant is safely isolated to the new testRestricting this variant to
only gagent_install_no_device, settinggagent_serial_type = none, and disablingcheck_vioserensures no other tests run without a GA channel and that this scenario does not expect virtio-serial.qemu/tests/qemu_guest_agent.py (2)
430-431: Guardinggagent_createwhen serial type is"none"Conditionally skipping
gagent_createwhengagent_serial_typeis"none"avoids constructing a QemuAgent without a backing chardev, while preserving previous behaviour for all other values (includingNone).
4429-4431: Skipping baserun_onceforinstall_no_deviceavoids accessingself.gagentBy not calling
QemuGuestAgentTest.run_oncewhengagent_check_type == "install_no_device", this path correctly avoidsgagent_verify()in cases where noQemuAgentwas created (theno_serialscenario), while all other check types retain the original verification.
|
Please add JIRA ID to the PR's description. |
|
Does this PR need to be reviewed by Boqiao? |
Yes, will add him to review and test later. |
Add a new test case `gagent_install_no_device` to verify that 'QEMU Guest Agent (QGA)' can be successfully installed on a VM booted without a virtio-serial device. Changes: - cfg: Add `gagent_install_no_device` test variant and `no_serial` serial variant. - py: - Implement `gagent_check_install_no_device` using direct MSI install command. - Update `setup` to skip agent creation when serial type is `none`. - Update `run_once` to skip verification for `install_no_device` case. Signed-off-by: Dehan Meng <[email protected]>
4f0b173 to
0f6c21b
Compare
|
@fbq815 Could you please help to review when you have a moment? |
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
qemu/tests/cfg/qemu_guest_agent.cfg(2 hunks)qemu/tests/qemu_guest_agent.py(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- qemu/tests/cfg/qemu_guest_agent.cfg
🔇 Additional comments (3)
qemu/tests/qemu_guest_agent.py (3)
430-431: LGTM: Conditional agent creation is correctly gated.The logic appropriately skips guest agent creation when
gagent_serial_typeis"none", which aligns with the test objective to verify QGA installation without a virtio-serial device.
4427-4428: LGTM: Correctly bypasses verification for no-device install test.Skipping the base
run_once(which callsgagent_verify) whengagent_check_typeis"install_no_device"is appropriate, as verification requires a working virtio-serial channel that won't exist in this test scenario.
4678-4679: LGTM: Consistent with base class implementation.The conditional agent creation pattern matches the base class implementation (Lines 430-431), ensuring consistent behavior across Windows and Linux guests.
Add a new test case
gagent_install_no_deviceto verify that 'QEMU Guest Agent (QGA)' can be successfully installed on a VM booted without a virtio-serial device.Changes:
gagent_install_no_devicetest variant andno_serialserial variant.gagent_check_install_no_deviceusing direct MSI install command.setupto skip agent creation when serial type isnone.run_onceto skip verification forinstall_no_devicecase.ID: 4506
Signed-off-by: Dehan Meng [email protected]
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.