Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 41 additions & 24 deletions app/models/miq_provision_virt_workflow.rb
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,13 @@ def tag_symbol
:vm_tags
end

VM_OR_TEMPLATE_EXTRA_COLS = %i[mem_cpu cpu_total_cores v_total_snapshots allocated_disk_storage].freeze
VM_OR_TEMPLATE_EXTRA_COLS = %i[
platform
mem_cpu
cpu_total_cores
v_total_snapshots
allocated_disk_storage
].freeze
def allowed_templates(options = {})
# Return pre-selected VM if we are called for cloning
if [:clone_to_vm, :clone_to_template].include?(request_type)
Expand Down Expand Up @@ -342,13 +348,13 @@ def allowed_templates(options = {})
# Only select the colums we need
vms = vms.select(:id, :type, :name, :guid, :uid_ems, :ems_id, :cloud_tenant_id)

allowed_templates_list = source_vm_rbac_filter(vms, condition, VM_OR_TEMPLATE_EXTRA_COLS).to_a
sql = source_vm_rbac_filter(vms, condition, VM_OR_TEMPLATE_EXTRA_COLS).to_sql
allowed_templates_list = ActiveRecord::Base.connection.select_all(sql).to_a
@allowed_templates_filter = filter_id
@allowed_templates_tag_filters = @values[:vm_tags]
rails_logger('allowed_templates', 1)
log_allowed_template_list(allowed_templates_list)

MiqPreloader.preload(allowed_templates_list, [:operating_system])
@_ems_allowed_templates_cache = {}
@allowed_templates_cache = allowed_templates_list.collect do |template|
create_hash_struct_from_vm_or_template(template, options)
Expand Down Expand Up @@ -1036,47 +1042,58 @@ def setup_parameters_for_visibility_service(options)
end

def create_hash_struct_from_vm_or_template(vm_or_template, options)
data_hash = {:id => vm_or_template.id,
:name => vm_or_template.name,
:guid => vm_or_template.guid,
:uid_ems => vm_or_template.uid_ems,
:platform => vm_or_template.platform,
:logical_cpus => vm_or_template.cpu_total_cores,
:mem_cpu => vm_or_template.mem_cpu,
:allocated_disk_storage => vm_or_template.allocated_disk_storage,
:v_total_snapshots => vm_or_template.v_total_snapshots,
:evm_object_class => :Vm}
data_hash[:cloud_tenant] = vm_or_template.cloud_tenant if vm_or_template.cloud_tenant_id
if vm_or_template.operating_system
data_hash[:operating_system] = MiqHashStruct.new(:product_name => vm_or_template.operating_system.product_name)
end
if options[:include_datacenter] == true
data_hash[:datacenter_name] = vm_or_template.owning_blue_folder.try(:parent_datacenter).try(:name)
if vm_or_template.kind_of?(Hash)
data_hash = vm_or_template
data_hash[:logical_cpus] = data_hash.delete(:cpu_total_cores)
if vm_or_template[:operating_system_product_name]
data_hash[:operating_system] = MiqHashStruct.new(:product_name => vm_or_template[:operating_system_product_name])
end
# TODO: solve owning_blue_folder for the "Hash method" (if needed...)
Copy link
Member

@kbrock kbrock May 28, 2019

Choose a reason for hiding this comment

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

owning_blue_folder tends to be a performance issue in lots of places

I agree that we should not address here

else
data_hash = {:id => vm_or_template.id,
:name => vm_or_template.name,
:guid => vm_or_template.guid,
:uid_ems => vm_or_template.uid_ems,
:platform => vm_or_template.platform,
:logical_cpus => vm_or_template.cpu_total_cores,
:mem_cpu => vm_or_template.mem_cpu,
:allocated_disk_storage => vm_or_template.allocated_disk_storage,
:v_total_snapshots => vm_or_template.v_total_snapshots}
data_hash[:cloud_tenant] = vm_or_template.cloud_tenant if vm_or_template[:cloud_tenant_id]
if vm_or_template.operating_system
data_hash[:operating_system] = MiqHashStruct.new(:product_name => vm_or_template.operating_system_product_name)
end
if vm_or_template[:ems_id]
data_hash[:ext_management_system] = MiqHashStruct.new(:name => vm_or_template.ext_management_system.name)
end
if options[:include_datacenter] == true
data_hash[:datacenter_name] = vm_or_template.owning_blue_folder.try(:parent_datacenter).try(:name)
end
end
assign_ems_data_to_data_hash(data_hash, vm_or_template)

MiqHashStruct.new(data_hash)
end

def assign_ems_data_to_data_hash(data_hash, vm_or_template)
data_hash[:evm_object_class] = :Vm

# Handle EMS data, either with a cache, or with the relation (assumes it is
# preloaded usually)
if @_ems_allowed_templates_cache
# don't have a key, so don't attempt to add to the data_hash
if vm_or_template.ems_id
ems_id = vm_or_template.ems_id
if vm_or_template[:ems_id]
ems_id = vm_or_template[:ems_id]

# only fetch the ems if not fetched previously
unless @_ems_allowed_templates_cache.key?(vm_or_template.ems_id)
unless @_ems_allowed_templates_cache.key?(vm_or_template[:ems_id])
@_ems_allowed_templates_cache[ems_id] = ExtManagementSystem.find(ems_id).try(:name)
Copy link
Member

Choose a reason for hiding this comment

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

Will we have a bunch of EMSes?

If so, would it make sense to collecting the :ems_id values first and make a single query?

I'm guessing you already looked at this and deemed it unnecessary

Copy link
Member

Choose a reason for hiding this comment

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

ugh - ignore this - we already discussed

end

if @_ems_allowed_templates_cache[ems_id]
data_hash[:ext_management_system] = MiqHashStruct.new(:name => @_ems_allowed_templates_cache[ems_id])
end
end
elsif vm_or_template.ems_id
data_hash[:ext_management_system] = MiqHashStruct.new(:name => vm_or_template.ext_management_system.name)
end
end

Expand Down
5 changes: 4 additions & 1 deletion app/models/vm_or_template.rb
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,8 @@ class VmOrTemplate < ApplicationRecord
virtual_delegate :name, :to => :host, :prefix => true, :allow_nil => true, :type => :string
virtual_delegate :name, :to => :storage, :prefix => true, :allow_nil => true, :type => :string
virtual_delegate :name, :to => :ems_cluster, :prefix => true, :allow_nil => true, :type => :string
virtual_delegate :platform, :to => :operating_system, :allow_nil => true
virtual_delegate :product_name, :to => :operating_system, :prefix => true, :allow_nil => true
virtual_delegate :vmm_product, :to => :host, :prefix => :v_host, :allow_nil => true, :type => :string
virtual_delegate :v_pct_free_disk_space, :v_pct_used_disk_space, :to => :hardware, :allow_nil => true, :type => :float
virtual_delegate :num_cpu, :to => "hardware.cpu_sockets", :allow_nil => true, :default => 0, :type => :integer
Expand Down Expand Up @@ -532,7 +534,8 @@ def os_image_name
end

def platform
name = OperatingSystem.platform(self)
name = self[:platform] if has_attribute?(:platform)
name ||= OperatingSystem.platform(self)
if name == 'unknown'
parent = genealogy_parent
name = OperatingSystem.platform(parent) unless parent.nil?
Expand Down