From 86aac4ce9c1e327f5e15b51a47f134d86e47661b Mon Sep 17 00:00:00 2001 From: Chris Crebolder Date: Sun, 23 Feb 2020 15:35:12 -0500 Subject: [PATCH] Use fog-libvirt filters when querying libvirt host This commit replaces the pattern where the metadata for all volumes, servers, or pools was downloaded and then searched. Instead a filter argument is passed to the connection and only the metadata for the named resource is returned, if it exists. This results in significant speed increases for libvirt hosts that are offsite or have many volumes. --- lib/vagrant-libvirt/action/create_domain.rb | 10 +++------- .../action/create_domain_volume.rb | 14 +++++++------- lib/vagrant-libvirt/action/handle_box_image.rb | 6 +++--- .../action/handle_storage_pool.rb | 6 +++--- .../action/remove_stale_volume.rb | 16 +++++++++------- lib/vagrant-libvirt/action/set_name_of_domain.rb | 11 ++++------- 6 files changed, 29 insertions(+), 34 deletions(-) diff --git a/lib/vagrant-libvirt/action/create_domain.rb b/lib/vagrant-libvirt/action/create_domain.rb index 14dc9e4..0c02ba2 100644 --- a/lib/vagrant-libvirt/action/create_domain.rb +++ b/lib/vagrant-libvirt/action/create_domain.rb @@ -126,13 +126,9 @@ module VagrantPlugins pool_name = @storage_pool_name end @logger.debug "Search for volume in pool: #{pool_name}" - actual_volumes = - env[:machine].provider.driver.connection.volumes.all.select do |x| - x.pool_name == pool_name - end - domain_volume = ProviderLibvirt::Util::Collection.find_matching( - actual_volumes, "#{@name}.img" - ) + domain_volume = env[:machine].provider.driver.connection.volumes.all( + name: "#{@name}.img" + ).find { |x| x.pool_name == pool_name } raise Errors::DomainVolumeExists if domain_volume.nil? @domain_volume_path = domain_volume.path end diff --git a/lib/vagrant-libvirt/action/create_domain_volume.rb b/lib/vagrant-libvirt/action/create_domain_volume.rb index 6d41c7b..159697f 100644 --- a/lib/vagrant-libvirt/action/create_domain_volume.rb +++ b/lib/vagrant-libvirt/action/create_domain_volume.rb @@ -25,15 +25,15 @@ module VagrantPlugins @name = "#{env[:domain_name]}.img" # Verify the volume doesn't exist already. - domain_volume = ProviderLibvirt::Util::Collection.find_matching( - env[:machine].provider.driver.connection.volumes.all, @name - ) - raise Errors::DomainVolumeExists if domain_volume + domain_volume = env[:machine].provider.driver.connection.volumes.all( + name: @name + ).first + raise Errors::DomainVolumeExists if domain_volume.id # Get path to backing image - box volume. - box_volume = ProviderLibvirt::Util::Collection.find_matching( - env[:machine].provider.driver.connection.volumes.all, env[:box_volume_name] - ) + box_volume = env[:machine].provider.driver.connection.volumes.all( + name: env[:box_volume_name] + ).first @backing_file = box_volume.path # Virtual size of image. Take value worked out by HandleBoxImage diff --git a/lib/vagrant-libvirt/action/handle_box_image.rb b/lib/vagrant-libvirt/action/handle_box_image.rb index 9f705b1..5412ec4 100644 --- a/lib/vagrant-libvirt/action/handle_box_image.rb +++ b/lib/vagrant-libvirt/action/handle_box_image.rb @@ -63,9 +63,9 @@ module VagrantPlugins # locking all subsequent actions as well. @@lock.synchronize do # Don't continue if image already exists in storage pool. - break if ProviderLibvirt::Util::Collection.find_matching( - env[:machine].provider.driver.connection.volumes.all, env[:box_volume_name] - ) + break if env[:machine].provider.driver.connection.volumes.all( + name: env[:box_volume_name] + ).first.id # Box is not available as a storage pool volume. Create and upload # it as a copy of local box image. diff --git a/lib/vagrant-libvirt/action/handle_storage_pool.rb b/lib/vagrant-libvirt/action/handle_storage_pool.rb index c2f0a72..0f48ce3 100644 --- a/lib/vagrant-libvirt/action/handle_storage_pool.rb +++ b/lib/vagrant-libvirt/action/handle_storage_pool.rb @@ -24,9 +24,9 @@ module VagrantPlugins # locking all subsequent actions as well. @@lock.synchronize do # Check for storage pool, where box image should be created - break if ProviderLibvirt::Util::Collection.find_matching( - env[:machine].provider.driver.connection.pools.all, config.storage_pool_name - ) + break unless env[:machine].provider.driver.connection.pools.all( + name: config.storage_pool_name + ).empty? @logger.info("No storage pool '#{config.storage_pool_name}' is available.") diff --git a/lib/vagrant-libvirt/action/remove_stale_volume.rb b/lib/vagrant-libvirt/action/remove_stale_volume.rb index 8aa15cf..cd82c53 100644 --- a/lib/vagrant-libvirt/action/remove_stale_volume.rb +++ b/lib/vagrant-libvirt/action/remove_stale_volume.rb @@ -18,9 +18,9 @@ module VagrantPlugins # Remove stale server volume config = env[:machine].provider_config # Check for storage pool, where box image should be created - fog_pool = ProviderLibvirt::Util::Collection.find_matching( - env[:machine].provider.driver.connection.pools.all, config.storage_pool_name - ) + fog_pool = env[:machine].provider.driver.connection.pools.all( + name: config.storage_pool_name + ).first env[:result] = nil @@ -36,14 +36,16 @@ module VagrantPlugins @logger.debug("**** Volume name #{name}") # remove root storage - box_volume = ProviderLibvirt::Util::Collection.find_matching( - env[:machine].provider.driver.connection.volumes.all, name - ) - if box_volume && box_volume.pool_name == fog_pool.name + box_volume = env[:machine].provider.driver.connection.volumes.all( + name: name + ).find { |x| x.pool_name == fog_pool.name } + if box_volume && box_volume.id env[:ui].info(I18n.t('vagrant_libvirt.remove_stale_volume')) @logger.info("Deleting volume #{box_volume.key}") box_volume.destroy env[:result] = box_volume + else + @logger.debug("**** Volume #{name} not found in pool #{fog_pool.name}") end # Continue the middleware chain. @app.call(env) diff --git a/lib/vagrant-libvirt/action/set_name_of_domain.rb b/lib/vagrant-libvirt/action/set_name_of_domain.rb index ce5d1a3..57f79b7 100644 --- a/lib/vagrant-libvirt/action/set_name_of_domain.rb +++ b/lib/vagrant-libvirt/action/set_name_of_domain.rb @@ -13,20 +13,17 @@ module VagrantPlugins env[:domain_name] = build_domain_name(env) begin - @logger.info("Looking for domain #{env[:domain_name]} through list " \ - "#{env[:machine].provider.driver.connection.servers.all}") + @logger.info("Looking for domain #{env[:domain_name]}") # Check if the domain name is not already taken - domain = ProviderLibvirt::Util::Collection.find_matching( - env[:machine].provider.driver.connection.servers.all, env[:domain_name] + domain = env[:machine].provider.driver.connection.servers.all( + name: env[:domain_name] ) - rescue Fog::Errors::Error => e + rescue Libvirt::RetrieveError => e @logger.info(e.to_s) domain = nil end - @logger.info("Looking for domain #{env[:domain_name]}") - unless domain.nil? raise ProviderLibvirt::Errors::DomainNameExists, domain_name: env[:domain_name]