Skip to content

Conversation

@nanli1
Copy link
Contributor

@nanli1 nanli1 commented Jan 25, 2026

no sleep after case running would make bridge clean and host get ip process be more stable

Signed-off-by: nanli [email protected]

Also need avocado-framework/avocado-vt#4309 to make case pass

Before fixed:

virtual_network.migrate.migrate_with_bridge_type_interface.start_with_interface.precopy_migration.ovs_bridge FATAL: command execution failed

After fixed:

~# avocado run --vt-type libvirt  --vt-machine-type q35 virtual_network.migrate.migrate_with_bridge_type_interface
RESULTS    : PASS 12 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0

Summary by CodeRabbit

  • Bug Fixes
    • Improved host connection stability by adding a short stabilization delay after bridge cleanup during virtual network teardown.
    • Modified post-migration guest network setup to skip DHCP lease refresh during session initialization.
    • Fixed remote session reliability by tightening the expected shell prompt handling during remote login.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 25, 2026

Walkthrough

This test file modification adds an import of the time module, removes the post-migration DHCP lease refresh command (dhclient) on the guest, introduces a 5-second delay in teardown to stabilize host connectivity after bridge cleanup, and changes two remote_login SSH prompt expectations from the regex r'[$#%]' to the literal #.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title directly addresses the main change: improving stability for migration with bridge interface by adding delays and adjusting SSH expectations.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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

Copy link

@coderabbitai coderabbitai bot left a 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

🤖 Fix all issues with AI agents
In
`@libvirt/tests/src/virtual_network/migrate/migrate_with_bridge_type_interface.py`:
- Around line 211-213: The SSH prompt is hardcoded as '#' in the remote_session
creation (aexpect.remote.remote_login) which breaks for non-root users or custom
prompts; update the prompt argument to use a regex pattern such as r'[$#%]' (or
the same configurable prompt pattern used across other migration tests) so the
login matches common shell prompts and avoids hangs/timeouts.
🧹 Nitpick comments (1)
libvirt/tests/src/virtual_network/migrate/migrate_with_bridge_type_interface.py (1)

175-179: Make the teardown delay configurable or readiness-based.
A fixed 5s sleep can still be flaky (too short) or wasteful (too long). Consider a parameterized wait or polling for connectivity.

♻️ Proposed tweak (parameterize the wait)
-        # Add some waiting time to make host connection stable after clean bridge.
-        time.sleep(5)
+        # Add some waiting time to make host connection stable after clean bridge.
+        bridge_cleanup_wait = int(params.get("bridge_cleanup_wait", "5"))
+        time.sleep(bridge_cleanup_wait)

Comment on lines 211 to 213
remote_session = aexpect.remote.remote_login("ssh", server_ip, "22",
server_user, server_pwd,
r'[$#%]')
'#')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, check if the file exists and examine the relevant lines
if [ -f "libvirt/tests/src/virtual_network/migrate/migrate_with_bridge_type_interface.py" ]; then
  echo "=== File found. Checking lines 205-220 for context ==="
  sed -n '205,220p' libvirt/tests/src/virtual_network/migrate/migrate_with_bridge_type_interface.py
else
  echo "File not found at expected path. Searching for it..."
  find . -name "migrate_with_bridge_type_interface.py" -type f 2>/dev/null
fi

Repository: autotest/tp-libvirt

Length of output: 653


🏁 Script executed:

#!/bin/bash
# Check existing prompt usage patterns across the repo
echo "=== Remote login prompt patterns ==="
rg "remote_login" -A 3 -B 1 --max-count 20

Repository: autotest/tp-libvirt

Length of output: 25885


🏁 Script executed:

#!/bin/bash
# Check for params.get usage patterns in test files
echo "=== Checking params.get patterns in test files ==="
rg "params\.get.*prompt" --type py -C 2

Repository: autotest/tp-libvirt

Length of output: 10806


Fix SSH prompt hardcoding to support non-root users and custom prompts.

The literal '#' prompt fails for non-root users and systems with custom shell prompts, causing login hangs/timeouts. This is inconsistent with other migration test files in the same codebase, which use configurable patterns like r'[$#%]'.

Fix: Use regex pattern matching (lines 211-213)
     remote_session = aexpect.remote.remote_login("ssh", server_ip, "22",
-                                                 server_user, server_pwd,
-                                                 '#')
+                                                 server_user, server_pwd,
+                                                 r'[$#%]')

Alternatively, for test parameterization:

+    remote_prompt = params.get("remote_prompt", r'[$#%]')
     remote_session = aexpect.remote.remote_login("ssh", server_ip, "22",
-                                                 server_user, server_pwd,
-                                                 '#')
+                                                 server_user, server_pwd,
+                                                 remote_prompt)
🤖 Prompt for AI Agents
In
`@libvirt/tests/src/virtual_network/migrate/migrate_with_bridge_type_interface.py`
around lines 211 - 213, The SSH prompt is hardcoded as '#' in the remote_session
creation (aexpect.remote.remote_login) which breaks for non-root users or custom
prompts; update the prompt argument to use a regex pattern such as r'[$#%]' (or
the same configurable prompt pattern used across other migration tests) so the
login matches common shell prompts and avoids hangs/timeouts.

@nanli1 nanli1 force-pushed the fix_migration_with_bridge_iface_failed branch from fd5910d to ed8ed0f Compare January 25, 2026 02:50
no sleep after case running would make bridge clean and host get ip
process be more stable

Signed-off-by: nanli <[email protected]>
@nanli1 nanli1 force-pushed the fix_migration_with_bridge_iface_failed branch from ed8ed0f to 3f4aa04 Compare January 25, 2026 03:00
@nanli1 nanli1 marked this pull request as draft January 27, 2026 01:16
@nanli1 nanli1 marked this pull request as ready for review January 27, 2026 13:23
@nanli1 nanli1 requested a review from cliping January 27, 2026 13:23
@nanli1
Copy link
Contributor Author

nanli1 commented Jan 28, 2026

No need to update this pr, All fixes are in avocado-framework/avocado-vt#4309

@nanli1 nanli1 closed this Jan 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant