iommu_device_lifecycle: Recreate serial console after reboot/reset #6645
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request modifies the test file for iommu device lifecycle handling. It replaces the VM reset and reboot flow implementations by removing direct virsh command invocations and login prompt verification. Instead, the code now consistently closes the existing serial session, performs cleanup and recreation of the serial console, and re-establishes a new serial login session after reset or reboot operations. Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libvirt/tests/src/sriov/vIOMMU/iommu_device_lifecycle.py(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: hholoubk
Repo: autotest/tp-libvirt PR: 6579
File: libvirt/tests/src/sriov/vIOMMU/iommu_device_lifecycle.py:95-97
Timestamp: 2025-09-24T08:01:27.899Z
Learning: In the libvirt test framework used in tp-libvirt, VM cleanup including destroying running VMs is handled by the teardown method (test_obj.teardown_iommu_test()) called in the finally block, so explicit VM destroy calls on timeout are not necessary according to the maintainers.
📚 Learning: 2025-09-24T08:01:27.899Z
Learnt from: hholoubk
Repo: autotest/tp-libvirt PR: 6579
File: libvirt/tests/src/sriov/vIOMMU/iommu_device_lifecycle.py:95-97
Timestamp: 2025-09-24T08:01:27.899Z
Learning: In the libvirt test framework used in tp-libvirt, VM cleanup including destroying running VMs is handled by the teardown method (test_obj.teardown_iommu_test()) called in the finally block, so explicit VM destroy calls on timeout are not necessary according to the maintainers.
Applied to files:
libvirt/tests/src/sriov/vIOMMU/iommu_device_lifecycle.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Python 3.11
- GitHub Check: Python 3.12
- GitHub Check: Python 3.8
- GitHub Check: Python 3.9
| test.log.info("TEST_STEP: Reset the VM.") | ||
| session = vm.wait_for_serial_login( | ||
| timeout=int(params.get('login_timeout'))) | ||
| virsh.reset(vm.name, **VIRSH_ARGS) | ||
| _match, _text = session.read_until_last_line_matches( | ||
| [r"[Ll]ogin:\s*"], timeout=240, internal_timeout=0.5) | ||
| session.close() | ||
| vm.cleanup_serial_console() | ||
| vm.create_serial_console() | ||
| session = vm.wait_for_serial_login( | ||
| timeout=int(params.get('login_timeout'))) | ||
| session.close() |
There was a problem hiding this comment.
Critical: Missing VM reset operation.
The test logs "Reset the VM" but no actual reset command is executed. The current code only recreates the serial console without performing a VM reset operation. The original virsh.reset(vm.name, **VIRSH_ARGS) call appears to have been removed.
The reset should be performed after the initial login and before recreating the console:
if test_scenario == "reset":
test.log.info("TEST_STEP: Reset the VM.")
session = vm.wait_for_serial_login(
timeout=int(params.get('login_timeout')))
session.close()
+ virsh.reset(vm.name, **VIRSH_ARGS)
vm.cleanup_serial_console()
vm.create_serial_console()
session = vm.wait_for_serial_login(
timeout=int(params.get('login_timeout')))
session.close()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test.log.info("TEST_STEP: Reset the VM.") | |
| session = vm.wait_for_serial_login( | |
| timeout=int(params.get('login_timeout'))) | |
| virsh.reset(vm.name, **VIRSH_ARGS) | |
| _match, _text = session.read_until_last_line_matches( | |
| [r"[Ll]ogin:\s*"], timeout=240, internal_timeout=0.5) | |
| session.close() | |
| vm.cleanup_serial_console() | |
| vm.create_serial_console() | |
| session = vm.wait_for_serial_login( | |
| timeout=int(params.get('login_timeout'))) | |
| session.close() | |
| test.log.info("TEST_STEP: Reset the VM.") | |
| session = vm.wait_for_serial_login( | |
| timeout=int(params.get('login_timeout'))) | |
| session.close() | |
| virsh.reset(vm.name, **VIRSH_ARGS) | |
| vm.cleanup_serial_console() | |
| vm.create_serial_console() | |
| session = vm.wait_for_serial_login( | |
| timeout=int(params.get('login_timeout'))) | |
| session.close() |
🤖 Prompt for AI Agents
In libvirt/tests/src/sriov/vIOMMU/iommu_device_lifecycle.py around lines 64 to
72, the code logs "Reset the VM" but never executes the actual VM reset; restore
the removed virsh.reset(vm.name, **VIRSH_ARGS) call so the VM is reset after the
first serial login and before cleaning/recreating the serial console. Ensure
VIRSH_ARGS is available in scope (import or reference the existing constant) and
call virsh.reset(vm.name, **VIRSH_ARGS) between session.close() and
vm.cleanup_serial_console().
| for _ in range(int(params.get('loop_time', '5'))): | ||
| test.log.info("TEST_STEP: Reboot the VM.") | ||
| session = vm.wait_for_serial_login( | ||
| timeout=int(params.get('login_timeout'))) | ||
| session.sendline(params.get("reboot_command")) | ||
| _match, _text = session.read_until_last_line_matches( | ||
| [r"[Ll]ogin:\s*"], timeout=240, internal_timeout=0.5) | ||
| session.close() | ||
| vm.cleanup_serial_console() | ||
| vm.create_serial_console() | ||
| session = vm.wait_for_serial_login(timeout=int(params.get('login_timeout'))) | ||
| session.close() |
There was a problem hiding this comment.
Critical: Missing VM reboot operation.
The test logs "Reboot the VM" but no actual reboot command is executed. The current code only recreates the serial console without performing a VM reboot. The loop runs multiple times without actually rebooting the VM between iterations.
The reboot should be triggered after the initial login:
for _ in range(int(params.get('loop_time', '5'))):
test.log.info("TEST_STEP: Reboot the VM.")
session = vm.wait_for_serial_login(
timeout=int(params.get('login_timeout')))
+ session.cmd("reboot")
session.close()
vm.cleanup_serial_console()
vm.create_serial_console()
session = vm.wait_for_serial_login(timeout=int(params.get('login_timeout')))
session.close()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for _ in range(int(params.get('loop_time', '5'))): | |
| test.log.info("TEST_STEP: Reboot the VM.") | |
| session = vm.wait_for_serial_login( | |
| timeout=int(params.get('login_timeout'))) | |
| session.sendline(params.get("reboot_command")) | |
| _match, _text = session.read_until_last_line_matches( | |
| [r"[Ll]ogin:\s*"], timeout=240, internal_timeout=0.5) | |
| session.close() | |
| vm.cleanup_serial_console() | |
| vm.create_serial_console() | |
| session = vm.wait_for_serial_login(timeout=int(params.get('login_timeout'))) | |
| session.close() | |
| for _ in range(int(params.get('loop_time', '5'))): | |
| test.log.info("TEST_STEP: Reboot the VM.") | |
| session = vm.wait_for_serial_login( | |
| timeout=int(params.get('login_timeout'))) | |
| session.cmd("reboot") | |
| session.close() | |
| vm.cleanup_serial_console() | |
| vm.create_serial_console() | |
| session = vm.wait_for_serial_login(timeout=int(params.get('login_timeout'))) | |
| session.close() |
🤖 Prompt for AI Agents
In libvirt/tests/src/sriov/vIOMMU/iommu_device_lifecycle.py around lines 74 to
82, the loop logs "Reboot the VM" but never actually reboots; after the initial
login and session.close() insert a vm.reboot() call so the VM is restarted
before cleaning and recreating the serial console, then keep the subsequent
vm.cleanup_serial_console(), vm.create_serial_console(), and the
wait_for_serial_login() call to verify the VM came back up.
|
Before fix After fix |
…se wait_for_serial_login instead of regex to fix login prompt timeouts Committer: Bolatbek Issakh <[email protected]>
BulaYoungR
force-pushed
the
device_lifecycle_fix
branch
from
e88f413 to
3ead9cf
Compare
1 participant
Use wait_for_serial_login instead of regex to fix login prompt timeouts
Committer: Bolatbek Issakh [email protected]
Summary by CodeRabbit
Bug Fixes
Tests