Skip to content

migration: Add case about suspend virsh command #6656

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

migration: Add case about suspend virsh command #6656

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions ...migration/destructive_operations_around_live_migration/suspend_virsh_during_migration.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
- migration.destructive_operations_around_live_migration.suspend_virsh_during_migration:
type = suspend_virsh_during_migration
migration_setup = 'yes'
storage_type = 'nfs'
setup_local_nfs = 'yes'
disk_type = "file"
disk_source_protocol = "netfs"
mnt_path_name = ${nfs_mount_dir}
# Console output can only be monitored via virsh console output
only_pty = True
take_regular_screendumps = no
# Extra options to pass after <domain> <desturi>
virsh_migrate_extra = ''
# SSH connection time out
ssh_timeout = 60
# Local URI
virsh_migrate_connect_uri = 'qemu:///system'
virsh_migrate_dest_state = "running"
virsh_migrate_src_state = "shut off"
image_convert = 'no'
server_ip = "${migrate_dest_host}"
server_user = "root"
server_pwd = "${migrate_dest_pwd}"
check_network_accessibility_after_mig = "yes"
migrate_desturi_port = "16509"
migrate_desturi_type = "tcp"
virsh_migrate_desturi = "qemu+tcp://${migrate_dest_host}/system"
status_error = "no"
migrate_speed = "15"
virsh_migrate_dest_state = "running"
virsh_migrate_src_state = "shut off"
start_vm = "yes"
variants:
- with_precopy:
action_during_mig = '[{"func": "MigrationTest().do_cancel", "func_param": "signal.SIGTSTP", "after_event": "migration-iteration"}]'
variants:
- p2p:
virsh_migrate_options = '--live --p2p --verbose'
expected_event_src = ["lifecycle.*Stopped Migrated", "job-completed"]
expected_event_target = ["lifecycle.*: Resumed Migrated"]
- non_p2p:
virsh_migrate_options = '--live --verbose'
unexpected_event_src = ["Stopped Migrated", "job-completed"]
expected_event_target = ["lifecycle.*: Shutdown Finished after host request", "lifecycle.*: Stopped Destroyed"]
virsh_migrate_dest_state = "nonexist"
virsh_migrate_src_state = "running"
status_error = "yes"
- with_postcopy:
action_during_mig = '[{"func": "virsh.migrate_postcopy", "func_param": "'%s' % params.get('migrate_main_vm')", "after_event": "migration-iteration"}, {"func": "MigrationTest().do_cancel", "func_param": "signal.SIGTSTP", "after_event": "Suspended Post-copy"}]'
Copy link

@coderabbitai coderabbitai bot Nov 17, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Fix quoting in action_during_mig for with_postcopy

The action_during_mig value here is single‑quoted as a whole and also contains unescaped single quotes in '%s' and params.get('migrate_main_vm'). In the usual VT cfg workflow (where these values are interpreted as Python expressions/strings), this will very likely lead to a syntax/parse error for this scenario, unlike the simpler precopy case on Line 35.

Consider escaping the inner single quotes (or switching the outer string to double quotes) so that the expression remains valid, for example:

-            action_during_mig = '[{"func": "virsh.migrate_postcopy", "func_param": "'%s' % params.get('migrate_main_vm')", "after_event": "migration-iteration"}, {"func": "MigrationTest().do_cancel", "func_param": "signal.SIGTSTP", "after_event": "Suspended Post-copy"}]'
+            action_during_mig = '[{"func": "virsh.migrate_postcopy", "func_param": "\'%s\' % params.get(\'migrate_main_vm\')", "after_event": "migration-iteration"}, {"func": "MigrationTest().do_cancel", "func_param": "signal.SIGTSTP", "after_event": "Suspended Post-copy"}]'

This keeps the intended '%s' % params.get('migrate_main_vm') expression intact while avoiding conflicts with the outer quotes.

Please rerun the parser/tests for this config after adjusting the quoting to confirm it loads and the postcopy variants execute correctly.

I'll verify this quoting concern by examining the configuration file and understanding how the VT cfg framework processes these strings.


---

🏁 Script executed:

```shell
#!/bin/bash
# Locate and examine the config file
fd "suspend_virsh_during_migration.cfg" --type f -x cat {}

Length of output: 3058

🏁 Script executed:

#!/bin/bash
# Search for similar action_during_mig patterns to understand the pattern
rg -n "action_during_mig\s*=" --type cfg -A 2 -B 2 | head -100

Length of output: 27

🏁 Script executed:

#!/bin/bash
# Look for VT cfg parser code to understand string handling
fd -e py -e txt | xargs grep -l "cfg.*parse\|parse.*cfg" | grep -i parser | head -5

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Check if there are any escape patterns used elsewhere in cfg files
rg "\\\\'" --type cfg | head -20

Length of output: 27


---

🏁 Script executed:

```shell
#!/bin/bash
# Find VT cfg loader/parser code
fd -e py | xargs grep -l "action_during_mig\|load.*cfg\|parse.*cfg" | head -10

Length of output: 501

🏁 Script executed:

#!/bin/bash
# Search for test files that use this config
fd -e py | xargs grep -l "suspend_virsh_during_migration" | head -5

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Look for how action_during_mig values are processed/evaluated
fd -e py | xargs grep -n "action_during_mig" -B 3 -A 3 | head -60

Length of output: 4819

🏁 Script executed:

#!/bin/bash
# Check if there's documentation on VT cfg string quoting rules
fd -e md -e txt -e rst | xargs grep -l "cfg\|quoting\|escape" | head -10

Length of output: 72


---

🏁 Script executed:

```shell
#!/bin/bash
# Search more broadly for test methods that might use this config
fd -e py -path "*/libvirt/tests/src/migration/*" | xargs grep -l "migrate_postcopy\|postcopy" | head -10

Length of output: 168

🏁 Script executed:

#!/bin/bash
# Try to find the main test that uses suspend_virsh_during_migration config
fd -e py | xargs grep -l "destructive_operations_around_live_migration" 2>/dev/null | head -5

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Test the exact syntax error with Python eval
python3 << 'EOF'
# Simulate the problematic line 49 value
line_49_value = '[{"func": "virsh.migrate_postcopy", "func_param": "'%s' % params.get('migrate_main_vm')", "after_event": "migration-iteration"}]'
try:
    result = eval(line_49_value)
    print("SUCCESS: Parsed without error")
except SyntaxError as e:
    print(f"SYNTAX ERROR: {e}")
    print(f"Error at position: {e.offset}")
except Exception as e:
    print(f"OTHER ERROR ({type(e).__name__}): {e}")
EOF

Length of output: 295

🏁 Script executed:

#!/bin/bash
# Test the fixed version with escaped quotes
python3 << 'EOF'
# Test with escaped inner quotes
line_49_fixed = '[{"func": "virsh.migrate_postcopy", "func_param": "\'%s\' % params.get(\'migrate_main_vm\')", "after_event": "migration-iteration"}]'
try:
    result = eval(line_49_fixed)
    print("SUCCESS: Fixed version parses correctly")
    print(f"Result type: {type(result)}, length: {len(result)}")
except Exception as e:
    print(f"ERROR ({type(e).__name__}): {e}")
EOF

Length of output: 78

🏁 Script executed:

#!/bin/bash
# Also test with double quotes for outer string
python3 << 'EOF'
# Test with double quotes for outer string
line_49_double_quotes = "[{\"func\": \"virsh.migrate_postcopy\", \"func_param\": \"'%s' % params.get('migrate_main_vm')\", \"after_event\": \"migration-iteration\"}]"
try:
    result = eval(line_49_double_quotes)
    print("SUCCESS: Double-quoted version parses correctly")
    print(f"Result type: {type(result)}, length: {len(result)}")
except Exception as e:
    print(f"ERROR ({type(e).__name__}): {e}")
EOF

Length of output: 86

Fix critical quoting syntax error in action_during_mig

The action_during_mig value on line 49 will cause a SyntaxError when evaluated. The string is enclosed in single quotes but contains unescaped single quotes in '%s' and params.get('migrate_main_vm'), prematurely terminating the outer string literal. When eval(action_during_mig) is called in provider/migration/migration_base.py:55, this will fail at runtime.

Fix by escaping the inner single quotes:

-            action_during_mig = '[{"func": "virsh.migrate_postcopy", "func_param": "'%s' % params.get('migrate_main_vm')", "after_event": "migration-iteration"}, {"func": "MigrationTest().do_cancel", "func_param": "signal.SIGTSTP", "after_event": "Suspended Post-copy"}]'
+            action_during_mig = '[{"func": "virsh.migrate_postcopy", "func_param": "\'%s\' % params.get(\'migrate_main_vm\')", "after_event": "migration-iteration"}, {"func": "MigrationTest().do_cancel", "func_param": "signal.SIGTSTP", "after_event": "Suspended Post-copy"}]'

Alternatively, switch to double quotes for the outer string to allow the inner single quotes to remain unescaped.

📝 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.

Suggested change
action_during_mig = '[{"func": "virsh.migrate_postcopy", "func_param": "'%s' % params.get('migrate_main_vm')", "after_event": "migration-iteration"}, {"func": "MigrationTest().do_cancel", "func_param": "signal.SIGTSTP", "after_event": "Suspended Post-copy"}]'
action_during_mig = '[{"func": "virsh.migrate_postcopy", "func_param": "\'%s\' % params.get(\'migrate_main_vm\')", "after_event": "migration-iteration"}, {"func": "MigrationTest().do_cancel", "func_param": "signal.SIGTSTP", "after_event": "Suspended Post-copy"}]'
🤖 Prompt for AI Agents 
libvirt/tests/cfg/migration/destructive_operations_around_live_migration/suspend_virsh_during_migration.cfg
around line 49: the assigned literal for action_during_mig uses single quotes
around the whole string but also contains unescaped single quotes inside (e.g.
'%s' and params.get('migrate_main_vm')), which will raise a SyntaxError when
eval() is called; fix by making the outer string use double quotes (or properly
escape the inner single quotes) so the embedded single-quoted '%s' and
params.get(...) stay intact and the value becomes a valid Python expression for
eval; ensure the final string preserves the inner quoting and is syntactically
correct.

variants:
- p2p:
virsh_migrate_options = '--live --p2p --verbose --postcopy'
expected_event_src = ["lifecycle.*Stopped Migrated", "job-completed"]
expected_event_target = ["lifecycle.*: Resumed Post-copy", "lifecycle.*: Resumed Migrated"]
- non_p2p:
virsh_migrate_options = '--live --verbose --postcopy'
expected_event_src = ["lifecycle.*Suspended Post-copy", "lifecycle.*Stopped Migrated", "job-completed"]
expected_event_target = ["lifecycle.*: Resumed Post-copy", "lifecycle.*: Resumed Migrated"]
34 changes: 34 additions & 0 deletions .../migration/destructive_operations_around_live_migration/suspend_virsh_during_migration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#
# Copyright Redhat
#
# SPDX-License-Identifier: GPL-2.0
#
# Author: Liping Cheng <[email protected]>
#
# ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

from provider.migration import base_steps
from provider.migration import migration_base


def run(test, params, env):
"""
This case is to verify that if virsh(management app) is suspended during
migration, migration will succeed.

"""
vm_name = params.get("migrate_main_vm")
vm = env.get_vm(vm_name)
migration_obj = base_steps.MigrationBase(test, vm, params)
virsh_session = None
remote_virsh_session = None

try:
migration_obj.setup_connection()
virsh_session, remote_virsh_session = migration_base.monitor_event(params)
migration_obj.run_migration()
migration_obj.verify_default()
migration_base.check_event_output(params, test, virsh_session, remote_virsh_session)
finally:
migration_obj.cleanup_connection()