[Ovirt-devel] [PATCH] added VM-level migration support to the WUI. Still no back end bits here, though.

Jason Guiditta jguiditt at redhat.com
Tue Jul 15 22:04:25 UTC 2008


Scott, could you give me a quick scenario to test against this so I can
see it does what you meant it to?  Comments based just on the code
inline.

On Tue, 2008-07-15 at 11:28 -0400, Scott Seago wrote:
> Signed-off-by: Scott Seago <sseago at redhat.com>
> ---
>  wui/src/app/controllers/hardware_controller.rb  |   18 ++++--
>  wui/src/app/controllers/host_controller.rb      |   11 +++
>  wui/src/app/controllers/resources_controller.rb |   25 -------
>  wui/src/app/controllers/vm_controller.rb        |   13 +++-
>  wui/src/app/models/vm.rb                        |   31 ++++++---
>  wui/src/app/models/vm_task.rb                   |   70 +++++++++++++++-----
>  wui/src/app/views/hardware/show_hosts.rhtml     |    5 +-
>  wui/src/app/views/hardware/show_storage.rhtml   |    2 +-
>  wui/src/app/views/host/_grid.rhtml              |   15 +++--
>  wui/src/app/views/host/addhost.html.erb         |    5 +-
>  wui/src/app/views/host/quick_summary.rhtml      |    1 +
>  wui/src/app/views/storage/_grid.rhtml           |    4 +-
>  wui/src/app/views/storage/add.rhtml             |    2 +-
>  wui/src/app/views/vm/migrate.rhtml              |   81 +++++++++++++++++++++++
>  wui/src/app/views/vm/show.rhtml                 |   15 +++-
>  wui/src/public/javascripts/flexigrid.js         |    4 +-
>  wui/src/public/javascripts/ovirt.js             |    2 +-
>  17 files changed, 230 insertions(+), 74 deletions(-)
>  create mode 100644 wui/src/app/views/host/quick_summary.rhtml
>  create mode 100644 wui/src/app/views/vm/migrate.rhtml
> 
> diff --git a/wui/src/app/controllers/hardware_controller.rb b/wui/src/app/controllers/hardware_controller.rb
> index 4175fd8..926b6c8 100644
> --- a/wui/src/app/controllers/hardware_controller.rb
> +++ b/wui/src/app/controllers/hardware_controller.rb
> @@ -112,7 +112,12 @@ class HardwareController < ApplicationController
>    end
>  
>    def hosts_json
> -    if params[:id]
> +    if params[:exclude_host]
> +      pre_json
> +      hosts = @pool.hosts
> +      find_opts = {:conditions => ["id != ?", params[:exclude_host]]}
> +      include_pool = false
> +    elsif params[:id]
>        pre_json
>        hosts = @pool.hosts
>        find_opts = {}
> @@ -120,14 +125,17 @@ class HardwareController < ApplicationController
>      else
>        # FIXME: no permissions or usage checks here yet
>        # filtering on which pool to exclude
> -      id = params[:exclude_id]
> +      id = params[:exclude_pool]
>        hosts = Host
>        find_opts = {:include => :hardware_pool, 
>          :conditions => ["pools.id != ?", id]}
>        include_pool = true
>      end
> -    attr_list = [:id, :hostname, :uuid, :hypervisor_type, :num_cpus, :cpu_speed, :arch, :memory_in_mb, :status_str, :id]
> -    attr_list.insert(2, [:hardware_pool, :name]) if include_pool
> +    attr_list = []
> +    attr_list << :id if params[:checkboxes]
> +    attr_list << :hostname
> +    attr_list << [:hardware_pool, :name] if include_pool
> +    attr_list += [:uuid, :hypervisor_type, :num_cpus, :cpu_speed, :arch, :memory_in_mb, :status_str, :id]
>      json_list(hosts, attr_list, [:all], find_opts)
>    end
>  
> @@ -152,7 +160,7 @@ class HardwareController < ApplicationController
>      else
>        # FIXME: no permissions or usage checks here yet
>        # filtering on which pool to exclude
> -      id = params[:exclude_id]
> +      id = params[:exclude_pool]
>        storage_pools = StoragePool
>        find_opts = {:include => :hardware_pool, 
>          :conditions => ["pools.id != ?", id]}
> diff --git a/wui/src/app/controllers/host_controller.rb b/wui/src/app/controllers/host_controller.rb
> index 20571d7..4e4375a 100644
> --- a/wui/src/app/controllers/host_controller.rb
> +++ b/wui/src/app/controllers/host_controller.rb
> @@ -44,6 +44,17 @@ class HostController < ApplicationController
>      render :layout => 'selection'    
>    end
>  
> +  def quick_summary
> +    pre_show
> +    set_perms(@perm_obj)
> +    unless @can_view
> +      flash[:notice] = 'You do not have permission to view this host: redirecting to top level'
> +      #perm errors for ajax should be done differently
Considering it is almost all ajax now, we should figure out what
'differently' is asap
> +      redirect_to :controller => 'dashboard', :action => 'list'
> +    end
> +    render :layout => false
> +  end
> +
>    # retrieves data used by snapshot graphs
>    def snapshot_graph
>    end
> diff --git a/wui/src/app/controllers/resources_controller.rb b/wui/src/app/controllers/resources_controller.rb
> index 694ef74..ca10960 100644
> --- a/wui/src/app/controllers/resources_controller.rb
> +++ b/wui/src/app/controllers/resources_controller.rb
> @@ -184,31 +184,6 @@ class ResourcesController < ApplicationController
>        @failure_list = []
>      end
>      render :layout => 'confirmation'    
> -        
> -    #if params[:vm_actions][:vms]
> -    #  vms = params[:vm_actions][:vms]
> -    #  if params[:vm_actions][VmTask::ACTION_START_VM]
> -    #    flash[:notice] = "Starting Machines #{vms.join(',')}."
> -    #  elsif params[:vm_actions][VmTask::ACTION_SHUTDOWN_VM]
> -    #    flash[:notice] = "Stopping Machines #{vms.join(',')}."
> -    #  elsif params[:vm_actions][:other_actions]
> -    #    case params[:vm_actions][:other_actions]
> -    #    when VmTask::ACTION_SHUTDOWN_VM then flash[:notice] = "Stopping Machines #{vms.join(',')}."
> -    #    when VmTask::ACTION_START_VM then flash[:notice] = "Starting Machines #{vms.join(',')}."
> -    #    when VmTask::ACTION_SUSPEND_VM then flash[:notice] = "Suspending Machines #{vms.join(',')}."
> -    #    when VmTask::ACTION_RESUME_VM then flash[:notice] = "Resuming Machines #{vms.join(',')}."
> -    #    when VmTask::ACTION_SAVE_VM then flash[:notice] = "Saving Machines #{vms.join(',')}."
> -    #    when VmTask::ACTION_RESTORE_VM then flash[:notice] = "Restoring Machines #{vms.join(',')}."
> -    #    when "destroy" then flash[:notice] = "Destroying Machines #{vms.join(',')}."
> -    #    else
> -    #      flash[:notice] = 'No Action Chosen.'
> -    #    end
> -    #  else
> -    #    flash[:notice] = 'No Action Chosen.'
> -    #  end
> -    #else
> -    #  flash[:notice] = 'No Virtual Machines Selected.'
> -    #end
>    end
>  
>    protected
> diff --git a/wui/src/app/controllers/vm_controller.rb b/wui/src/app/controllers/vm_controller.rb
> index b9156d2..4f9962d 100644
> --- a/wui/src/app/controllers/vm_controller.rb
> +++ b/wui/src/app/controllers/vm_controller.rb
> @@ -26,7 +26,7 @@ class VmController < ApplicationController
>  
>    def show
>      set_perms(@perm_obj)
> -    @actions = @vm.get_action_and_label_list
> +    @actions = @vm.get_action_hash(@user)
>      unless @can_view
>        flash[:notice] = 'You do not have permission to view this vm: redirecting to top level'
>        redirect_to :controller => 'resources', :controller => 'dashboard'
> @@ -149,8 +149,9 @@ class VmController < ApplicationController
>  
>    def vm_action
>      vm_action = params[:vm_action]
> +    data = params[:vm_action_data]
>      begin
> -      if @vm.queue_action(get_login_user, vm_action)
> +      if @vm.queue_action(get_login_user, vm_action, data)
>          render :json => { :object => "vm", :success => true, :alert => "#{vm_action} was successfully queued." }
>        else
>          render :json => { :object => "vm", :success => false, :alert => "#{vm_action} is an invalid action." }
> @@ -171,6 +172,14 @@ class VmController < ApplicationController
>      end
>    end
>  
> +  def migrate
> +    @vm = Vm.find(params[:id])
> +    @perm_obj = @vm.get_hardware_pool
> +    @redir_obj = @vm
> +    @current_pool_id=@vm.vm_resource_pool.id
> +    authorize_admin
> +    render :layout => 'popup'
> +  end
>  
>    def console
>      @show_vnc_error = "Console is unavailable for VM #{@vm.description}" unless @vm.has_console
> diff --git a/wui/src/app/models/vm.rb b/wui/src/app/models/vm.rb
> index 617512e..b607886 100644
> --- a/wui/src/app/models/vm.rb
> +++ b/wui/src/app/models/vm.rb
> @@ -22,7 +22,7 @@ require 'util/ovirt'
>  class Vm < ActiveRecord::Base
>    belongs_to :vm_resource_pool
>    belongs_to :host
> -  has_many :tasks, :class_name => "VmTask", :dependent => :destroy, :order => "id DESC" do
> +  has_many :tasks, :class_name => "VmTask", :dependent => :destroy, :order => "id ASC" do
>      def queued
>        find(:all, :conditions=>{:state=>Task::STATE_QUEUED})
>      end
> @@ -58,6 +58,9 @@ class Vm < ActiveRecord::Base
>    STATE_SAVING         = "saving"
>    STATE_SAVED          = "saved"
>    STATE_RESTORING      = "restoring"
> +
> +  STATE_MIGRATING      = "migrating"
> +
>    STATE_CREATE_FAILED  = "create_failed"
>    STATE_INVALID        = "invalid"
>  
> @@ -68,7 +71,8 @@ class Vm < ActiveRecord::Base
>                            STATE_SUSPENDING,
>                            STATE_RESUMING,
>                            STATE_SAVING,
> -                          STATE_RESTORING]
> +                          STATE_RESTORING,
> +                          STATE_MIGRATING]
>  
>    EFFECTIVE_STATE = {  STATE_PENDING       => STATE_PENDING,
>                         STATE_UNREACHABLE   => STATE_UNREACHABLE,
> @@ -83,9 +87,15 @@ class Vm < ActiveRecord::Base
>                         STATE_SAVING        => STATE_SAVED,
>                         STATE_SAVED         => STATE_SAVED,
>                         STATE_RESTORING     => STATE_RUNNING,
> +                       STATE_MIGRATING     => STATE_RUNNING,
>                         STATE_CREATE_FAILED => STATE_CREATE_FAILED}
>    TASK_STATE_TRANSITIONS = []
>  
> +  def get_hardware_pool
> +    pool = vm_resource_pool
> +    pool = pool.get_hardware_pool if pool
> +    pool
> +  end
>    def storage_volume_ids
>      storage_volumes.collect {|x| x.id }
>    end
> @@ -126,9 +136,9 @@ class Vm < ActiveRecord::Base
>      RUNNING_STATES.include?(get_pending_state)
>    end
>  
> -  def get_action_list
> +  def get_action_list(user=nil)
You do this in several places below, and I must be misunderstanding -
what is the purpose of having this user var if it is always nil?

>      # return empty list rather than nil
> -    return_val = VmTask::VALID_ACTIONS_PER_VM_STATE[get_pending_state] || []
> +    return_val = VmTask.valid_actions_for_vm_state(get_pending_state, self, user) || []
>      # filter actions based on quota
>      unless resources_for_start?
>        return_val = return_val - [VmTask::ACTION_START_VM, VmTask::ACTION_RESTORE_VM]
> @@ -136,10 +146,12 @@ class Vm < ActiveRecord::Base
>      return_val
>    end
>  
> -  def get_action_and_label_list
> -    get_action_list.collect do |action|
> -      VmTask.label_and_action(action)
> +  def get_action_hash(user=nil)
> +    actions = {}
> +    get_action_list(user).each do |action|
> +      actions[action] = VmTask::ACTIONS[action]
>      end
> +    actions
>    end
>  
>    # these resource checks are made at VM start/restore time
> @@ -158,11 +170,12 @@ class Vm < ActiveRecord::Base
>      return return_val
>    end
>  
> -  def queue_action(user, action)
> +  def queue_action(user, action, data = nil)
>      return false unless get_action_list.include?(action)
>      task = VmTask.new({ :user    => user,
>                          :vm_id   => id,
> -                         :action  => action,
> +                        :action  => action,
> +                        :args    => data,
>                          :state   => Task::STATE_QUEUED})
>      task.save!
>      return true
> diff --git a/wui/src/app/models/vm_task.rb b/wui/src/app/models/vm_task.rb
> index aa12903..3f52478 100644
> --- a/wui/src/app/models/vm_task.rb
> +++ b/wui/src/app/models/vm_task.rb
> @@ -33,59 +33,97 @@ class VmTask < Task
>  
>    ACTION_UPDATE_STATE_VM = "update_state_vm"
>  
> +  # for migrate VM action, args provides the optional target host
> +  ACTION_MIGRATE_VM  = "migrate_vm"
> +
> +  PRIV_OBJECT_VM_POOL = "vm_resource_pool"
> +  PRIV_OBJECT_HW_POOL = "get_hardware_pool"
> +
> +

Perhaps this is less a comment on the patch and more on the way we are
doing this.  It feels vaguely wrong to me to do it this way.  I see a
lot of repeated attributes, which to me says it should be more
encapsulated in some way.  Would it make more sense to put these
'contants' in the database?  Or even a fixture of some sort?

>    # a hash of task actions which point to a hash which define valid state transitions
>    ACTIONS = { ACTION_CREATE_VM   => { :label => "Create",
>                                        :icon  => "icon_start.png",
>                                        :start => Vm::STATE_PENDING,
>                                        :running => Vm::STATE_CREATING,
>                                        :success => Vm::STATE_STOPPED,
> -                                      :failure => Vm::STATE_CREATE_FAILED},
> +                                      :failure => Vm::STATE_CREATE_FAILED,
> +                                      :privilege => [Permission::PRIV_MODIFY,
> +                                                     PRIV_OBJECT_VM_POOL]},
>                ACTION_START_VM    => { :label => "Start",
>                                        :icon  => "icon_start.png",
>                                        :start => Vm::STATE_STOPPED,
>                                        :running => Vm::STATE_STARTING,
>                                        :success => Vm::STATE_RUNNING,
> -                                      :failure => Vm::STATE_STOPPED}, 
> +                                      :failure => Vm::STATE_STOPPED,
> +                                      :privilege => [Permission::PRIV_VM_CONTROL,
> +                                                     PRIV_OBJECT_VM_POOL]},
>                ACTION_SHUTDOWN_VM => { :label => "Shutdown",
>                                        :icon  => "icon_x.png",
>                                        :start => Vm::STATE_RUNNING,
>                                        :running => Vm::STATE_STOPPING,
>                                        :success => Vm::STATE_STOPPED,
> -                                      :failure => Vm::STATE_RUNNING}, 
> +                                      :failure => Vm::STATE_RUNNING,
> +                                      :privilege => [Permission::PRIV_VM_CONTROL,
> +                                                     PRIV_OBJECT_VM_POOL]},
>                ACTION_SUSPEND_VM  => { :label => "Suspend",
>                                        :icon  => "icon_suspend.png",
>                                        :start => Vm::STATE_RUNNING,
>                                        :running => Vm::STATE_SUSPENDING,
>                                        :success => Vm::STATE_SUSPENDED,
> -                                      :failure => Vm::STATE_RUNNING}, 
> +                                      :failure => Vm::STATE_RUNNING,
> +                                      :privilege => [Permission::PRIV_VM_CONTROL,
> +                                                     PRIV_OBJECT_VM_POOL]},
>                ACTION_RESUME_VM   => { :label => "Resume",
>                                        :icon  => "icon_start.png",
>                                        :start => Vm::STATE_SUSPENDED,
>                                        :running => Vm::STATE_RESUMING,
>                                        :success => Vm::STATE_RUNNING,
> -                                      :failure => Vm::STATE_SUSPENDED},
> +                                      :failure => Vm::STATE_SUSPENDED,
> +                                      :privilege => [Permission::PRIV_VM_CONTROL,
> +                                                     PRIV_OBJECT_VM_POOL]},
>                ACTION_SAVE_VM     => { :label => "Save",
>                                        :icon  => "icon_save.png",
>                                        :start => Vm::STATE_RUNNING,
>                                        :running => Vm::STATE_SAVING,
>                                        :success => Vm::STATE_SAVED,
> -                                      :failure => Vm::STATE_RUNNING},
> +                                      :failure => Vm::STATE_RUNNING,
> +                                      :privilege => [Permission::PRIV_VM_CONTROL,
> +                                                     PRIV_OBJECT_VM_POOL]},
>                ACTION_RESTORE_VM  => { :label => "Restore",
>                                        :icon  => "icon_restore.png",
>                                        :start => Vm::STATE_SAVED,
>                                        :running => Vm::STATE_RESTORING,
>                                        :success => Vm::STATE_RUNNING,
> -                                      :failure => Vm::STATE_SAVED} }
> +                                      :failure => Vm::STATE_SAVED,
> +                                      :privilege => [Permission::PRIV_VM_CONTROL,
> +                                                     PRIV_OBJECT_VM_POOL]},
> +              ACTION_MIGRATE_VM  => { :label => "Migrate",
> +                                      :icon  => "icon_restore.png",
> +                                      :start => Vm::STATE_RUNNING,
> +                                      :running => Vm::STATE_MIGRATING,
> +                                      :success => Vm::STATE_RUNNING,
> +                                      :failure => Vm::STATE_RUNNING,
> +                                      :privilege => [Permission::PRIV_MODIFY,
> +                                                     PRIV_OBJECT_HW_POOL],
> +                                      :popup_action => 'migrate'} }
>  
> -  VALID_ACTIONS_PER_VM_STATE = {  Vm::STATE_PENDING       => [ACTION_CREATE_VM],
> -                                  Vm::STATE_RUNNING       => [ACTION_SHUTDOWN_VM,
> -                                                              ACTION_SUSPEND_VM,
> -                                                              ACTION_SAVE_VM],
> -                                  Vm::STATE_STOPPED       => [ACTION_START_VM],
> -                                  Vm::STATE_SUSPENDED     => [ACTION_RESUME_VM],
> -                                  Vm::STATE_SAVED         => [ACTION_RESTORE_VM],
> -                                  Vm::STATE_CREATE_FAILED => [],
> -                                  Vm::STATE_INVALID       => []}
> +  def self.valid_actions_for_vm_state(state, vm=nil, user=nil)
> +    actions = []
> +    ACTIONS.each do |action, hash|
> +      if hash[:start] == state
> +        add_action = true
> +        print "vm: #{vm}\n user: #{user}\n"
> +        if (vm and user)
> +          pool = vm.send(hash[:privilege][1])
> +          print "pool: #{pool}\n privilege: #{hash[:privilege][1]}\n"
> +          add_action = pool ? pool.has_privilege(user, hash[:privilege][0]) : false
> +        end
> +        print "add_action: #{add_action}\n"
> +        actions << action if add_action
> +      end
> +    end
> +    actions
> +  end
>  
>    def self.action_label(action)
>      return ACTIONS[action][:label]
> diff --git a/wui/src/app/views/hardware/show_hosts.rhtml b/wui/src/app/views/hardware/show_hosts.rhtml
> index aaf28b6..167c601 100644
> --- a/wui/src/app/views/hardware/show_hosts.rhtml
> +++ b/wui/src/app/views/hardware/show_hosts.rhtml
> @@ -60,7 +60,10 @@
>     <div class="data_section">
>        <%= render :partial => "/host/grid", :locals => { :table_id => "hosts_grid",
>                                                          :hwpool => @pool,
> -                                                        :exclude_id => nil,
> +                                                        :exclude_pool => nil,
> +                                                        :exclude_host => nil,
> +                                                        :show_pool => false,
> +                                                        :checkboxes => true,
>                                                          :on_select => "hosts_select", 
>                                                          :on_deselect => "load_widget_deselect",
>                                                          :on_hover => "load_widget_hover",
> diff --git a/wui/src/app/views/hardware/show_storage.rhtml b/wui/src/app/views/hardware/show_storage.rhtml
> index ec1a82d..dc1460a 100644
> --- a/wui/src/app/views/hardware/show_storage.rhtml
> +++ b/wui/src/app/views/hardware/show_storage.rhtml
> @@ -66,7 +66,7 @@
>    <div class="data_section">
>         <%= render :partial => "/storage/grid", :locals => { :table_id => "storage_grid",
>                                                              :hwpool => @pool,
> -                                                            :exclude_id => nil,
> +                                                            :exclude_pool => nil,
>                                                              :on_select => "storage_select" } %>
>    </div>
>  
> diff --git a/wui/src/app/views/host/_grid.rhtml b/wui/src/app/views/host/_grid.rhtml
> index 98a8af4..d4d4d11 100644
> --- a/wui/src/app/views/host/_grid.rhtml
> +++ b/wui/src/app/views/host/_grid.rhtml
> @@ -2,20 +2,25 @@
>  
>  <% hosts_per_page = 40 %>
>  <div id="<%= table_id %>_div">
> -<form id="<%= table_id %>_form">
> +<%= '<form id="#{table_id}_form">' if checkboxes %>
>  <table id="<%= table_id %>" style="display:none"></table>
> -</form>
> +<%= '</form>' if checkboxes %>
>  </div>
>  <script type="text/javascript">
>      $("#<%= table_id %>").flexigrid
>      (
>      {
> -    url: '<%=  url_for :controller => "hardware", :action => "hosts_json", :id => (hwpool.nil? ? nil : hwpool.id), :exclude_id => exclude_id %>',
> +    url: '<%=  url_for :controller => "hardware",
> +                       :action => "hosts_json",
> +                       :id => (hwpool.nil? ? nil : hwpool.id),
> +                       :exclude_pool => exclude_pool,
> +                       :exclude_host => exclude_host,
> +                       :checkboxes => checkboxes %>',
>      dataType: 'json',
>      colModel : [
> -        {display: '', width : 20, align: 'left', process: <%= table_id %>checkbox},
> +        <%= "{display: '', width : 20, align: 'left', process: #{table_id}checkbox}," if checkboxes %>
>          {display: 'Hostname', name : 'hostname', width : 60, align: 'left'},
> -        <%= "{display: 'Hardware Pool', name : 'pools.name', width : 100, align: 'left'}," if exclude_id %>
> +        <%= "{display: 'Hardware Pool', name : 'pools.name', width : 100, align: 'left'}," if exclude_pool %>
>          {display: 'UUID', name : 'uuid', width : 180, align: 'left'},
>          {display: 'Hypervisor', name : 'hypervisor_type', width : 60, align: 'left'},
>          {display: 'CPUs', name : 'num_cpus', width : 30, align: 'left'},
> diff --git a/wui/src/app/views/host/addhost.html.erb b/wui/src/app/views/host/addhost.html.erb
> index cfcc43b..41b6213 100644
> --- a/wui/src/app/views/host/addhost.html.erb
> +++ b/wui/src/app/views/host/addhost.html.erb
> @@ -8,7 +8,10 @@
>  <div class="panel_header"></div>
>  <div class="dialog_body">
>    <%= render :partial => "/host/grid", :locals => { :table_id => "addhosts_grid",
> -             :hwpool => nil, :exclude_id => @hardware_pool.id,
> +             :hwpool => nil,
> +             :exclude_pool => @hardware_pool.id,
> +             :exclude_host => nil,
> +             :checkboxes => true,
>               :on_select => "load_widget_select",
>               :on_deselect => "load_widget_deselect",
>               :on_hover => "load_widget_hover",
> diff --git a/wui/src/app/views/host/quick_summary.rhtml b/wui/src/app/views/host/quick_summary.rhtml
> new file mode 100644
> index 0000000..eceb561
> --- /dev/null
> +++ b/wui/src/app/views/host/quick_summary.rhtml
> @@ -0,0 +1 @@
> +  <%=h @host.hostname %>
> diff --git a/wui/src/app/views/storage/_grid.rhtml b/wui/src/app/views/storage/_grid.rhtml
> index 8b10aaa..3bdf407 100644
> --- a/wui/src/app/views/storage/_grid.rhtml
> +++ b/wui/src/app/views/storage/_grid.rhtml
> @@ -9,12 +9,12 @@
>      $("#<%= table_id %>").flexigrid
>      (
>      {
> -    url: '<%=  url_for :controller => "hardware", :action => "storage_pools_json", :id => (hwpool.nil? ? nil : hwpool.id), :exclude_id => exclude_id %>',
> +    url: '<%=  url_for :controller => "hardware", :action => "storage_pools_json", :id => (hwpool.nil? ? nil : hwpool.id), :exclude_pool => exclude_pool %>',
>      dataType: 'json',
>      colModel : [
>          {display: '', width : 20, align: 'left', process: <%= table_id %>checkbox},
>          {display: 'Alias', width : 180, align: 'left'},
> -        <%= "{display: 'Hardware Pool', name : 'pools.name', width : 100, align: 'left'}," if exclude_id %>
> +        <%= "{display: 'Hardware Pool', name : 'pools.name', width : 100, align: 'left'}," if exclude_pool %>
>          {display: 'IP', name : 'ip_addr', width : 80, align: 'left'},
>          {display: 'Type', name : 'storage_pools.type', width : 80, align: 'left'}
>          ],
> diff --git a/wui/src/app/views/storage/add.rhtml b/wui/src/app/views/storage/add.rhtml
> index 5dd70e6..11cda06 100644
> --- a/wui/src/app/views/storage/add.rhtml
> +++ b/wui/src/app/views/storage/add.rhtml
> @@ -3,7 +3,7 @@
>  <div class="dialog_body_small">
>  <div class="panel_header"></div>
>    <%= render :partial => "/storage/grid", :locals => { :table_id => "addstorage_grid",
> -             :hwpool => nil, :exclude_id => @hardware_pool.id,
> +             :hwpool => nil, :exclude_pool => @hardware_pool.id,
>               :on_select => "false" } %>
>  </div>
>  <%= popup_footer("add_storage('#{url_for :controller => 'hardware', 
> diff --git a/wui/src/app/views/vm/migrate.rhtml b/wui/src/app/views/vm/migrate.rhtml
> new file mode 100644
> index 0000000..f94723a
> --- /dev/null
> +++ b/wui/src/app/views/vm/migrate.rhtml
> @@ -0,0 +1,81 @@
> +<%- content_for :title do -%>
> +  Migrate Virtual Machine
> +<%- end -%>
> +<%- content_for :description do -%>
> +  Please choose migration destination. Leave the selection blank to allow oVirt to choose the most appropriate destination host.
> +<%- end -%>
> +<script type="text/javascript">
> +<%= popup_footer("migrate_vm()", "Migrate") %>
> +  function migrate_vm_select(selected_rows)
> +  {
> +    var selected_ids = new Array();
> +    for(i=0; i<selected_rows.length; i++) {
> +      load_widget_select(selected_rows[i]);
> +      selected_ids[i] = selected_rows[i].id;
> +    }
> +    if (selected_ids.length == 1)
> +    {
> +      $('#selected_migration_target').load('<%= url_for :controller => "host", :action => "quick_summary" %>',
> +                { id: parseInt(selected_ids[0].substring(3))});
> +      $('#vm_action_data').val(selected_ids[0].substring(3));
> +    }
> +  }
> +  function migrate_vm_deselect(selected_rows)
> +  {
> +    var selected_ids = new Array()
> +    for(i=0; i<selected_rows.length; i++) {
> +      load_widget_deselect(selected_rows[i]);
> +      selected_ids[i] = selected_rows[i].id;
> +    }
> +    refresh_summary_static('selected_migration_target', '<div class="selection_left"> \
> +    <div>No migration target selected.</div> \
> +    </div>')
> +    $('#vm_action_data').val('')
> +  }
> +</script>
> +<div class="panel_header"></div>
> +
> +
> +<form method="POST" id="migrate_vm_form" action="<%= url_for :action => 'vm_action' %>" >
> +<div class="dialog_form">
> +    <%= error_messages_for 'migrate_vm' %>
> +
> +  <%= render :partial => "/host/grid", :locals => { :table_id => "migrate_vm_grid",
> +             :hwpool => @vm.get_hardware_pool,
> +             :exclude_pool => nil,
> +             :exclude_host => @vm.host_id,
> +             :checkboxes => false,
> +             :on_select => "migrate_vm_select",
> +             :on_deselect => "migrate_vm_deselect",
> +             :on_hover => "load_widget_hover",
> +             :on_unhover => "load_widget_unhover"  } %>
> +
> +  <% form_tag  do %>
> +    <!--[form:migrate_vm]-->
> +    <%= hidden_field_tag 'id', @vm.id %>
> +    <%= hidden_field_tag 'vm_action', VmTask::ACTION_MIGRATE_VM %>
> +    Selected Migration Target:
> +    <div id='selected_migration_target'>
> +      <div class="selection_left">
> +        <div>No migration target selected.</div>
> +      </div>
> +    </div>
> +    <%= hidden_field_tag 'vm_action_data', "" %>
> +  <% end %>
> +</div>
> +<%= popup_footer("$('#migrate_vm_form').submit()", "Migrate Virtual Machine") %>
> +</form>
> +<script type="text/javascript">
> +$(function() {
> +    var vmoptions = {
> +        target:        '<%= url_for :action => 'create' %>',   // target element to update
> +        //beforeSubmit:  showStorageRequest,  // pre-submit callback
> +	dataType:      'json',
> +        success:       afterVm  // post-submit callback
> +    };
> +
> +    // bind form using 'ajaxForm'
> +    $('#migrate_vm_form').ajaxForm(vmoptions);
> +});
> +</script>
> +
> diff --git a/wui/src/app/views/vm/show.rhtml b/wui/src/app/views/vm/show.rhtml
> index 8b0a760..fe671ef 100644
> --- a/wui/src/app/views/vm/show.rhtml
> +++ b/wui/src/app/views/vm/show.rhtml
> @@ -17,10 +17,17 @@
>      <%= link_to image_tag("icon_edit.png") + " Edit",
>                            {:controller => 'vm', :action => 'edit', :id => @vm},
>                            :rel=>"facebox[.bolder]", :class=>"selection_facebox" %>
> -    <% for action in @actions %>
> -      <a href="#" onClick="single_vm_action('<%= action[1] %>')">
> -        <%= image_tag action[2] %> <%= action[0] %>
> -      </a> 
> +    <% for name, action in @actions %>
> +      <% if action[:popup_action] -%>
> +        <%= link_to image_tag(action[:icon]) + action[:label],
> +                          {:controller => 'vm',
> +                           :action => action[:popup_action], :id => @vm},
> +                          :rel=>"facebox[.bolder]", :class=>"selection_facebox" %>
> +      <% else -%>
> +        <a href="#" onClick="single_vm_action('<%= name %>')">
> +          <%= image_tag action[:icon] %> <%= action[:label] %>
> +        </a>
> +      <% end -%>
>      <% end %>
>      <a href="#" onClick="cancel_queued_tasks()">
>        <%= image_tag "icon_x.png" %> Cancel queued tasks

I had trouble applying this (manually dropped in the 4 lines), but it
seems to have worked for steve, so hopefully it is just me

> diff --git a/wui/src/public/javascripts/flexigrid.js b/wui/src/public/javascripts/flexigrid.js
> index d7b6416..3715e6f 100644
> --- a/wui/src/public/javascripts/flexigrid.js
> +++ b/wui/src/public/javascripts/flexigrid.js
> @@ -719,7 +719,9 @@
>  									    } 
>                                          $(this).toggleClass('trSelected');
>                                          if($(this).hasClass('trSelected')){
> -									        if (p.onSelect) p.onSelect($(t).find("tr.trSelected"));
> +                                            if (p.onSelect) p.onSelect($(t).find("tr.trSelected"));
> +                                        } else {
> +                                            if (p.onDeselect) p.onDeselect(this);
>                                          }
>  									}
>  							)
> diff --git a/wui/src/public/javascripts/ovirt.js b/wui/src/public/javascripts/ovirt.js
> index c776458..e0aa222 100644
> --- a/wui/src/public/javascripts/ovirt.js
> +++ b/wui/src/public/javascripts/ovirt.js
> @@ -74,7 +74,7 @@ function ajax_validation(response, status)
>    if (response.object) {
>      $(".fieldWithErrors").removeClass("fieldWithErrors");
>      $("div.errorExplanation").remove();
> -    if (!response.success) {
> +    if (!response.success && response.errors ) {
>        for(i=0; i<response.errors.length; i++) { 
>          var element = $("div.form_field:has(#"+response.object + "_" + response.errors[i][0]+")");
>          if (element) {




More information about the ovirt-devel mailing list