migrate_options_shared:fix removing vm

[dzhengfy]

Summary by CodeRabbit

• Tests
  • Expanded migration coverage by adding xbzrle method variants alongside parallel multifd scenarios in both success and failure paths, and removing a redundant default variant.
  • Adjusted expected error assertions for incompatible parallel+xvzrle combinations.
  • Improved test control flow to skip “migrate back” steps when an error is expected, leading to cleaner execution and teardown in error scenarios.

[coderabbitai]

Walkthrough

The config adds new xbzrle-with-parallel variants in both positive and negative multifd/customized_connection paths and removes a prior default_connection xbzrle variant. The Python test flow now gates “migrate back” logic behind a new status_error flag, skipping migration-back steps when status_error is enabled.

Changes

Cohort / File(s)	Summary
Migration config: xbzrle parallel variants libvirt/tests/cfg/migration/migrate_options_shared.cfg	Added with_xbzrle_method variants using virsh_migrate_extra with parallel=4 and specified incompatibility error messages in both positive and negative multifd/customized_connection paths; removed prior default_connection xbzrle variant.
Test flow gating by status_error libvirt/tests/src/migration/migrate_options_shared.py	Introduced status_error flag; conditioned migrate-back pre-checks and cleanup to require migrate_vm_back and not status_error, skipping migration-back steps when status_error is "yes".

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant T as Test Runner
  participant S as migrate_options_shared.py
  participant Src as Source Host
  participant Dst as Destination Host

  rect rgba(230,240,255,0.5)
    note over S: Setup & parameter parsing
    T->>S: Run migration test
    S->>S: status_error = (params["status_error"] == "yes")
  end

  rect rgba(240,255,240,0.5)
    S->>Src: Start migration to Dst
    Src-->>Dst: VM state transfer (multifd/customized)
    S->>S: do_actions_after_migrate()
  end

  alt status_error == no and migrate_vm_back == yes
    note over S: Post-migration checks and prepare migrate-back
    S->>Dst: Pre-check/setup for migrate back
    Dst-->>Src: Migrate VM back
  else status_error == yes
    note over S: Migration-back path skipped
  end

  rect rgba(255,245,230,0.5)
    note over S: Cleanup
    alt status_error == no and migrate_vm_back == yes
      S->>Src: Remove pre-migration setup
    else
      note right of S: Skip migrate-back cleanup
    end
  end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

minorfix

Suggested reviewers

• chloerh
• xiaodwan

Poem

I thump my paws—new paths appear,
XBZRLĒ meets parallel here.
If errors rise, I softly steer,
No hopping back, I wait, I peer.
Configs tweaked, flows crisp and clear—
A happy hare with change to cheer. 🐇✨

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 specifies the component under test and succinctly describes the nature of the change—fixing the issue removing a VM in the migrate_options_shared tests—so it accurately reflects a key outcome of the pull request without unnecessary detail.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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

[dzhengfy]

Before fix:

rhel.virsh.migrate_options_shared.positive_test.p2p_migration.multifd.with_xbzrle_method.without_postcopy
 Error Details
Failed to run 'virsh destroy avocado-vt-vm1' on remote: Command 'virsh destroy avocado-vt-vm1 1>/var/tmp/tmpe6_92irf 2>/var/tmp/tmpn0sltxya' failed.
stdout: '\n'
stderr: "error: failed to get domain 'avocado-vt-vm1'\n"
additional_info: None

After fix:

 (1/1) type_specific.io-github-autotest-libvirt.virsh.migrate_options_shared.negative_test.multifd.with_xbzrle_method.without_postcopy: PASS (105.60 s)
RESULTS    : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0

libvirt-11.5.0

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (3)

libvirt/tests/cfg/migration/migrate_options_shared.cfg (2)

521-524: Make err_msg resilient across libvirt versions.

The exact wording can vary slightly by version. Prefer a regex/substring that tolerates punctuation/wording differences.

-                            err_msg = "Compression method 'xbzrle' isn't supported with parallel migration"
+                            # Accept minor wording/punctuation differences across versions
+                            err_msg = "(?i)Compression method ['\"]?xbzrle['\"]? (isn'?t|is not) supported (with|for) (parallel|multifd) migration"

---

513-516: Typo in variant name.

Rename “inavalid_connect_num” → “invalid_connect_num” for clarity.

-                        - inavalid_connect_num:
+                        - invalid_connect_num:

libvirt/tests/src/migration/migrate_options_shared.py (1)

1605-1607: Good fix: skip migrate-back on status_error; avoid shadowing earlier status_error.

Re-reading status_error is correct, but reuse of the same variable name can confuse given the earlier assignment (Line 1055). Use a new name and reference it in both places.

-        status_error = params.get("status_error", "no") == "yes"
-        if migrate_vm_back and not status_error:
+        status_error_effective = params.get("status_error", "no") == "yes"
+        if migrate_vm_back and not status_error_effective:
@@
-            if migrate_vm_back and not status_error:
+            if migrate_vm_back and not status_error_effective:

Also applies to: 1678-1678

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8266542 and c25b56c.

📒 Files selected for processing (2)

• libvirt/tests/cfg/migration/migrate_options_shared.cfg (1 hunks)
• libvirt/tests/src/migration/migrate_options_shared.py (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

libvirt/tests/src/migration/migrate_options_shared.py (1)

libvirt/tests/src/virsh_cmd/domain/virsh_attach_device.py (1)

• status_error (81-83)