Skip to content

Conversation

@smitterl
Copy link
Contributor

@smitterl smitterl commented Dec 5, 2025

Several test cases fail because the migration finishes before checks are executed. As usual, slow down these migrations with the bandwidth parameter.

We might have solved this, too, by setting these values downstream only with string replacement but this can lead to conflicts per test case as we only substitute lines.

Summary by CodeRabbit

  • Tests
    • Extended migration test coverage for s390-virtio architecture across multiple scenarios including async operations, destructive operations, memory copy modes, performance tuning, resource limits, and postcopy recovery.
    • Added bandwidth configuration options to migration tests for improved performance and resource limit testing on the s390-virtio architecture.

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

@coderabbitai
Copy link

coderabbitai bot commented Dec 5, 2025

Walkthrough

This pull request modifies 18+ libvirt test configuration files across the migration directory. The changes consistently add s390-virtio variant entries to existing migration test configurations, introducing virsh_migrate_options with bandwidth settings under both p2p and non_p2p variants. Bandwidth values range from 50 to 100 depending on the specific test. Some files additionally modify migrate_speed or virsh_migrate_extra fields for s390-virtio-specific test scenarios. All changes are additive and configuration-only, without altering existing variant logic.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'migration: slow down migration on s390x' accurately reflects the main objective of the PR, which is to slow down migration tests on s390x by adding bandwidth parameters across multiple migration test configuration files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6178376 and b0b4de1.

📒 Files selected for processing (19)
  • libvirt/tests/cfg/migration/async_ops/query_info_during_migration.cfg
  • libvirt/tests/cfg/migration/destructive_operations_around_live_migration/kill_qemu_during_performphase.cfg
  • libvirt/tests/cfg/migration/destructive_operations_around_live_migration/kill_virtiofsd_during_performphase.cfg
  • libvirt/tests/cfg/migration/memory_copy_mode/postcopy.cfg
  • libvirt/tests/cfg/migration/memory_copy_mode/precopy.cfg
  • libvirt/tests/cfg/migration/migration_performance_tuning/migration_maxdowntime.cfg
  • libvirt/tests/cfg/migration/migration_performance_tuning/migration_memory_compression.cfg
  • libvirt/tests/cfg/migration/migration_performance_tuning/migration_parallel_connections.cfg
  • libvirt/tests/cfg/migration/migration_performance_tuning/migration_precopy_bandwidth.cfg
  • libvirt/tests/cfg/migration/migration_performance_tuning/migration_timeout_action.cfg
  • libvirt/tests/cfg/migration/migration_performance_tuning/migration_vm_cpu_auto_converge.cfg
  • libvirt/tests/cfg/migration/migration_performance_tuning/migration_zerocopy.cfg
  • libvirt/tests/cfg/migration/migration_performance_tuning/migration_zerocopy_abort.cfg
  • libvirt/tests/cfg/migration/migration_performance_tuning/migration_zerocopy_unsupported_feature_combinations.cfg
  • libvirt/tests/cfg/migration/migration_performance_tuning/parallel_migration_set_connection_num_without_enabing_parallel_capability.cfg
  • libvirt/tests/cfg/migration/migration_resource_limit/migration_postcopy_bandwidth.cfg
  • libvirt/tests/cfg/migration/migration_resource_limit/migration_precopy_and_postcopy_bandwidth.cfg
  • libvirt/tests/cfg/migration/pause_postcopy_migration_and_recover/no_paused_during_recover_migration.cfg
  • libvirt/tests/cfg/migration/pause_postcopy_migration_and_recover/pause_and_disruptive_and_recover.cfg
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: qiankehan
Repo: autotest/tp-libvirt PR: 6652
File: libvirt/tests/cfg/virtual_network/passt/passt_function.cfg:106-106
Timestamp: 2025-11-24T10:44:47.801Z
Learning: In libvirt/tests/cfg/virtual_network/passt/passt_function.cfg, passt test cases have only one interface, so running `dhcpcd` without specifying an interface is safe and doesn't cause network disruption.
📚 Learning: 2025-12-12T10:00:09.383Z
Learnt from: yanglei-rh
Repo: autotest/tp-libvirt PR: 6675
File: libvirt/tests/cfg/virtual_network/qemu/nic_bonding.cfg:16-16
Timestamp: 2025-12-12T10:00:09.383Z
Learning: In tp-libvirt test config files (*.cfg under libvirt/tests/cfg), the cartesian config parser supports variable substitution using the ${variable_name} syntax. Variables defined in the cfg (e.g., filesize = 4000) are substituted into strings that reference them (e.g., count=${filesize}) before the Python test code receives the parameters. Treat this as a general behavior for these cfg files: define variables early, reference them with ${var}, and verify substitutions in tests to avoid hard-coded literals.

Applied to files:

  • libvirt/tests/cfg/migration/migration_resource_limit/migration_postcopy_bandwidth.cfg
  • libvirt/tests/cfg/migration/migration_performance_tuning/migration_zerocopy.cfg
  • libvirt/tests/cfg/migration/memory_copy_mode/postcopy.cfg
  • libvirt/tests/cfg/migration/migration_resource_limit/migration_precopy_and_postcopy_bandwidth.cfg
  • libvirt/tests/cfg/migration/async_ops/query_info_during_migration.cfg
  • libvirt/tests/cfg/migration/migration_performance_tuning/migration_zerocopy_unsupported_feature_combinations.cfg
  • libvirt/tests/cfg/migration/migration_performance_tuning/migration_memory_compression.cfg
  • libvirt/tests/cfg/migration/migration_performance_tuning/migration_parallel_connections.cfg
  • libvirt/tests/cfg/migration/migration_performance_tuning/migration_precopy_bandwidth.cfg
  • libvirt/tests/cfg/migration/migration_performance_tuning/migration_zerocopy_abort.cfg
  • libvirt/tests/cfg/migration/memory_copy_mode/precopy.cfg
  • libvirt/tests/cfg/migration/migration_performance_tuning/parallel_migration_set_connection_num_without_enabing_parallel_capability.cfg
  • libvirt/tests/cfg/migration/destructive_operations_around_live_migration/kill_qemu_during_performphase.cfg
  • libvirt/tests/cfg/migration/migration_performance_tuning/migration_vm_cpu_auto_converge.cfg
  • libvirt/tests/cfg/migration/pause_postcopy_migration_and_recover/no_paused_during_recover_migration.cfg
  • libvirt/tests/cfg/migration/migration_performance_tuning/migration_timeout_action.cfg
  • libvirt/tests/cfg/migration/destructive_operations_around_live_migration/kill_virtiofsd_during_performphase.cfg
  • libvirt/tests/cfg/migration/migration_performance_tuning/migration_maxdowntime.cfg
  • libvirt/tests/cfg/migration/pause_postcopy_migration_and_recover/pause_and_disruptive_and_recover.cfg
📚 Learning: 2025-11-24T10:44:47.801Z
Learnt from: qiankehan
Repo: autotest/tp-libvirt PR: 6652
File: libvirt/tests/cfg/virtual_network/passt/passt_function.cfg:106-106
Timestamp: 2025-11-24T10:44:47.801Z
Learning: In libvirt/tests/cfg/virtual_network/passt/passt_function.cfg, passt test cases have only one interface, so running `dhcpcd` without specifying an interface is safe and doesn't cause network disruption.

Applied to files:

  • libvirt/tests/cfg/migration/migration_performance_tuning/parallel_migration_set_connection_num_without_enabing_parallel_capability.cfg
⏰ 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.8
  • GitHub Check: Python 3.9
  • GitHub Check: Python 3.11
  • GitHub Check: Python 3.12
🔇 Additional comments (22)
libvirt/tests/cfg/migration/memory_copy_mode/postcopy.cfg (1)

42-47: LGTM – proper s390-virtio bandwidth configuration.

The addition of s390-virtio variants with --bandwidth 50 is correctly nested under both p2p and non_p2p migration modes. Configuration syntax is correct.

libvirt/tests/cfg/migration/destructive_operations_around_live_migration/kill_virtiofsd_during_performphase.cfg (1)

41-42: Verify alignment of migrate_speed versus --bandwidth approaches across files.

This file uses migrate_speed = "50" to slow down s390-virtio migration, while most other files in this PR use --bandwidth 50 in virsh_migrate_options. Confirm that both approaches are appropriate for their respective test contexts and that they work as intended on s390x.

libvirt/tests/cfg/migration/migration_performance_tuning/parallel_migration_set_connection_num_without_enabing_parallel_capability.cfg (1)

38-43: LGTM – consistent s390-virtio bandwidth configuration.

Properly adds --bandwidth 50 to migration options for s390-virtio under both p2p and non_p2p variants, consistent with other performance tuning test configurations.

libvirt/tests/cfg/migration/async_ops/query_info_during_migration.cfg (1)

38-43: Clarify the choice of --bandwidth 10 for async operations.

This file uses --bandwidth 10 for s390-virtio variants, while most other migration test configs in this PR use --bandwidth 50. Given that this test monitors query operations during migration, the lower bandwidth may be intentional to ensure async queries complete before migration finishes. Confirm this is the intended differentiation and document the rationale if it differs from other migration scenarios.

libvirt/tests/cfg/migration/migration_performance_tuning/migration_parallel_connections.cfg (1)

32-37: LGTM – consistent s390-virtio bandwidth configuration.

Properly adds --bandwidth 50 to both p2p and non_p2p migration variants for s390-virtio, aligning with the parallel connections performance tuning configuration pattern.

libvirt/tests/cfg/migration/migration_performance_tuning/migration_memory_compression.cfg (1)

35-40: LGTM – proper s390-virtio bandwidth configuration for memory compression tests.

Adds --bandwidth 50 consistently to both p2p and non_p2p variants, appropriate for memory compression migration scenarios where bandwidth control is necessary to avoid completion before postcopy checks.

libvirt/tests/cfg/migration/migration_performance_tuning/migration_zerocopy.cfg (1)

41-46: Verify interaction between --bandwidth 50 and set_migrate_speed_to_high action.

The s390-virtio variants add --bandwidth 50 to control migration speed, but line 34 specifies an action_during_mig that calls set_migrate_speed_to_high after the first iteration. Confirm that this action doesn't unintentionally override the s390-virtio bandwidth constraint for zerocopy tests.

libvirt/tests/cfg/migration/migration_performance_tuning/migration_zerocopy_unsupported_feature_combinations.cfg (1)

33-38: LGTM – proper s390-virtio bandwidth configuration for unsupported feature combination tests.

Adds --bandwidth 50 to both variants, appropriately controlling migration speed even for tests that expect failures (status_error="yes").

libvirt/tests/cfg/migration/migration_resource_limit/migration_postcopy_bandwidth.cfg (1)

44-45: Consistent s390-virtio bandwidth addition.

The new s390-virtio sub-variants correctly extend virsh_migrate_options with --bandwidth 50 under both p2p and non_p2p paths. Configuration syntax is valid and follows the cartesian config parser conventions.

Also applies to: 48-49

libvirt/tests/cfg/migration/memory_copy_mode/precopy.cfg (1)

42-43: Consistent s390-virtio bandwidth addition.

The new s390-virtio sub-variants correctly extend virsh_migrate_options with --bandwidth 50 under both p2p and non_p2p paths. Configuration syntax is valid and follows the cartesian config parser conventions.

Also applies to: 47-48

libvirt/tests/cfg/migration/migration_performance_tuning/migration_vm_cpu_auto_converge.cfg (1)

39-40: Consistent s390-virtio bandwidth addition.

The new s390-virtio sub-variants correctly extend virsh_migrate_options with --bandwidth 50 under both p2p and non_p2p paths. Configuration syntax is valid and follows the cartesian config parser conventions.

Also applies to: 43-44

libvirt/tests/cfg/migration/migration_performance_tuning/migration_zerocopy_abort.cfg (1)

39-40: Consistent s390-virtio bandwidth addition.

The new s390-virtio sub-variants correctly extend virsh_migrate_options with --bandwidth 50 under both p2p and non_p2p paths. Configuration syntax is valid and follows the cartesian config parser conventions.

Also applies to: 43-44

libvirt/tests/cfg/migration/migration_performance_tuning/migration_timeout_action.cfg (1)

38-39: Consistent s390-virtio bandwidth addition.

The new s390-virtio sub-variants correctly extend virsh_migrate_options with --bandwidth 50 under both p2p and non_p2p paths. Configuration syntax is valid and follows the cartesian config parser conventions.

Also applies to: 42-43

libvirt/tests/cfg/migration/migration_resource_limit/migration_precopy_and_postcopy_bandwidth.cfg (2)

48-49: Consistent s390-virtio bandwidth addition.

The new s390-virtio sub-variants correctly extend virsh_migrate_options with --bandwidth 50 under both p2p and non_p2p paths. Configuration syntax is valid and follows the cartesian config parser conventions.

Also applies to: 52-53


61-62: Correct removal of redundant bandwidth option for s390-virtio.

For the set_bandwidth_when_initiating_live_migration test case, the s390-virtio override correctly removes the redundant --bandwidth ${precopy_bandwidth} from postcopy_options. Bandwidth is already controlled via virsh_migrate_options (--bandwidth 50), so this override prevents option duplication and aligns with the PR objective to remove unnecessary options.

libvirt/tests/cfg/migration/migration_performance_tuning/migration_precopy_bandwidth.cfg (3)

41-42: Consistent s390-virtio bandwidth addition.

The new s390-virtio sub-variants correctly extend virsh_migrate_options with --bandwidth 50 under both p2p and non_p2p paths. Configuration syntax is valid and follows the cartesian config parser conventions.

Also applies to: 45-46


55-56: Correct addition of s390-virtio-specific validation for set_bandwidth_when_vm_shutoff.

The s390-virtio override adds a variant-specific check_str_local_log to validate the expected bandwidth value (9223372036853727232) for the set_bandwidth_when_vm_shutoff test case. This aligns with the PR objective to fix bandwidth value validation for s390-virtio in this scenario.


61-63: Correct removal of redundant bandwidth option and addition of validation for set_bandwidth_when_initiating_live_migration.

The s390-virtio override for the set_bandwidth_when_initiating_live_migration test case:

  1. Clears virsh_migrate_extra to remove the redundant --bandwidth ${precopy_bandwidth} option (bandwidth is already controlled via virsh_migrate_options).
  2. Adds s390-virtio-specific check_str_local_log to validate the expected bandwidth value (9223372036853727232).

These changes prevent option duplication and ensure proper validation, aligning with the PR objective to remove unnecessary options and fix bandwidth handling for s390-virtio.

libvirt/tests/cfg/migration/migration_performance_tuning/migration_maxdowntime.cfg (1)

39-40: Consistent s390-virtio bandwidth addition.

The new s390-virtio sub-variants correctly extend virsh_migrate_options with --bandwidth 50 under both p2p and non_p2p paths. Configuration syntax is valid and follows the cartesian config parser conventions.

Also applies to: 43-44

libvirt/tests/cfg/migration/destructive_operations_around_live_migration/kill_qemu_during_performphase.cfg (1)

52-54: Syntax and structure look good.

The s390-virtio variant overrides are properly formatted and nested. Bandwidth values (50–100) are reduced from the baseline 1000, which aligns with the goal of slowing migrations on s390x. The approach of overriding virsh_migrate_extra is appropriate here.

Also applies to: 62-64, 72-74

libvirt/tests/cfg/migration/pause_postcopy_migration_and_recover/pause_and_disruptive_and_recover.cfg (1)

39-40: Variant format and bandwidth settings are correct.

The s390-virtio variants are properly nested under both p2p and non_p2p parent variants. Bandwidth value of 100 is consistent across both migration modes for this test.

Also applies to: 43-44

libvirt/tests/cfg/migration/pause_postcopy_migration_and_recover/no_paused_during_recover_migration.cfg (1)

32-33: Variant structure and bandwidth configuration look correct.

The s390-virtio variants are properly nested under both p2p and non_p2p. Bandwidth value of 50 is consistently applied across both migration modes for this test.

Also applies to: 36-37


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

@smitterl smitterl force-pushed the slow_down_migration_s390x branch from 3651e2f to 3b29d42 Compare December 5, 2025 16:19
@smitterl smitterl force-pushed the slow_down_migration_s390x branch 4 times, most recently from ce0b076 to e00bd62 Compare December 24, 2025 12:13
On our s390x systems some checks fail consistently because the migration finishes before the check is started. Slow the migration down as usual via bandwidth parameter.

Given the additional non-default bandwidth,
migration_precopy_bandwidth.set_bandwidth_when_vm_shutoff will test for the wrong bandwidth setup.
Fix the value.
migration_precopy_bandwidth.set_bandwidth_when_initiating_live_migration similarly will use another
bandwidth parameter and the extra option is unnecessary. Fix the values.
On our s390x systems some checks fail consistently because the migration finishes before the check is started. Slow the migration down as usual via bandwidth parameter.
Some test case already sets that parameter for precopy explicitly so
need to remove it for s390x.
On our s390x systems some checks fail consistently because the migration finishes before the check is started. Slow the migration down as usual via bandwidth parameter.
On our s390x systems some checks fail consistently because the migration finishes before the check is started. Slow the migration down as usual via bandwidth parameter.

Signed-off-by: Sebastian Mitterle <smitterl@redhat.com>
On our s390x systems some checks fail consistently because the migration finishes before the check is started. Slow the migration down as usual via bandwidth parameter.

Signed-off-by: Sebastian Mitterle <smitterl@redhat.com>
On our s390x systems some checks fail consistently because the
migration finishes before the check is started. Slow the migration down
as usual via bandwidth parameter.

Signed-off-by: Sebastian Mitterle <smitterl@redhat.com>
On our s390x systems some checks fail consistently because the
migration finishes before the check is started. Slow the migration
down as usual via bandwidth parameter.

Signed-off-by: Sebastian Mitterle <smitterl@redhat.com>
@smitterl smitterl force-pushed the slow_down_migration_s390x branch from e00bd62 to b0b4de1 Compare December 24, 2025 13:19
@smitterl smitterl requested a review from cliping December 24, 2025 13:19
@smitterl smitterl marked this pull request as ready for review December 24, 2025 13:19
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