Explicitly set model='virtio-scsi' for SCSI controllers on Q35 guests

[B-Woody]

Problem

When utilising image-based provisioning via Red Hat Satellite (specifically verified on Satellite 6.19.2 running fog-libvirt 0.14.0) onto a libvirt provider, cloud-init user-data fails to execute.

The breakdown occurs because Satellite/fog-libvirt targets a SCSI bus (bus='scsi') to attach the user-data ISO payload on modern Q35 machine types. However, because no explicit controller model is provided in the generated XML, libvirt defaults to a legacy controller emulation type (it was lsilogic on my system).

Modern Red Hat Enterprise Linux (RHEL) minimal/cloud images don't load legacy storage drivers for lsilogic. As a result, the guest kernel cannot see the underlying storage controller, /dev/sda is never initialized inside the VM, and cloud-init fails to detect its datasource.

Solution

This PR injects a conditional check inside the XML builder block within server.rb. If any attached volumes are configured to use a scsi bus, it programmatically forces libvirt to instantiate a modern virtio-scsi controller layout:

if volumes.any? { |v| v.bus == 'scsi' }
  xml.controller(:type => 'scsi', :index => '0', :model => 'virtio-scsi')end

This ensures out-of-the-box compatibility for RHEL cloud images.

Development & Testing Notes

• Disclaimer: I am not natively a Ruby developer! After troubleshooting and isolating the root cause in the hypervisor XML configurations, I consulted a Large Language Model (LLM) to assist me in mapping the logic safely into the native Nokogiri syntax used by the gem's codebase.

• Validation: This fix was live-patched directly into server.rb on a production Red Hat Satellite 6.19.2 server running fog-libvirt-0.14.0. Post-patch testing verified that newly provisioned VMs successfully attached the virtual media controller using virtio-scsi. The guest kernels immediately mapped the volume to /dev/sda, allowing cloud-init to fetch the metadata source and run the commands in user-data.

[ekohl]

Tests fail with this:

Error saving the server: undefined method `bus' for     <Fog::Libvirt::Compute::Volume

Not sure if this is due to the mock driver we're using, but in the docs I can't find a reference. I can see sometimes we're setting it as part of the target though.

[B-Woody]

Apologies for the test failure!

According to the LLM:

I encountered a structural edge case where raw storage volumes are passed to the server model during unit testing, which do not expose a .bus method natively.

I have updated the conditional statement to defensively verify object methods using respond_to?(:bus) alongside checking nested disk definitions. This prevents the NoMethodError and allows the test suite to pass cleanly while retaining the core fix for the provisioning workflow.

When I tested it on a live server I just had a straight device definition without the if condition:

xml.controller(:type => 'scsi', :index => '0', :model => 'virtio-scsi')

I'm a bit out of my depth here, as I'm not a Ruby dev. I'll step out of the way and let some more experienced folks take a look. Hopefully it's almost there.

[B-Woody]

Hi,

Just wondering if this needs any changes to be merged?

Initially this was just to fix an edge case in my homelab, but as it turns out, I now have a customer who may run into a similar issues eventually, as they're looking to do image-based provisioning from Satellite with some RHEL KVM/libvirt hosts.

If this needs changes I'd be happy to take another look at it.

[davispuh]

I can reproduce this issue. Basically any iso_file won't work because of this.
But this PR implementation is wrong and I don't see why it would even work...

if volumes.any? { |v| v.respond_to?(:bus) && v.bus == 'scsi' } || 
 (respond_to?(:disks) && disks.any? { |d| d.is_a?(Hash) && d[:bus] == 'scsi' })

Because volume doesn't have #bus and there is no such thing as disks either. So with these wrong respond_to? checks this code essentially does nothing because it will never enter into if condition.
There's also another issue that for s390x arch there could be 2x scsi controllers with same index=0 because it already creates one.
Also it has nothing to do with your Ruby knowledge. It doesn't matter which language is used - you need to use attributes that actually exist by looking at surrounding code.
Or if you use LLMs etc then use something smarter (for example MiMo V2.5 or DeepSeek V4 Flash).

Anyway this is how I would fix this issue:

diff --git a/lib/fog/libvirt/models/compute/server.rb b/lib/fog/libvirt/models/compute/server.rb
index 0b23755..763a95b 100644
--- a/lib/fog/libvirt/models/compute/server.rb
+++ b/lib/fog/libvirt/models/compute/server.rb
@@ -284,6 +284,7 @@ module Fog
 
         # rubocop:disable Metrics
         def to_xml
+          scsi_controller = nil
           builder = Nokogiri::XML::Builder.new do |xml|
             xml.domain(:type => domain_type) do
               xml.name(name)
@@ -368,7 +369,9 @@ module Fog
                         end
                       end
 
-                      xml.target(:dev => target_device, :bus => ceph_args["bus_type"] == "virtio" ? "virtio" : "scsi")
+                      ceph_bus = ceph_args["bus_type"] == "virtio" ? "virtio" : "scsi"
+                      scsi_controller ||= create_scsi_controller if ceph_bus == "scsi"
+                      xml.target(:dev => target_device, :bus => ceph_bus)
                     end
                   else
                     is_block = volume.path.start_with?("/dev/")
@@ -394,6 +397,7 @@ module Fog
                     xml.readonly
                     xml.address(:type => "drive", :controller => 0, :bus => 0, :unit => 0)
                   end
+                  scsi_controller ||= create_scsi_controller
                 end
 
                 nics.each do |nic|
@@ -419,7 +423,8 @@ module Fog
                 end
 
                 if arch == "s390x"
-                  xml.controller(:type => "scsi", :index => "0", :model => "virtio-scsi")
+                  scsi_controller ||= create_scsi_controller
+
                   xml.console(:type => "pty") do
                     xml.target(:type => "sclp")
                   end
@@ -449,6 +454,7 @@ module Fog
                     xml.model(video)
                   end
                 end
+                xml.controller(scsi_controller) if scsi_controller
               end
             end
           end
@@ -542,6 +548,14 @@ module Fog
           @volumes.nil? ? @volumes = [volume] : @volumes << volume
         end
 
+        def create_controller(type, index, model)
+          { :type => type, :index => index, :model => model }
+        end
+
+        def create_scsi_controller
+          create_controller("scsi", 0, "virtio-scsi")
+        end
+
         def default_iso_dir
           "/var/lib/libvirt/images"
         end

Also btw I really dislike how #to_xml does way more than building xml. I think it should be split in separate #synthesize (or something else) that would convert fog model to libvirt concepts and then #to_xml could just building resulting xml from earlier synthesized data.

[B-Woody]

Fair enough, thanks for the review. I'm actually not a developer and am out of my depth here. I blindly trusted an LLM suggestion that hallucinated non-existent attributes instead of properly understanding the surrounding code. I apologise for that! Lesson learned!

Good point about the s390x architecture index too, which I definitely hadn't accounted for. I'm not sure if I'll have the time or capacity to understand the codebase well enough to confidently test a proper fix in the near term, so on my end I'll just stick to using that straight device definition as a local workaround for the time being.