boot_from_disk_device: avoid os/boot plus disk boot conflict

[BulaYoungR]

When os_attrs_boots is set, remove all //boot nodes then apply os boots. Strip boot/loadparm from cloned disk attrs so inherited per-disk boot does not remain alongside (libvirt unsupported configuration).

Committer: Bolatbek Issakh bissakh@redhat.com

Summary by CodeRabbit

• Bug Fixes
  • Fixed boot configuration handling to prevent disk-specific boot settings from conflicting with OS-level boot settings.

[coderabbitai]

Walkthrough

This PR modifies the update_vm_xml() function in the boot order test module to properly manage boot configuration precedence. When OS-level boot attributes are provided, the function now removes all existing VM boot definitions and strips disk-specific boot attributes before applying the new OS boot settings, ensuring clean boot configuration without conflicting definitions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: avoiding conflicts between OS-level boot configuration and per-disk boot settings by clearing both types of boot definitions.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

libvirt/tests/src/guest_os_booting/boot_order/boot_from_disk_device.py (1)

63-69: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Replace eval() in boot parsing path with ast.literal_eval().

os_attrs_boots is parsed from params with eval() at line 63, which allows arbitrary code execution from cfg input. Please switch to ast.literal_eval() here and for the same pattern at lines 48 and 50 while touching this flow.

Proposed fix

+import ast
 import copy
 import os
@@
-        disk_org_attrs.update(eval(params.get("disk2_attrs", "{}")))
+        disk_org_attrs.update(ast.literal_eval(params.get("disk2_attrs", "{}")))
@@
-        disk_org_attrs.update(eval(params.get("disk1_attrs", "{}")))
+        disk_org_attrs.update(ast.literal_eval(params.get("disk1_attrs", "{}")))
@@
-    os_attrs_boots = eval(params.get("os_attrs_boots", "[]"))
+    os_attrs_boots = ast.literal_eval(params.get("os_attrs_boots", "[]"))

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@libvirt/tests/src/guest_os_booting/boot_order/boot_from_disk_device.py`
around lines 63 - 69, The use of eval() when parsing boot attributes (e.g.,
os_attrs_boots = eval(params.get("os_attrs_boots", "[]"))) is unsafe; replace
eval() with ast.literal_eval() and import ast at the top of the module. Update
the same pattern found earlier in this flow (the other occurrences that call
eval(params.get(...)) around the boot parsing code at the lines referenced) to
use ast.literal_eval(params.get(..., "[]")) so only Python literals are parsed;
ensure the import ast is added and run tests to confirm behavior unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@libvirt/tests/src/guest_os_booting/boot_order/boot_from_disk_device.py`:
- Around line 63-69: The use of eval() when parsing boot attributes (e.g.,
os_attrs_boots = eval(params.get("os_attrs_boots", "[]"))) is unsafe; replace
eval() with ast.literal_eval() and import ast at the top of the module. Update
the same pattern found earlier in this flow (the other occurrences that call
eval(params.get(...)) around the boot parsing code at the lines referenced) to
use ast.literal_eval(params.get(..., "[]")) so only Python literals are parsed;
ensure the import ast is added and run tests to confirm behavior unchanged.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 58479589-db7b-4be5-9e99-e914c418ae09

📥 Commits

Reviewing files that changed from the base of the PR and between c96ab65 and 924253f.

📒 Files selected for processing (1)

• libvirt/tests/src/guest_os_booting/boot_order/boot_from_disk_device.py

[harvey0100]

Closing this PR due to current team constraints. This is part of a broader effort to triage all in-flight work across our upstream repos. If this work is still needed, please feel free to reopen and it will be picked up. Apologies for any inconvenience.