This iteration of the host controller refactoring changes error handling once again, since the previous patch had some serious errors in that area. We now have a catch-all error handler, responding to Exception, that pulls validation errors out of the instance variables of the controller. In the future, we might want to refine that to handle validation errors separately from Exception (handling Exception really points to a bug in the app) David
David Lutterkort
2009-May-05  19:03 UTC
[Ovirt-devel] [PATCH server 1/2] Generic error handling
Instead of explicit rescue blocks in actions, we now register error
handlers for each exception (with a catch-all handler for Exception) The
handlers generate an appropriate response for each exception and output
method.
Remove handle_auth_error and json_error, since their functionality is now
covered by handle_perm_error and handle_general_error
---
 src/app/controllers/application.rb               |   81 +++++++++++++++-----
 src/app/controllers/hardware_controller.rb       |   37 +++-------
 src/app/controllers/pool_controller.rb           |   88 ++++++++--------------
 src/app/controllers/resources_controller.rb      |    4 +-
 src/app/controllers/smart_pools_controller.rb    |   47 ++----------
 src/app/controllers/storage_volume_controller.rb |    5 +-
 src/app/controllers/vm_controller.rb             |   61 ++++-----------
 7 files changed, 131 insertions(+), 192 deletions(-)
diff --git a/src/app/controllers/application.rb
b/src/app/controllers/application.rb
index bdd5b64..6932a1f 100644
--- a/src/app/controllers/application.rb
+++ b/src/app/controllers/application.rb
@@ -46,6 +46,10 @@ class ApplicationController < ActionController::Base
   before_filter :tmp_authorize_admin, :only => [:create, :update, :destroy]
   before_filter :is_logged_in, :get_help_section
 
+  # General error handlers, must be in order from least specific
+  # to most specific
+  rescue_from Exception, :with => :handle_general_error
+  rescue_from PermissionError, :with => :handle_perm_error
 
   def choose_layout
     if(params[:component_layout])
@@ -107,28 +111,38 @@ class ApplicationController < ActionController::Base
   def authorize_action(privilege, msg=nil)
     msg ||= 'You have insufficient privileges to perform action.'
     unless authorized?(privilege)
-      handle_auth_error(msg)
+      handle_error(:message => msg,
+                   :title => "Access Denied", :status =>
:forbidden)
       false
     else
       true
     end
   end
-  def handle_auth_error(msg)
+
+  def handle_perm_error(error)
+    handle_error(:error => error, :status => :forbidden,
+                 :title => "Access denied")
+  end
+
+  def handle_general_error(error)
+    handle_error(:error => error, :status => :internal_server_error,
+                 :title => "Internal Server Error")
+  end
+
+  def handle_error(hash)
+    log_error(hash[:error])
+    msg = hash[:message] || hash[:error].message
+    title = hash[:title] || "Internal Server Error"
+    status = hash[:status] || :internal_server_error
     respond_to do |format|
-      format.html do
-        html_error_page(msg)
-      end
-      format.json do
-        @json_hash ||= {}
-        @json_hash[:success] = false
-        @json_hash[:alert] = msg
-        render :json => @json_hash
-      end
-      format.xml { head :forbidden }
+      format.html { html_error_page(title, msg) }
+      format.json { render :json => json_error_hash(msg, status) }
+      format.xml { render :xml => xml_errors(msg), :status => status }
     end
   end
-  def html_error_page(msg)
-    @title = "Access denied"
+
+  def html_error_page(title, msg)
+    @title = title
     @errmsg = msg
     @ajax = params[:ajax]
     @nolayout = params[:nolayout]
@@ -140,6 +154,7 @@ class ApplicationController < ActionController::Base
       render :template => 'layouts/popup-error', :layout =>
'popup'
     end
   end
+
   # don't define find_opts for array inputs
   def json_hash(full_items, attributes, arg_list=[], find_opts={},
id_method=:id)
     page = params[:page].to_i
@@ -175,11 +190,37 @@ class ApplicationController < ActionController::Base
     render :json => json_hash(full_items, attributes, arg_list, find_opts,
id_method).to_json
   end
 
-  def json_error(obj_type, obj, exception)
-    json_hash = { :object => obj_type, :success => false}
-    json_hash[:errors] = obj.errors.localize_error_messages.to_a if obj
-    json_hash[:alert] = exception.message if obj.errors.size == 0
-    render :json => json_hash
+  private
+  def json_error_hash(msg, status)
+    json = {}
+    json[:success] = (status == :ok)
+    json.merge!(instance_errors)
+    # There's a potential issue here: if we add :errors for an object
+    # that the view won't generate inline error messages for, the user
+    # won't get any indication what the error is. But if we set :alert
+    # unconditionally, the user will get validation errors twice: once
+    # inline in the form, and once in the flash
+    json[:alert] = msg unless json[:errors]
+    return json
+  end
+
+  def xml_errors(msg)
+    xml = {}
+    xml[:message] = msg
+    xml.merge!(instance_errors)
+    return xml
+  end
+
+  def instance_errors
+    hash = {}
+    instance_variables.each do |ivar|
+      val = instance_variable_get(ivar)
+      if val && val.respond_to?(:errors) && val.errors.size
> 0
+        hash[:object] = ivar[1, ivar.size]
+        hash[:errors] ||= []
+        hash[:errors] += val.errors.localize_error_messages.to_a
+      end
+    end
+    return hash
   end
-
 end
diff --git a/src/app/controllers/hardware_controller.rb
b/src/app/controllers/hardware_controller.rb
index 56a2c47..0b6cf9b 100644
--- a/src/app/controllers/hardware_controller.rb
+++ b/src/app/controllers/hardware_controller.rb
@@ -111,14 +111,10 @@ class HardwareController < PoolController
   end
 
   def show_storage
-    begin
-      svc_show(params[:id])
-      @storage_tree = @pool.storage_tree(:filter_unavailable => false,
-                                         :include_used => true).to_json
-      render_show
-    rescue PermissionError => perm_error
-      handle_auth_error(perm_error.message)
-    end
+    svc_show(params[:id])
+    @storage_tree = @pool.storage_tree(:filter_unavailable => false,
+                                       :include_used => true).to_json
+    render_show
   end
 
   def show_tasks
@@ -219,24 +215,13 @@ class HardwareController < PoolController
   end
 
   def edit_items(svc_method, target_pool_id, item_action)
-    begin
-      alert = send(svc_method, params[:id],
params[:resource_ids].split(","),
-                   target_pool_id)
-      render :json => { :success => true, :alert => alert,
-                        :storage => @pool.storage_tree({:filter_unavailable
=>
-                                                        false,
-                                                        :include_used =>
true,
-                                                        :state =>
-                                                        item_action.to_s})}
-    rescue PermissionError => perm_error
-      handle_auth_error(perm_error.message)
-      # If we need to give more details as to which hosts/storage succeeded,
-      # they're in the exception
-    rescue PartialSuccessError => error
-      render :json => { :success => false, :alert => error.message }
-    rescue Exception => ex
-      render :json => { :success => false, :alert => error.message }
-    end
+    alert = send(svc_method, params[:id],
params[:resource_ids].split(","),
+                 target_pool_id)
+    render :json => { :success => true, :alert => alert,
+      :storage => @pool.storage_tree({:filter_unavailable => false,
+                                       :include_used => true,
+                                       :state =>
+                                       item_action.to_s})}
   end
 
   def removestorage
diff --git a/src/app/controllers/pool_controller.rb
b/src/app/controllers/pool_controller.rb
index 911bc92..6e41ac6 100644
--- a/src/app/controllers/pool_controller.rb
+++ b/src/app/controllers/pool_controller.rb
@@ -37,12 +37,8 @@ class PoolController < ApplicationController
   end
 
   def show
-    begin
-      svc_show(params[:id])
-      render_show
-    rescue PermissionError => perm_error
-      handle_auth_error(perm_error.message)
-    end
+    svc_show(params[:id])
+    render_show
   end
 
   def render_show
@@ -58,12 +54,8 @@ class PoolController < ApplicationController
 
   end
   def quick_summary
-    begin
-      svc_show(params[:id])
-      render :layout => 'selection'
-    rescue PermissionError => perm_error
-      handle_auth_error(perm_error.message)
-    end
+    svc_show(params[:id])
+    render :layout => 'selection'
   end
 
   # resource's users list page
@@ -111,56 +103,36 @@ class PoolController < ApplicationController
   def create
     # FIXME: REST and browsers send params differently. Should be fixed
     # in the views
-    begin
-      alert = svc_create(params[:pool] ? params[:pool] :
params[:hardware_pool],
-                         additional_create_params)
-      respond_to do |format|
-        format.json {
-          reply = { :object => "pool", :success => true,
-            :alert => alert }
-          reply[:resource_type] = params[:resource_type] if
params[:resource_type]
-          render :json => reply
-        }
-        format.xml {
-          render :xml => @pool.to_xml(XML_OPTS),
-          :status => :created,
-          :location => hardware_pool_url(@pool)
-        }
-      end
-    rescue PermissionError => perm_error
-      handle_auth_error(perm_error.message)
-    rescue Exception => ex
-      respond_to do |format|
-        format.json { json_error("pool", @pool, ex) }
-        format.xml  { render :xml => @pool.errors,
-          :status => :unprocessable_entity }
-      end
+    alert = svc_create(params[:pool] ? params[:pool] : params[:hardware_pool],
+                       additional_create_params)
+    respond_to do |format|
+      format.json {
+        reply = { :object => "pool", :success => true,
+          :alert => alert }
+        reply[:resource_type] = params[:resource_type] if
params[:resource_type]
+        render :json => reply
+      }
+      format.xml {
+        render :xml => @pool.to_xml(XML_OPTS),
+        :status => :created,
+        :location => hardware_pool_url(@pool)
+      }
     end
   end
 
   def update
-    begin
-      alert = svc_update(params[:id], params[:pool] ? params[:pool] :
-                                      params[:hardware_pool])
-      respond_to do |format|
-        format.json {
-          reply = { :object => "pool", :success => true, :alert
=> alert }
-          render :json => reply
-        }
-        format.xml {
-          render :xml => @pool.to_xml(XML_OPTS),
-          :status => :created,
-          :location => hardware_pool_url(@pool)
-        }
-      end
-    rescue PermissionError => perm_error
-      handle_auth_error(perm_error.message)
-    rescue Exception => ex
-      respond_to do |format|
-        format.json { json_error("pool", @pool, ex) }
-        format.xml  { render :xml => @pool.errors,
-          :status => :unprocessable_entity }
-      end
+    alert = svc_update(params[:id], params[:pool] ? params[:pool] :
+                       params[:hardware_pool])
+    respond_to do |format|
+      format.json {
+        reply = { :object => "pool", :success => true, :alert
=> alert }
+        render :json => reply
+      }
+      format.xml {
+        render :xml => @pool.to_xml(XML_OPTS),
+        :status => :created,
+        :location => hardware_pool_url(@pool)
+      }
     end
   end
 
diff --git a/src/app/controllers/resources_controller.rb
b/src/app/controllers/resources_controller.rb
index 7ad79ca..edee59d 100644
--- a/src/app/controllers/resources_controller.rb
+++ b/src/app/controllers/resources_controller.rb
@@ -113,8 +113,8 @@ class ResourcesController < PoolController
       @success_list = @vms
       @failures = {}
       render :layout => 'confirmation'
-    rescue PermissionError => perm_error
-      handle_auth_error(perm_error.message)
+    rescue PermissionError
+      raise
     rescue PartialSuccessError => error
       @success_list = error.successes
       @failures = error.failures
diff --git a/src/app/controllers/smart_pools_controller.rb
b/src/app/controllers/smart_pools_controller.rb
index 993d285..0198d47 100644
--- a/src/app/controllers/smart_pools_controller.rb
+++ b/src/app/controllers/smart_pools_controller.rb
@@ -35,13 +35,9 @@ class SmartPoolsController < PoolController
   end
 
   def show_storage
-    begin
-      svc_show(params[:id])
-      @storage_tree = @pool.storage_tree(:filter_unavailable => false,
:include_used => true).to_json
-      render_show
-    rescue PermissionError => perm_error
-      handle_auth_error(perm_error.message)
-    end
+    svc_show(params[:id])
+    @storage_tree = @pool.storage_tree(:filter_unavailable => false,
:include_used => true).to_json
+    render_show
   end
 
   def additional_create_params
@@ -138,22 +134,9 @@ class SmartPoolsController < PoolController
   end
 
   def add_or_remove_items(item_class, item_action)
-    begin
-      alert = svc_add_remove_items(params[:id], item_class, item_action,
-                           params[:resource_ids].split(","))
-      render :json => { :success => true, :alert => alert}
-    rescue
-      render :json => { :success => false,
-        :alert => "#{item_action.to_s}
#{item_class.table_name.humanize} failed." }
-    rescue PermissionError => perm_error
-      handle_auth_error(perm_error.message)
-      # If we need to give more details as to which hosts/storage succeeded,
-      # they're in the exception
-    rescue PartialSuccessError => error
-      render :json => { :success => false, :alert => error.message }
-    rescue Exception => ex
-      render :json => { :success => false, :alert => error.message }
-    end
+    alert = svc_add_remove_items(params[:id], item_class, item_action,
+                                 params[:resource_ids].split(","))
+    render :json => { :success => true, :alert => alert}
   end
 
   def add_items
@@ -164,22 +147,8 @@ class SmartPoolsController < PoolController
       class_and_id[1] = class_and_id[1].to_a
     end
 
-    begin
-      alert = svc_add_remove_items(params[:id], nil, :add, class_and_ids)
-      render :json => { :success => true, :alert => alert}
-    rescue
-      render :json => { :success => false,
-        :alert => "#{item_action.to_s} failed." }
-    rescue PermissionError => perm_error
-      handle_auth_error(perm_error.message)
-    # If we need to give more details as to which hosts/storage succeeded,
-    # they're in the exception
-    rescue PartialSuccessError => error
-      render :json => { :success => false, :alert => error.message }
-    rescue Exception => ex
-      render :json => { :success => false, :alert => error.message }
-    end
-
+    alert = svc_add_remove_items(params[:id], nil, :add, class_and_ids)
+    render :json => { :success => true, :alert => alert}
   end
 
   protected
diff --git a/src/app/controllers/storage_volume_controller.rb
b/src/app/controllers/storage_volume_controller.rb
index d4a2561..6bdbbdc 100644
--- a/src/app/controllers/storage_volume_controller.rb
+++ b/src/app/controllers/storage_volume_controller.rb
@@ -108,7 +108,10 @@ class StorageVolumeController < ApplicationController
 
   def destroy
     unless authorized?(Privilege::MODIFY) and
@storage_volume.storage_pool.user_subdividable
-      handle_auth_error("You do not have permission to delete this storage
volume.")
+      handle_error(:message =>
+                   "You do not have permission to delete this storage
volume.",
+                   :status => :forbidden,
+                   :title => "Access Denied")
     else
       alert, success = delete_volume_internal(@storage_volume)
       respond_to do |format|
diff --git a/src/app/controllers/vm_controller.rb
b/src/app/controllers/vm_controller.rb
index ee6d79e..29c0f16 100644
--- a/src/app/controllers/vm_controller.rb
+++ b/src/app/controllers/vm_controller.rb
@@ -41,13 +41,9 @@ class VmController < ApplicationController
   end
 
   def show
-    begin
-      svc_show(params[:id])
-      @actions = @vm.get_action_hash(@user)
-      render :layout => 'selection'
-    rescue PermissionError => perm_error
-      handle_auth_error(perm_error.message)
-    end
+    svc_show(params[:id])
+    @actions = @vm.get_action_hash(@user)
+    render :layout => 'selection'
   end
 
   def add_to_smart_pool
@@ -62,14 +58,8 @@ class VmController < ApplicationController
 
   def create
     params[:vm][:forward_vnc] = params[:forward_vnc]
-    begin
-      alert = svc_create(params[:vm], params[:start_now])
-      render :json => { :object => "vm", :success => true,
:alert => alert  }
-    rescue PermissionError => perm_error
-      handle_auth_error(perm_error.message)
-    rescue Exception => error
-      json_error("vm", @vm, error)
-    end
+    alert = svc_create(params[:vm], params[:start_now])
+    render :json => { :object => "vm", :success => true,
:alert => alert  }
   end
 
   def edit
@@ -79,14 +69,9 @@ class VmController < ApplicationController
 
   def update
     params[:vm][:forward_vnc] = params[:forward_vnc]
-    begin
-      alert = svc_update(params[:id], params[:vm], params[:start_now],
-                         params[:restart_now])
-      render :json => { :object => "vm", :success => true,
:alert => alert  }
-    rescue Exception => error
-      # FIXME: need to distinguish vm vs. task save errors (but should mostly
be vm)
-      json_error("vm", @vm, error)
-    end
+    alert = svc_update(params[:id], params[:vm], params[:start_now],
+                       params[:restart_now])
+    render :json => { :object => "vm", :success => true,
:alert => alert  }
   end
 
   #FIXME: we need permissions checks. user must have permission. Also state
checks
@@ -123,14 +108,8 @@ class VmController < ApplicationController
   end
 
   def destroy
-    begin
-      alert = svc_destroy(params[:id])
-      render :json => { :object => "vm", :success => true,
:alert => alert  }
-    rescue ActionError => error
-      json_error("vm", @vm, error)
-    rescue Exception => error
-      json_error("vm", @vm, error)
-    end
+    alert = svc_destroy(params[:id])
+    render :json => { :object => "vm", :success => true,
:alert => alert  }
   end
 
   def storage_volumes_for_vm_json
@@ -144,24 +123,14 @@ class VmController < ApplicationController
   end
 
   def vm_action
-    begin
-      alert = svc_vm_action(params[:id], params[:vm_action],
-                            params[:vm_action_data])
-      render :json => { :object => "vm", :success => true,
:alert => alert  }
-    rescue ActionError => error
-      json_error("vm", @vm, error)
-    rescue Exception => error
-      json_error("vm", @vm, error)
-    end
+    alert = svc_vm_action(params[:id], params[:vm_action],
+                          params[:vm_action_data])
+    render :json => { :object => "vm", :success => true,
:alert => alert  }
   end
 
   def cancel_queued_tasks
-    begin
-      alert = svc_cancel_queued_tasks(params[:id])
-      render :json => { :object => "vm", :success => true,
:alert => alert  }
-    rescue Exception => error
-      json_error("vm", @vm, error)
-    end
+    alert = svc_cancel_queued_tasks(params[:id])
+    render :json => { :object => "vm", :success => true,
:alert => alert  }
   end
 
   def migrate
-- 
1.6.0.6
David Lutterkort
2009-May-05  19:03 UTC
[Ovirt-devel] [PATCH server 2/2] * app/controllers/host_controller.rb: factor biz logic out
The business logic is now in HostService.
The main functional difference is that svc_list filters the host list by
permission.
---
 src/app/controllers/host_controller.rb |  125 +++++++++++++-------------------
 src/app/services/host_service.rb       |  119 ++++++++++++++++++++++++++++++
 2 files changed, 168 insertions(+), 76 deletions(-)
 create mode 100644 src/app/services/host_service.rb
diff --git a/src/app/controllers/host_controller.rb
b/src/app/controllers/host_controller.rb
index 994b5e2..9b683fa 100644
--- a/src/app/controllers/host_controller.rb
+++ b/src/app/controllers/host_controller.rb
@@ -19,8 +19,13 @@
 
 class HostController < ApplicationController
 
-  EQ_ATTRIBUTES = [ :state, :arch, :hostname, :uuid,
-                    :hardware_pool_id ]
+  include HostService
+
+  # GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
+  verify :method => [:post, :put], :only => [ :create, :update ],
+         :redirect_to => { :action => :list }
+  verify :method => [:post, :delete], :only => :destroy,
+         :redirect_to => { :action => :list }
 
   def index
     list
@@ -30,42 +35,21 @@ class HostController < ApplicationController
     end
   end
 
-  before_filter :pre_action, :only => [:host_action, :enable, :disable,
:clear_vms, :edit_network]
-  before_filter :pre_addhost, :only => [:addhost]
-
-  # GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
-  verify :method => [:post, :put], :only => [ :create, :update ],
-         :redirect_to => { :action => :list }
-  verify :method => [:post, :delete], :only => :destroy,
-         :redirect_to => { :action => :list }
-
-   def list
-     conditions = []
-     EQ_ATTRIBUTES.each do |attr|
-       if params[attr]
-         conditions << "#{attr} = :#{attr}"
-       end
-     end
-     @hosts = Host.find(:all,
-                        :conditions => [conditions.join(" and "),
params],
-                        :order => "id")
-   end
-
+  def list
+    svc_list(params)
+  end
 
   def show
-    if authorize_view
-      respond_to do |format|
-        format.html { render :layout => 'selection' }
-        format.xml { render :xml => @host.to_xml(:include => [ :cpus ] )
}
-      end
+    svc_show(params[:id])
+    respond_to do |format|
+      format.html { render :layout => 'selection' }
+      format.xml { render :xml => @host.to_xml(:include => [ :cpus ] ) }
     end
   end
 
   def quick_summary
-    pre_show
-    if authorize_view
-      render :layout => false
-    end
+    svc_show(id)
+    render :layout => false
   end
 
   # retrieves data used by snapshot graphs
@@ -73,15 +57,17 @@ class HostController < ApplicationController
   end
 
   def addhost
-    @hardware_pool = Pool.find(params[:hardware_pool_id])
-    render :layout => 'popup'
-  end
+    # FIXME: This probably should go into PoolService.svc_modify,
+    # so that we have permission checks in only one place
 
-  def pre_addhost
+    # Old pre_addhost
     @pool = Pool.find(params[:hardware_pool_id])
     @parent = @pool.parent
     set_perms(@pool)
     authorize_admin
+    # Old addhost
+    @hardware_pool = Pool.find(params[:hardware_pool_id])
+    render :layout => 'popup'
   end
 
   def add_to_smart_pool
@@ -89,6 +75,11 @@ class HostController < ApplicationController
     render :layout => 'popup'
   end
 
+  # FIXME: We implement the standard controller actions, but catch
+  # them in filters and kick out friendly warnings that you can't
+  # perform them on hosts. Tat's overkill - the only way for a user
+  # to get to these actions is with URL surgery or from a bug in the
+  # application, both of which deserve a noisy error
   def new
   end
 
@@ -116,52 +107,40 @@ class HostController < ApplicationController
   end
 
   def disable
-    set_disabled(1)
+    svc_enable(params[:id], "disabled")
+    render :json => {
+      :object => :host,
+      :alert => "Host was successfully disabled",
+      :success => true
+    }
   end
+
   def enable
-    set_disabled(0)
-  end
-
-  def set_disabled(value)
-    operation = value == 1 ? "disabled" : "enabled"
-    begin
-      @host.is_disabled = value
-      @host.save!
-      @json_hash[:alert]="Host was successfully #{operation}"
-      @json_hash[:success]=true
-    rescue
-      @json_hash[:alert]="Error setting host to #{operation}"
-      @json_hash[:success]=false
-    end
-    render :json => @json_hash
+    svc_enable(params[:id], "enabled")
+    render :json => {
+      :object => :host,
+      :alert => "Host was successfully enabled",
+      :success => true
+    }
   end
 
   def clear_vms
-    begin
-      Host.transaction do
-        task = HostTask.new({ :user        => get_login_user,
-                              :task_target => @host,
-                              :action      => HostTask::ACTION_CLEAR_VMS,
-                              :state       => Task::STATE_QUEUED})
-        task.save!
-        @host.is_disabled = true
-        @host.save!
-      end
-      @json_hash[:alert]="Clear VMs action was successfully queued."
-      @json_hash[:success]=true
-    rescue
-      @json_hash[:alert]="Error in queueing Clear VMs action."
-      @json_hash[:success]=false
-    end
-    render :json => @json_hash
+    svc_clear_vms(params[:id])
+    render :json => {
+      :object => :host,
+      :alert => "Clear VMs action was successfully queued.",
+      :success => true
+    }
   end
 
   def edit_network
+    svc_modify(params[:id])
     render :layout => 'popup'
   end
 
   def bondings_json
-    bondings = Host.find(params[:id]).bondings
+    svc_show(params[:id])
+    bondings = @host.bondings
     render :json => bondings.collect{ |x| {:id => x.id, :name =>
x.name} }
   end
 
@@ -180,12 +159,6 @@ class HostController < ApplicationController
     flash[:notice] = 'Hosts may not be edited via the web UI'
     redirect_to :action=> 'show', :id => @host
   end
-  def pre_action
-    @host = Host.find(params[:id])
-    set_perms(@host.hardware_pool)
-    @json_hash = { :object => :host }
-    authorize_admin
-  end
   def pre_show
     @host = Host.find(params[:id])
     set_perms(@host.hardware_pool)
diff --git a/src/app/services/host_service.rb b/src/app/services/host_service.rb
new file mode 100644
index 0000000..074d830
--- /dev/null
+++ b/src/app/services/host_service.rb
@@ -0,0 +1,119 @@
+#--
+# Copyright (C) 2009 Red Hat, Inc.
+# Written by David Lutterkort <lutter at redhat.com>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; version 2 of the License.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+# MA  02110-1301, USA.  A copy of the GNU General Public License is
+# also available at http://www.gnu.org/copyleft/gpl.html.
+#++
+# Business logic around hosts aka nodes
+module HostService
+
+  include ApplicationService
+
+  # Host attributes on which we filter with '='
+  EQ_ATTRIBUTES = [ :state, :arch, :hostname, :uuid,
+                    :hardware_pool_id ]
+
+  # List hosts matching criteria described by +params+; only the entries
+  # for keys in +EQ_ATTRIBUTES+ are used
+  #
+  # === Instance variables
+  # [<tt>@hosts</tt>] stores list of hosts matching criteria
+  # === Required permissions
+  # [<tt>Privilege::VIEW</tt>] no exception raised,
<tt>@hosts</tt>
+  #                            is filtered by privilege
+  def svc_list(params)
+    conditions = []
+    EQ_ATTRIBUTES.each do |attr|
+      if params[attr]
+        conditions << "hosts.#{attr} = :#{attr}"
+      end
+    end
+    # Add permission check
+    params = params.dup
+    params[:user] = get_login_user
+    params[:priv] = Privilege::VIEW
+    conditions << "privileges.name=:priv"
+    conditions << "permissions.uid=:user"
+    incl = [{ :hardware_pool => { :permissions => { :role =>
:privileges}}}]
+    @hosts = Host.find(:all,
+                       :include => incl,
+                       :conditions => [conditions.join(" and "),
params],
+                       :order => "hosts.id")
+  end
+
+  # Load the Host with +id+ for viewing
+  #
+  # === Instance variables
+  # [<tt>@host</tt>] stores the Host with +id+
+  # === Required permissions
+  # [<tt>Privilege::VIEW</tt>] on host's HardwarePool
+  def svc_show(id)
+    lookup(id, Privilege::VIEW)
+  end
+
+  # Load the Host with +id+ for editing
+  #
+  # === Instance variables
+  # [<tt>@host</tt>] stores the Host with +id+
+  # === Required permissions
+  # [<tt>Privilege::MODIFY</tt>] on host's HardwarePool
+  def svc_modify(id)
+    lookup(id, Privilege::MODIFY)
+  end
+
+  # Set the disabled state of the Host with +id+ to
<tt>:enabled</tt>
+  # or <tt>:disabled</tt>
+  #
+  # === Instance variables
+  # [<tt>@host</tt>] stores the Host with +id+
+  # === Required permissions
+  # [<tt>Privilege::MODIFY</tt>] on host's HardwarePool
+  def svc_enable(id, state)
+    ind = ["enabled", "disabled"].index(state.to_s)
+    if ind.nil?
+      raise ArgumentError, "STATE must be 'enabled' or
'disabled'"
+    end
+    svc_modify(id)
+    @host.is_disabled = ind
+    @host.save!
+  end
+
+  # Queue task to migrate all VM's off the Host with +id+, and mark the
+  # host as disabled
+  #
+  # === Instance variables
+  # [<tt>@host</tt>] stores the Host with +id+
+  # === Required permissions
+  # [<tt>Privilege::MODIFY</tt>] on host's HardwarePool
+  def svc_clear_vms(id)
+    svc_modify(id)
+    Host.transaction do
+      task = HostTask.new({ :user        => get_login_user,
+                            :task_target => @host,
+                            :action      => HostTask::ACTION_CLEAR_VMS,
+                            :state       => Task::STATE_QUEUED})
+      task.save!
+      @host.is_disabled = true
+      @host.save!
+    end
+  end
+
+  private
+  def lookup(id, priv)
+    @host = Host.find(id)
+    authorized!(priv, @host.hardware_pool)
+  end
+end
-- 
1.6.0.6