Skip to content

Implement complete support for disks - #199

Open
davispuh wants to merge 7 commits into
fog:masterfrom
davispuh:disks
Open

Implement complete support for disks#199
davispuh wants to merge 7 commits into
fog:masterfrom
davispuh:disks

Conversation

@davispuh

Copy link
Copy Markdown
Contributor

Currently it's not possible to configure libvirt disks like adding multiple cdrom's or NVMe's.
This PR implements complete support for disks so all disk configurations that are supported by libvirt can be achieved.

Copilot AI lite review requested due to automatic review settings August 16, 2026 21:07

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR extends fog-libvirt’s Server disk handling to support a wider range of libvirt disk configurations (multiple disks, CD-ROMs, NVMe/network-backed sources, and richer disk XML), by introducing a structured Disk model hierarchy, updating domain listing to parse disks, and adding test coverage.

Changes:

  • Added a full set of Server::Disk (and nested) models to parse/build libvirt disk XML with many optional sub-elements.
  • Updated domain listing to parse disks from domain XML and expose them on server attributes.
  • Refactored server XML generation to emit disks from explicit :disks plus derived volume/ISO disks, and added comprehensive disk minitests.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
minitests/server/disks_test.rb Adds end-to-end coverage for disk JSON/XML roundtrips, mirrors, block disks, and Ceph disk XML.
lib/fog/libvirt/requests/compute/list_domains.rb Parses <devices><disk> elements into disk attribute hashes for server listings.
lib/fog/libvirt/models/compute/util/util.rb Adds XML attribute/value helpers and assignment casting helpers used by new models.
lib/fog/libvirt/models/compute/server/seclabel.rb Implements a Seclabel model for disk/source security labels.
lib/fog/libvirt/models/compute/server/encryption.rb Implements an Encryption model for disk/source encryption XML.
lib/fog/libvirt/models/compute/server/driver/virtio.rb Adds Virtio driver feature attributes (iommu/ats/packed/page_per_vq).
lib/fog/libvirt/models/compute/server/driver/statistics.rb Adds Driver statistics model with intervals and latency histograms.
lib/fog/libvirt/models/compute/server/driver/latency_histogram.rb Adds latency histogram model for driver statistics XML.
lib/fog/libvirt/models/compute/server/driver/iothread.rb Adds IOThread model for driver iothreads XML.
lib/fog/libvirt/models/compute/server/disk/target.rb Adds Disk::Target model for target device/bus/tray/etc.
lib/fog/libvirt/models/compute/server/disk/source/reservations.rb Adds Source reservations model for reservation XML.
lib/fog/libvirt/models/compute/server/disk/source/data_store.rb Adds Source DataStore support (network datastore XML).
lib/fog/libvirt/models/compute/server/disk/source.rb Adds Disk::Source model with many libvirt source options and nested elements.
lib/fog/libvirt/models/compute/server/disk/mirror.rb Adds Disk::Mirror parsing model for mirror XML.
lib/fog/libvirt/models/compute/server/disk/metadata_cache.rb Adds MetadataCache model used by format/driver.
lib/fog/libvirt/models/compute/server/disk/format.rb Adds Disk::Format model (type + metadata cache).
lib/fog/libvirt/models/compute/server/disk/driver.rb Adds Disk::Driver model with virtio fields, statistics, iothreads, metadata cache.
lib/fog/libvirt/models/compute/server/disk/backing_store.rb Adds BackingStore model with nested backing stores, format, and source.
lib/fog/libvirt/models/compute/server/disk.rb Adds the top-level Disk model with parse/build logic and nested components.
lib/fog/libvirt/models/compute/server.rb Switches domain XML disk emission to new disk models and adds derived volume/ISO disk handling.
lib/fog/libvirt/models/compute/common/pci_address.rb Adds PCI address subtype for Address.
lib/fog/libvirt/models/compute/common/drive_address.rb Adds Drive address subtype for Address.
lib/fog/libvirt/models/compute/common/clonable_model.rb Adds deep-clone/deep-dup support for attribute-backed models.
lib/fog/libvirt/models/compute/common/attribute_model.rb Introduces a Fog::Model subclass tailored to attribute-only models (no service identity assumptions).
lib/fog/libvirt/models/compute/common/address.rb Adds Address base class with subtype dispatch and XML parsing/building.
Suppressed comments (3)

lib/fog/libvirt/models/compute/server.rb:511

  • volume_disks calls disks.any?, which raises when disks is nil. Since callers can create servers without :disks, this should be nil-safe (e.g., by using disks.to_a).
        def volume_disks
          used_names = disk_device_names
          @volumes.to_a.filter_map do |volume|
            next if disks.any? { |disk| disk_source_path(disk) == volume.path }

lib/fog/libvirt/models/compute/server.rb:565

  • iso_disk checks disks.any?, which raises when disks is nil. Use disks.to_a.any? to handle servers created without :disks.
          return nil if disks.any? { |disk| disk_source_path(disk) == source }

lib/fog/libvirt/models/compute/server.rb:580

  • disk_device_names assumes disks is always an Array, but it can be nil. This will raise when building volume/ISO disks for servers created without :disks.
        def disk_device_names
          disks.filter_map do |disk|
            target = model_cast(disk, Disk).target
            target&.dev
          end

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lib/fog/libvirt/models/compute/server.rb
Comment thread lib/fog/libvirt/models/compute/util/util.rb
Comment thread lib/fog/libvirt/models/compute/server.rb Outdated
Comment thread lib/fog/libvirt/models/compute/server/disk/mirror.rb
Comment thread lib/fog/libvirt/models/compute/common/address.rb
@davispuh
davispuh force-pushed the disks branch 2 times, most recently from db98b9d to 4d7f180 Compare August 16, 2026 22:19
@davispuh

davispuh commented Aug 16, 2026

Copy link
Copy Markdown
Contributor Author

Use this functionality like:

server = compute.servers.create(
  disks: [
    {
      type:   :file,
      device: :cdrom,
      source: { file: iso_path },
      target: { dev: :sda, bus: :scsi }
    },
    {
      type:   :nvme,
      device: :disk,
      source: {
        type: :pci,
        managed: true,
        namespace: 1,
        address: { bus: '0x30', slot: '0x00' }
      },
      target: { dev: :nvme0n1, bus: :virtio }
    }
  ]
)

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.

2 participants