Ovirt devel - Jun 2008 - [PATCH](resend) added logic around when to show the 'clear host' link:

Resending after rebase:

link will show unless:
1) there's already a pending clear VM task for this host or
2) host is disabled and there are no VMs running or
3) host is unavailable

Signed-off-by: Scott Seago <sseago at redhat.com>
---
 wui/src/app/controllers/vm_controller.rb |    2 +-
 wui/src/app/models/host.rb               |   30 ++++++++++++++++++++++++++++--
 wui/src/app/models/storage_pool.rb       |   10 +++++-----
 wui/src/app/models/vm.rb                 |   24 ++++++------------------
 wui/src/app/views/host/show.rhtml        |   16 +++++++++-------
 wui/src/host-browser/host-browser.rb     |    2 +-
 wui/src/host-status/host-status.rb       |    8 ++++----
 7 files changed, 54 insertions(+), 38 deletions(-)

diff --git a/wui/src/app/controllers/vm_controller.rb
b/wui/src/app/controllers/vm_controller.rb
index c12686a..b9156d2 100644
--- a/wui/src/app/controllers/vm_controller.rb
+++ b/wui/src/app/controllers/vm_controller.rb
@@ -163,7 +163,7 @@ class VmController < ApplicationController
   def cancel_queued_tasks
     begin
       Task.transaction do
-        @vm.get_queued_tasks.each { |task| task.cancel}
+        @vm.tasks.queued.each { |task| task.cancel}
       end
       render :json => { :object => "vm", :success => true,
:alert => "queued tasks were canceled." }
     rescue
diff --git a/wui/src/app/models/host.rb b/wui/src/app/models/host.rb
index efee14a..6917226 100644
--- a/wui/src/app/models/host.rb
+++ b/wui/src/app/models/host.rb
@@ -22,10 +22,26 @@ require 'util/ovirt'
 class Host < ActiveRecord::Base
   belongs_to :hardware_pool
   has_many :nics, :dependent => :destroy
-  has_many :vms, :dependent => :nullify
+  has_many :vms, :dependent => :nullify do
+    def consuming_resources
+      find(:all, :conditions=>{:state=>Vm::RUNNING_STATES})
+    end
+  end
+  has_many :tasks, :class_name => "HostTask", :dependent =>
:destroy, :order => "id DESC" do
+    def queued
+      find(:all, :conditions=>{:state=>Task::STATE_QUEUED})
+    end
+    def pending_clear_tasks
+      find(:all, :conditions=>{:state=>Task::WORKING_STATES, 
+                               :action=>HostTask::ACTION_CLEAR_VMS})
+    end
+  end
 
   KVM_HYPERVISOR_TYPE = "KVM"
   HYPERVISOR_TYPES = [KVM_HYPERVISOR_TYPE]
+  STATE_UNAVAILABLE = "unavailable"
+  STATE_AVAILABLE   = "available"
+  STATES = [STATE_UNAVAILABLE, STATE_AVAILABLE]
   def memory_in_mb
     kb_to_mb(memory)
   end
@@ -33,6 +49,16 @@ class Host < ActiveRecord::Base
     self[:memory]=(mb_to_kb(mem))
   end
   def status_str
-    "#{state} (#{(is_disabled.nil? or is_disabled==0) ?
'enabled':'disabled'})"
+    "#{state} (#{disabled? ? 'disabled':'enabled'})"
+  end
+
+  def disabled?
+    not(is_disabled.nil? or is_disabled==0)
+  end
+
+  def is_clear_task_valid?
+    state==STATE_AVAILABLE and
+      not(disabled? and vms.consuming_resources.empty?) and
+      tasks.pending_clear_tasks.empty?
   end
 end
diff --git a/wui/src/app/models/storage_pool.rb
b/wui/src/app/models/storage_pool.rb
index 27479ee..a135047 100644
--- a/wui/src/app/models/storage_pool.rb
+++ b/wui/src/app/models/storage_pool.rb
@@ -19,7 +19,11 @@
 
 class StoragePool < ActiveRecord::Base
   belongs_to              :hardware_pool
-  has_many                :storage_tasks, :dependent => :destroy, :order
=> "id DESC"
+  has_many :tasks, :class_name => "StorageTask", :dependent =>
:destroy, :order => "id DESC" do
+    def queued
+      find(:all, :conditions=>{:state=>Task::STATE_QUEUED})
+    end
+  end
   has_many                :storage_volumes, :dependent => :destroy, :include
=> :storage_pool do
     def total_size_in_gb
       find(:all).inject(0){ |sum, sv| sum + sv.size_in_gb }
@@ -51,8 +55,4 @@ class StoragePool < ActiveRecord::Base
   def get_type_label
     STORAGE_TYPES.invert[self.class.name.gsub("StoragePool",
"")]
   end
-
-  def tasks
-    storage_tasks
-  end
 end
diff --git a/wui/src/app/models/vm.rb b/wui/src/app/models/vm.rb
index ae4e8ff..617512e 100644
--- a/wui/src/app/models/vm.rb
+++ b/wui/src/app/models/vm.rb
@@ -22,7 +22,11 @@ require 'util/ovirt'
 class Vm < ActiveRecord::Base
   belongs_to :vm_resource_pool
   belongs_to :host
-  has_many :vm_tasks, :dependent => :destroy, :order => "id
DESC"
+  has_many :tasks, :class_name => "VmTask", :dependent =>
:destroy, :order => "id DESC" do
+    def queued
+      find(:all, :conditions=>{:state=>Task::STATE_QUEUED})
+    end
+  end
   has_and_belongs_to_many :storage_volumes
   validates_presence_of :uuid, :description, :num_vcpus_allocated,
                         :memory_allocated_in_mb, :memory_allocated,
:vnic_mac_addr
@@ -107,7 +111,7 @@ class Vm < ActiveRecord::Base
   def get_pending_state
     pending_state = state
     pending_state = EFFECTIVE_STATE[state] if pending_state
-    get_queued_tasks.each do |task|
+    tasks.queued.each do |task|
       return STATE_INVALID unless VmTask::ACTIONS[task.action][:start] ==
pending_state
       pending_state = VmTask::ACTIONS[task.action][:success]
     end
@@ -122,18 +126,6 @@ class Vm < ActiveRecord::Base
     RUNNING_STATES.include?(get_pending_state)
   end
 
-  def get_queued_tasks(state=nil)
-    get_tasks(Task::STATE_QUEUED)
-  end
-
-  def get_tasks(state=nil)
-    conditions = "vm_id = '#{id}'"
-    conditions += " AND state = '#{Task::STATE_QUEUED}'" if
state
-    VmTask.find(:all, 
-              :conditions => conditions,
-              :order => "id")
-  end    
-
   def get_action_list
     # return empty list rather than nil
     return_val = VmTask::VALID_ACTIONS_PER_VM_STATE[get_pending_state] || []
@@ -166,10 +158,6 @@ class Vm < ActiveRecord::Base
     return return_val
   end
 
-  def tasks
-    vm_tasks
-  end
-
   def queue_action(user, action)
     return false unless get_action_list.include?(action)
     task = VmTask.new({ :user    => user,
diff --git a/wui/src/app/views/host/show.rhtml
b/wui/src/app/views/host/show.rhtml
index 619a126..e1bafac 100644
--- a/wui/src/app/views/host/show.rhtml
+++ b/wui/src/app/views/host/show.rhtml
@@ -3,18 +3,20 @@
 <%- end -%>
 <%- content_for :action_links do -%>
   <%if @can_modify -%>
-    <%if @host.is_disabled.nil? or @host.is_disabled == 0  -%>
+    <%if @host.disabled?  -%>
+      <a href="#"
onClick="host_action('enable')">
+        <%= image_tag "icon_start.png" %> Enable Host
+      </a> 
+    <% else -%>
       <a href="#"
onClick="host_action('disable')">
         <%= image_tag "icon_suspend.png" %> Disable Host
       </a> 
-    <% else -%>
-      <a href="#"
onClick="host_action('enable')">
-        <%= image_tag "icon_start.png" %> Enable Host
+    <% end -%>
+    <%if @host.is_clear_task_valid? -%>
+      <a href="#"
onClick="host_action('clear_vms')">
+        <%= image_tag "icon_x.png" %> Clear VMs
       </a> 
     <% end -%>
-    <a href="#"
onClick="host_action('clear_vms')">
-      <%= image_tag "icon_x.png" %> Clear VMs
-    </a> 
   <%- end -%>
 <%- end -%>
 <script type="text/javascript">
diff --git a/wui/src/host-browser/host-browser.rb
b/wui/src/host-browser/host-browser.rb
index bcb7d60..691e4ed 100755
--- a/wui/src/host-browser/host-browser.rb
+++ b/wui/src/host-browser/host-browser.rb
@@ -124,7 +124,7 @@ class HostBrowser
                     "hardware_pool"   =>
HardwarePool.get_default_pool,
                     # Let host-status mark it available when it
                     # successfully connects to it via libvirt.
-                    "state"           =>
"unavailable").save
+                    "state"           =>
Host::STATE_UNAVAILABLE).save
             rescue Exception => error
                 puts "Error while creating record: #{error.message}"
unless defined?(TESTING)
             end
diff --git a/wui/src/host-status/host-status.rb
b/wui/src/host-status/host-status.rb
index fcfd586..7908da7 100755
--- a/wui/src/host-status/host-status.rb
+++ b/wui/src/host-status/host-status.rb
@@ -110,9 +110,9 @@ def check_status(host)
     # we couldn't contact the host for whatever reason.  Since we can't
get
     # to this host, we have to mark all vms on it as disconnected or stopped
     # or such.
-    if host.state != "unavailable"
+    if host.state != Host::STATE_UNAVAILABLE
       puts "Updating host state to unavailable: " + host.hostname
-      host.state = "unavailable"
+      host.state = Host::STATE_UNAVAILABLE
       host.save
     end
 
@@ -135,9 +135,9 @@ def check_status(host)
     return
   end
 
-  if host.state != "available"
+  if host.state != Host::STATE_AVAILABLE
     puts "Updating host state to available: " + host.hostname
-    host.state = "available"
+    host.state = Host::STATE_AVAILABLE
     host.save
   end
 
-- 
1.5.5.1

On Mon, 2008-06-30 at 09:32 -0400, Scott Seago wrote:
> Resending after rebase:
> 
> link will show unless:
> 1) there's already a pending clear VM task for this host or
> 2) host is disabled and there are no VMs running or
> 3) host is unavailable
> 
> Signed-off-by: Scott Seago <sseago at redhat.com>
> ---
>  wui/src/app/controllers/vm_controller.rb |    2 +-
>  wui/src/app/models/host.rb               |   30
++++++++++++++++++++++++++++--
>  wui/src/app/models/storage_pool.rb       |   10 +++++-----
>  wui/src/app/models/vm.rb                 |   24 ++++++------------------
>  wui/src/app/views/host/show.rhtml        |   16 +++++++++-------
>  wui/src/host-browser/host-browser.rb     |    2 +-
>  wui/src/host-status/host-status.rb       |    8 ++++----
>  7 files changed, 54 insertions(+), 38 deletions(-)
> 
> diff --git a/wui/src/app/controllers/vm_controller.rb
b/wui/src/app/controllers/vm_controller.rb
> index c12686a..b9156d2 100644
> --- a/wui/src/app/controllers/vm_controller.rb
> +++ b/wui/src/app/controllers/vm_controller.rb
> @@ -163,7 +163,7 @@ class VmController < ApplicationController
>    def cancel_queued_tasks
>      begin
>        Task.transaction do
> -        @vm.get_queued_tasks.each { |task| task.cancel}
> +        @vm.tasks.queued.each { |task| task.cancel}
>        end
>        render :json => { :object => "vm", :success =>
true, :alert => "queued tasks were canceled." }
>      rescue
> diff --git a/wui/src/app/models/host.rb b/wui/src/app/models/host.rb
> index efee14a..6917226 100644
> --- a/wui/src/app/models/host.rb
> +++ b/wui/src/app/models/host.rb
> @@ -22,10 +22,26 @@ require 'util/ovirt'
>  class Host < ActiveRecord::Base
>    belongs_to :hardware_pool
>    has_many :nics, :dependent => :destroy
> -  has_many :vms, :dependent => :nullify
> +  has_many :vms, :dependent => :nullify do
> +    def consuming_resources
> +      find(:all, :conditions=>{:state=>Vm::RUNNING_STATES})
> +    end
> +  end
> +  has_many :tasks, :class_name => "HostTask", :dependent
=> :destroy, :order => "id DESC" do
> +    def queued
> +      find(:all, :conditions=>{:state=>Task::STATE_QUEUED})
> +    end
> +    def pending_clear_tasks
> +      find(:all, :conditions=>{:state=>Task::WORKING_STATES, 
> +                               :action=>HostTask::ACTION_CLEAR_VMS})
> +    end
> +  end
>  
>    KVM_HYPERVISOR_TYPE = "KVM"
>    HYPERVISOR_TYPES = [KVM_HYPERVISOR_TYPE]
> +  STATE_UNAVAILABLE = "unavailable"
> +  STATE_AVAILABLE   = "available"
> +  STATES = [STATE_UNAVAILABLE, STATE_AVAILABLE]
>    def memory_in_mb
>      kb_to_mb(memory)
>    end
> @@ -33,6 +49,16 @@ class Host < ActiveRecord::Base
>      self[:memory]=(mb_to_kb(mem))
>    end
>    def status_str
> -    "#{state} (#{(is_disabled.nil? or is_disabled==0) ?
'enabled':'disabled'})"
> +    "#{state} (#{disabled? ?
'disabled':'enabled'})"
> +  end
> +
> +  def disabled?
> +    not(is_disabled.nil? or is_disabled==0)
> +  end
> +
> +  def is_clear_task_valid?
> +    state==STATE_AVAILABLE and
> +      not(disabled? and vms.consuming_resources.empty?) and
> +      tasks.pending_clear_tasks.empty?
>    end
>  end
> diff --git a/wui/src/app/models/storage_pool.rb
b/wui/src/app/models/storage_pool.rb
> index 27479ee..a135047 100644
> --- a/wui/src/app/models/storage_pool.rb
> +++ b/wui/src/app/models/storage_pool.rb
> @@ -19,7 +19,11 @@
>  
>  class StoragePool < ActiveRecord::Base
>    belongs_to              :hardware_pool
> -  has_many                :storage_tasks, :dependent => :destroy,
:order => "id DESC"
> +  has_many :tasks, :class_name => "StorageTask", :dependent
=> :destroy, :order => "id DESC" do
> +    def queued
> +      find(:all, :conditions=>{:state=>Task::STATE_QUEUED})
> +    end
> +  end
>    has_many                :storage_volumes, :dependent => :destroy,
:include => :storage_pool do
>      def total_size_in_gb
>        find(:all).inject(0){ |sum, sv| sum + sv.size_in_gb }
> @@ -51,8 +55,4 @@ class StoragePool < ActiveRecord::Base
>    def get_type_label
>      STORAGE_TYPES.invert[self.class.name.gsub("StoragePool",
"")]
>    end
> -
> -  def tasks
> -    storage_tasks
> -  end
>  end
> diff --git a/wui/src/app/models/vm.rb b/wui/src/app/models/vm.rb
> index ae4e8ff..617512e 100644
> --- a/wui/src/app/models/vm.rb
> +++ b/wui/src/app/models/vm.rb
> @@ -22,7 +22,11 @@ require 'util/ovirt'
>  class Vm < ActiveRecord::Base
>    belongs_to :vm_resource_pool
>    belongs_to :host
> -  has_many :vm_tasks, :dependent => :destroy, :order => "id
DESC"
> +  has_many :tasks, :class_name => "VmTask", :dependent =>
:destroy, :order => "id DESC" do
> +    def queued
> +      find(:all, :conditions=>{:state=>Task::STATE_QUEUED})
> +    end
> +  end
>    has_and_belongs_to_many :storage_volumes
>    validates_presence_of :uuid, :description, :num_vcpus_allocated,
>                          :memory_allocated_in_mb, :memory_allocated,
:vnic_mac_addr
> @@ -107,7 +111,7 @@ class Vm < ActiveRecord::Base
>    def get_pending_state
>      pending_state = state
>      pending_state = EFFECTIVE_STATE[state] if pending_state
> -    get_queued_tasks.each do |task|
> +    tasks.queued.each do |task|
>        return STATE_INVALID unless VmTask::ACTIONS[task.action][:start] ==
pending_state
>        pending_state = VmTask::ACTIONS[task.action][:success]
>      end
> @@ -122,18 +126,6 @@ class Vm < ActiveRecord::Base
>      RUNNING_STATES.include?(get_pending_state)
>    end
>  
> -  def get_queued_tasks(state=nil)
> -    get_tasks(Task::STATE_QUEUED)
> -  end
> -
> -  def get_tasks(state=nil)
> -    conditions = "vm_id = '#{id}'"
> -    conditions += " AND state = '#{Task::STATE_QUEUED}'"
if state
> -    VmTask.find(:all, 
> -              :conditions => conditions,
> -              :order => "id")
> -  end    
> -
>    def get_action_list
>      # return empty list rather than nil
>      return_val = VmTask::VALID_ACTIONS_PER_VM_STATE[get_pending_state] ||
[]
> @@ -166,10 +158,6 @@ class Vm < ActiveRecord::Base
>      return return_val
>    end
>  
> -  def tasks
> -    vm_tasks
> -  end
> -
>    def queue_action(user, action)
>      return false unless get_action_list.include?(action)
>      task = VmTask.new({ :user    => user,
> diff --git a/wui/src/app/views/host/show.rhtml
b/wui/src/app/views/host/show.rhtml
> index 619a126..e1bafac 100644
> --- a/wui/src/app/views/host/show.rhtml
> +++ b/wui/src/app/views/host/show.rhtml
> @@ -3,18 +3,20 @@
>  <%- end -%>
>  <%- content_for :action_links do -%>
>    <%if @can_modify -%>
> -    <%if @host.is_disabled.nil? or @host.is_disabled == 0  -%>
> +    <%if @host.disabled?  -%>
> +      <a href="#"
onClick="host_action('enable')">
> +        <%= image_tag "icon_start.png" %> Enable Host
> +      </a> 
> +    <% else -%>
>        <a href="#"
onClick="host_action('disable')">
>          <%= image_tag "icon_suspend.png" %> Disable Host
>        </a> 
> -    <% else -%>
> -      <a href="#"
onClick="host_action('enable')">
> -        <%= image_tag "icon_start.png" %> Enable Host
> +    <% end -%>
> +    <%if @host.is_clear_task_valid? -%>
> +      <a href="#"
onClick="host_action('clear_vms')">
> +        <%= image_tag "icon_x.png" %> Clear VMs
>        </a> 
>      <% end -%>
> -    <a href="#"
onClick="host_action('clear_vms')">
> -      <%= image_tag "icon_x.png" %> Clear VMs
> -    </a> 
>    <%- end -%>
>  <%- end -%>
>  <script type="text/javascript">
> diff --git a/wui/src/host-browser/host-browser.rb
b/wui/src/host-browser/host-browser.rb
> index bcb7d60..691e4ed 100755
> --- a/wui/src/host-browser/host-browser.rb
> +++ b/wui/src/host-browser/host-browser.rb
> @@ -124,7 +124,7 @@ class HostBrowser
>                      "hardware_pool"   =>
HardwarePool.get_default_pool,
>                      # Let host-status mark it available when it
>                      # successfully connects to it via libvirt.
> -                    "state"           =>
"unavailable").save
> +                    "state"           =>
Host::STATE_UNAVAILABLE).save
>              rescue Exception => error
>                  puts "Error while creating record:
#{error.message}" unless defined?(TESTING)
>              end
> diff --git a/wui/src/host-status/host-status.rb
b/wui/src/host-status/host-status.rb
> index fcfd586..7908da7 100755
> --- a/wui/src/host-status/host-status.rb
> +++ b/wui/src/host-status/host-status.rb
> @@ -110,9 +110,9 @@ def check_status(host)
>      # we couldn't contact the host for whatever reason.  Since we
can't get
>      # to this host, we have to mark all vms on it as disconnected or
stopped
>      # or such.
> -    if host.state != "unavailable"
> +    if host.state != Host::STATE_UNAVAILABLE
>        puts "Updating host state to unavailable: " +
host.hostname
> -      host.state = "unavailable"
> +      host.state = Host::STATE_UNAVAILABLE
>        host.save
>      end
>  
> @@ -135,9 +135,9 @@ def check_status(host)
>      return
>    end
>  
> -  if host.state != "available"
> +  if host.state != Host::STATE_AVAILABLE
>      puts "Updating host state to available: " + host.hostname
> -    host.state = "available"
> +    host.state = Host::STATE_AVAILABLE
>      host.save
>    end
>  
ACK, on testing, this appears to work as desired.

-j