Replace xml_element(s) with direct inline Nokogiri use

This uses the Nokogiri feature to directly query an attribute by its
path to avoid needing to retrieve it.

Especially in list_domains this avoids the need to parse the same XML
over and over again.

Author: ekohl

lib/fog/libvirt.rb

@@ -2,6 +2,7 @@
 require 'fog/xml'
 require 'fog/json'
 require 'libvirt'
+require 'nokogiri'
 
 require File.expand_path('../libvirt/version', __FILE__)
 

lib/fog/libvirt/models/compute/util/util.rb

@@ -1,19 +1,8 @@
-require 'nokogiri'
 require 'securerandom'
 
 module Fog
   module Libvirt
     module Util
-      def xml_element(xml, path, attribute=nil)
-        xml = Nokogiri::XML(xml)
-        attribute.nil? ? (xml/path).first.text : (xml/path).first[attribute.to_sym]
-      end
-
-      def xml_elements(xml, path, attribute=nil)
-        xml = Nokogiri::XML(xml)
-        attribute.nil? ? (xml/path).map : (xml/path).map{|element| element[attribute.to_sym]}
-      end
-
       def randomized_name
         "fog-#{(SecureRandom.random_number*10E14).to_i.round}"
       end

lib/fog/libvirt/requests/compute/list_domains.rb

@@ -31,41 +31,41 @@ def catchLibvirtExceptions
         def domain_display xml
           attrs = {}
           [:type, :port, :password, :listen].each do |element|
-            attrs[element] = xml_element(xml, "domain/devices/graphics",element.to_s) rescue nil
+            attrs[element] = (xml / "domain/devices/graphics/@#{element}").text
           end
-          attrs.reject{|k,v| v.nil? or v == ""}
+          attrs.reject! { |k, v| v.empty? }
         end
 
         def domain_volumes xml
-          xml_elements(xml, "domain/devices/disk/source").map do |element|
+          (xml / "domain/devices/disk/source").map do |element|
             element[:file] || element[:dev] || element[:name]
           end
         end
 
         def boot_order xml
-          xml_elements(xml, "domain/os/boot", "dev")
+          (xml / "domain/os/boot/@dev").map(&:text)
         end
 
         def firmware(xml)
-          firmware_from_loader = xml_elements(xml, "domain/os/loader", "type").first
+          firmware_from_loader = (xml / "domain/os/loader/@type").text
 
           case firmware_from_loader
           when 'pflash'
             'efi'
           when 'rom'
             'bios'
           else
-            xml_elements(xml, "domain/os", "firmware").first || 'bios'
+            (xml / "domain/os/@firmware").first&.text || 'bios'
           end
         end
 
         # we rely on the fact that the secure attribute is only present when secure boot is enabled
         def secure_boot_enabled?(xml)
-          xml_elements(xml, "domain/os/loader", "secure").first == 'yes'
+          (xml / "domain/os/loader/@secure").text == 'yes'
         end
 
         def domain_interfaces xml
-          ifs = xml_elements(xml, "domain/devices/interface")
+          ifs = xml / "domain/devices/interface"
           ifs.map { |i|
             nics.new({
               :type    => i['type'],
@@ -80,6 +80,8 @@ def domain_interfaces xml
         def domain_to_attributes(dom)
           states= %w(nostate running blocked paused shutting-down shutoff crashed pmsuspended)
 
+          xml = Nokogiri::XML(dom.xml_desc)
+
           begin
             {
               :id              => dom.uuid,
@@ -92,13 +94,13 @@ def domain_to_attributes(dom)
               :autostart       => dom.autostart?,
               :os_type         => dom.os_type,
               :active          => dom.active?,
-              :display         => domain_display(dom.xml_desc),
-              :boot_order      => boot_order(dom.xml_desc),
-              :nics            => domain_interfaces(dom.xml_desc),
-              :volumes_path    => domain_volumes(dom.xml_desc),
+              :display         => domain_display(xml),
+              :boot_order      => boot_order(xml),
+              :nics            => domain_interfaces(xml),
+              :volumes_path    => domain_volumes(xml),
               :state           => states[dom.info.state],
-              :firmware        => firmware(dom.xml_desc),
-              :secure_boot     => secure_boot_enabled?(dom.xml_desc),
+              :firmware        => firmware(xml),
+              :secure_boot     => secure_boot_enabled?(xml),
             }
           rescue ::Libvirt::RetrieveError, ::Libvirt::Error
             # Catch libvirt exceptions to avoid race conditions involving

lib/fog/libvirt/requests/compute/list_volumes.rb

@@ -25,7 +25,8 @@ def list_volumes(filter = { })
         private
 
         def volume_to_attributes(vol)
-          format_type = xml_element(vol.xml_desc, "/volume/target/format", "type") rescue nil # not all volumes have types, e.g. LVM
+          xml = Nokogiri::XML(vol.xml_desc)
+          format_type = (xml / "/volume/target/format/@type").first&.text&.strip
           return nil if format_type == "dir"
 
           begin

lib/fog/libvirt/requests/compute/update_display.rb

@@ -13,7 +13,7 @@ def update_display(options = { })
           display[:listen] = options[:listen].to_s   if options[:listen]
           display[:passwd] = options[:password].to_s if options[:password]
           display[:autoport] = 'yes' if display[:port] == '-1'
-          new_keymap       = options[:keymap] || xml_elements(domain.xml_desc, "graphics", "keymap")[0]
+          new_keymap       = options.fetch(:keymap) { (Nokogiri::XML(domain.xml_desc) / "graphics/@keymap").first&.text }
           display[:keymap] = new_keymap unless new_keymap.nil?
 
           builder = Nokogiri::XML::Builder.new { graphics_ (display) }