Skip to content

Conversation

@BulaYoungR
Copy link

@BulaYoungR BulaYoungR commented Sep 3, 2025

Summary by CodeRabbit

  • Refactor
    • Added an internal utility to enumerate virtual machine disk target names from VM configurations.
    • No user-visible behavior, workflows, outputs, or performance impacts.
    • Error handling and logging unchanged; no documentation updates required.

@coderabbitai
Copy link

coderabbitai bot commented Sep 3, 2025

📝 Walkthrough

Walkthrough

Adds a new public helper function get_all_disks_target(vm_name, options="") in virttest/utils_libvirt/libvirt_disk.py that loads a VM XML by vm_name, collects disk target device names (disk.target.dev) from disk devices, and returns them as a list. Returns an empty list if VM XML is None.

Changes

Cohort / File(s) Summary of Changes
Libvirt disk utilities
virttest/utils_libvirt/libvirt_disk.py
Added public helper get_all_disks_target(vm_name, options="") which loads VM XML via VMXML.new_from_dumpxml, extracts disk.target.dev values from disk devices (safely handling missing targets), logs discovered targets, and returns the list; no existing behaviors modified.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title Check ✅ Passed The pull request title "Collect all VM disk target device names from dumpxml" accurately describes the main change in the changeset, which is the addition of a new public helper function get_all_disks_target() that retrieves disk target device names from VM XML via dumpxml. The title is clear, specific, and concise without vague terms or unnecessary noise, making it easy for teammates to understand the primary change at a glance.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

🧹 Nitpick comments (1)
virttest/utils_libvirt/libvirt_disk.py (1)

166-169: Optional: align with existing device access pattern

Elsewhere in this module, devices are accessed via vmxml.devices.by_device_tag("disk"). Consider using that for consistency with get_mirror_part_in_xml(). Functionality is the same.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled
  • Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 5308b66 and 9f40114.

📒 Files selected for processing (1)
  • virttest/utils_libvirt/libvirt_disk.py (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
virttest/utils_libvirt/libvirt_disk.py (1)
virttest/libvirt_xml/vm_xml.py (4)
  • VMXML (753-2082)
  • new_from_dumpxml (788-803)
  • get_devices (550-565)
  • append (40-43)
🪛 Ruff (0.12.2)
virttest/utils_libvirt/libvirt_disk.py

165-165: Undefined name vm_name

(F821)

🔇 Additional comments (1)
virttest/utils_libvirt/libvirt_disk.py (1)

163-171: No callers found for _all_disk_targets(); ignore signature‐break concern.

Likely an incorrect or invalid review comment.

@dzhengfy dzhengfy closed this Oct 16, 2025
@dzhengfy dzhengfy reopened this Oct 16, 2025
@dzhengfy
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants