Refactor Secure Boot Support and Firmware Handling

- Renamed firmware-related attributes to align with VMware conventions.
fog · Sep 5, 2024 · 5a05cd7 · 5a05cd7
1 parent e491682
commit 5a05cd7
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 19 deletions.
diff --git a/lib/fog/libvirt/models/compute/server.rb b/lib/fog/libvirt/models/compute/server.rb
@@ -13,8 +13,10 @@ class Server < Fog::Compute::Server
 
         attribute :cpus
         attribute :cputime
-        attribute :os_firmware
-        attribute :os_firmware_features
+        attribute :firmware
+        attribute :firmware_features
+        attribute :secure_boot
+        attribute :loader
         attribute :os_type
         attribute :memory_size
         attribute :max_memory_size
@@ -291,8 +293,7 @@ def to_xml
               xml.vcpu(cpus)
               os_tags = {}
 
-              # Set firmware only if it's EFI, BIOS don't need to be set
-              os_tags[:firmware] = "efi" if os_firmware == "efi"
+              os_tags[:firmware] = firmware
 
               xml.os(**os_tags) do
                 type = xml.type(os_type, :arch => arch)
@@ -302,9 +303,13 @@ def to_xml
                   xml.boot(:dev => dev)
                 end
 
-                if os_firmware == "efi"
+                loader.each do |key, value|
+                  xml.loader(key => value)
+                end
+
+                if firmware == "efi" && firmware_features&.any?
                   xml.firmware do
-                    os_firmware_features.each_pair do |key, value|
+                    firmware_features.each_pair do |key, value|
                       xml.feature(:name => key, :enabled => value)
                     end
                   end

diff --git a/lib/fog/libvirt/requests/compute/list_domains.rb b/lib/fog/libvirt/requests/compute/list_domains.rb
@@ -46,6 +46,24 @@ def boot_order xml
           xml_elements(xml, "domain/os/boot", "dev")
         end
 
+        def firmware(xml)
+          firmware_from_loader = xml_elements(xml, "domain/os/loader", "type").first
+
+          case firmware_from_loader
+          when 'pflash'
+            'efi'
+          when 'rom'
+            'bios'
+          else
+            xml_elements(xml, "domain/os", "firmware").first || 'bios'
+          end
+        end
+
+        # we rely on the fact that the secure attribute is only present when secure boot is enabled
+        def is_secure_boot_enabled?(xml)
+          xml_elements(xml, "domain/os/loader", "secure").first == 'yes'
+        end
+
         def domain_interfaces xml
           ifs = xml_elements(xml, "domain/devices/interface")
           ifs.map { |i|
@@ -61,7 +79,6 @@ def domain_interfaces xml
 
         def domain_to_attributes(dom)
           states= %w(nostate running blocked paused shutting-down shutoff crashed pmsuspended)
-
           begin
             {
               :id              => dom.uuid,
@@ -76,9 +93,11 @@ def domain_to_attributes(dom)
               :active          => dom.active?,
               :display         => domain_display(dom.xml_desc),
               :boot_order      => boot_order(dom.xml_desc),
+              :firmware        => firmware(dom.xml_desc),
               :nics            => domain_interfaces(dom.xml_desc),
               :volumes_path    => domain_volumes(dom.xml_desc),
-              :state           => states[dom.info.state]
+              :state           => states[dom.info.state],
+              :secure_boot     => is_secure_boot_enabled?(dom.xml_desc),
             }
           rescue ::Libvirt::RetrieveError, ::Libvirt::Error
             # Catch libvirt exceptions to avoid race conditions involving

diff --git a/tests/libvirt/models/compute/server_tests.rb b/tests/libvirt/models/compute/server_tests.rb
@@ -32,8 +32,10 @@
       attributes = [ :id,
         :cpus,
         :cputime,
-        :os_firmware,
-        :os_firmware_features,
+        :firmware,
+        :firmware_features,
+        :secure_boot,
+        :loader,
         :os_type,
         :memory_size,
         :max_memory_size,
@@ -92,10 +94,9 @@
     test("with efi firmware") do
       server = Fog::Libvirt::Compute::Server.new(
         {
-          :os_firmware => "efi",
-          :os_firmware_features => {
+          :firmware => "efi",
+          :firmware_features => {
             "secure-boot" => "no",
-            "enrolled-keys" => "no"
           },
           :nics => [],
           :volumes => []
@@ -104,19 +105,19 @@
       xml = server.to_xml
 
       os_firmware = xml.include?('<os firmware="efi">')
-      secure_boot = !xml.include?('<feature name="secure-boot" enabled="no" />')
-      enrolled_keys = !xml.include?('<feature name="enrolled-keys" enabled="no" />')
+      secure_boot = xml.include?('<feature name="secure-boot" enabled="no"/>')
 
-      os_firmware && secure_boot && enrolled_keys
+      os_firmware && secure_boot
     end
     test("with secure boot") do
       server = Fog::Libvirt::Compute::Server.new(
         {
-          :os_firmware => "efi",
-          :os_firmware_features => {
+          :firmware => "efi",
+          :firmware_features => {
             "secure-boot" => "yes",
             "enrolled-keys" => "yes"
           },
+          :loader => { "secure" => "yes" },
           :nics => [],
           :volumes => []
         }
@@ -126,8 +127,9 @@
       os_firmware = xml.include?('<os firmware="efi">')
       secure_boot = xml.include?('<feature name="secure-boot" enabled="yes"/>')
       enrolled_keys = xml.include?('<feature name="enrolled-keys" enabled="yes"/>')
+      loader = xml.include?('<loader secure="yes"/>')
 
-      os_firmware && secure_boot && enrolled_keys
+      os_firmware && secure_boot && enrolled_keys && loader
     end
   end
 end