thr3ads.net - Ovirt devel - [Ovirt-devel] [PATCH server] First attempt at refactoring the vm_controller using the service layer. [Apr 2009]

If this information is useful, please help other people find it:
Share via:

Scott Seago

2009-Apr-20 15:13 UTC

[Ovirt-devel] [PATCH server] First attempt at refactoring the vm_controller using the service layer.

This code is based on David Lutterkort's proposal outlined in
https://www.redhat.com/archives/ovirt-devel/2009-April/msg00086.html

I've included service-layer methods for show, create, update, destroy,
vm_action, and cancel_queued_tasks. These are the concrete actions that make
sense as part of the API. Most of the remaining vm_controller methods are
pure-WUI actions -- for things such as generating the web form for a new VM (the
subsequent 'create' action _is_ part of the API).

There's a certain amount of temporary code that we need in place while only
some of the controllers have been converted, as we still need permissions
pre-filters for non-converted create, etc. methods -- but for the converted
controllers we don't want to perform actions in before_filters that will be
done as part of the services layer. Any temporary methods, etc. that will go
away when the last controller is converted have been marked clearly with FIXME
comments.

Signed-off-by: Scott Seago <sseago at redhat.com>
---
 src/app/controllers/application.rb               |  113 +++++++----
 src/app/controllers/hardware_controller.rb       |    3 +-
 src/app/controllers/host_controller.rb           |    4 +-
 src/app/controllers/nic_controller.rb            |    2 +-
 src/app/controllers/permission_controller.rb     |   12 +-
 src/app/controllers/pool_controller.rb           |    2 +-
 src/app/controllers/quota_controller.rb          |    4 +-
 src/app/controllers/storage_controller.rb        |    5 +-
 src/app/controllers/storage_volume_controller.rb |    6 +-
 src/app/controllers/task_controller.rb           |    9 +-
 src/app/controllers/vm_controller.rb             |  230 +++++-----------------
 src/app/models/vm.rb                             |    1 +
 src/app/services/application_service.rb          |   56 ++++++
 src/app/services/vm_service.rb                   |  189 ++++++++++++++++++
 src/app/views/vm/_form.rhtml                     |    3 +-
 15 files changed, 394 insertions(+), 245 deletions(-)
 create mode 100644 src/app/services/application_service.rb
 create mode 100644 src/app/services/vm_service.rb

diff --git a/src/app/controllers/application.rb
b/src/app/controllers/application.rb
index fa88773..34283ec 100644
--- a/src/app/controllers/application.rb
+++ b/src/app/controllers/application.rb
@@ -20,19 +20,35 @@
 # Filters added to this controller apply to all controllers in the application.
 # Likewise, all the methods added will be available for all controllers.
 
+# FIXME: once all controller classes include this, remove here
+require 'services/vm_service'
+
 class ApplicationController < ActionController::Base
+  # FIXME: once all controller classes include this, remove here
+  include ApplicationService
+
   # Pick a unique cookie name to distinguish our session data from others'
   session :session_key => '_ovirt_session_id'
   init_gettext "ovirt"
   layout :choose_layout
 
+  # FIXME: once service layer is complete, the following before_filters will be
+  # removed as their functionality has been moved to the service layer
+  # pre_create
+  # pre_edit will remain only for :edit, not :update or :destroy
+  # pre_show
+  # authorize_admin will remain only for :new, :edit
   before_filter :pre_new, :only => [:new]
   before_filter :pre_create, :only => [:create]
-  before_filter :pre_edit, :only => [:edit, :update, :destroy]
+  before_filter :pre_edit, :only => [:edit]
+  # the following is to facilitate transition to service layer
+  before_filter :tmp_pre_update, :only => [:update, :destroy]
   before_filter :pre_show, :only => [:show]
-  before_filter :authorize_admin, :only => [:new, :create, :edit, :update,
:destroy]
+  before_filter :authorize_admin, :only => [:new, :edit]
+  before_filter :tmp_authorize_admin, :only => [:create, :update, :destroy]
   before_filter :is_logged_in, :get_help_section
 
+
   def choose_layout
     if(params[:component_layout])
       return (ENV["RAILS_ENV"] !=
"production")?'components/' <<
params[:component_layout]:'redux'
@@ -61,62 +77,66 @@ class ApplicationController < ActionController::Base
     (ENV["RAILS_ENV"] == "production") ? session[:user] :
"ovirtadmin"
   end
 
-  def set_perms(hwpool)
-    @user = get_login_user
-    @can_view = hwpool.can_view(@user)
-    @can_control_vms = hwpool.can_control_vms(@user)
-    @can_modify = hwpool.can_modify(@user)
-    @can_view_perms = hwpool.can_view_perms(@user)
-    @can_set_perms = hwpool.can_set_perms(@user)
-  end
-
   protected
   # permissions checking
 
   def pre_new
   end
-  def pre_create
-  end
   def pre_edit
   end
+
+  # FIXME: remove these when service layer transition is complete
+  def tmp_pre_update
+    pre_edit
+  end
+  def tmp_authorize_admin
+    authorize_admin
+  end
+  def pre_create
+  end
   def pre_show
   end
 
+  def authorize_view(msg=nil)
+    authorize_action(Privilege::VIEW,msg)
+  end
   def authorize_user(msg=nil)
-    authorize_action(false,msg)
+    authorize_action(Privilege::VM_CONTROL,msg)
   end
   def authorize_admin(msg=nil)
-    authorize_action(true,msg)
-  end
-  def authorize_action(is_modify_action, msg=nil)
-    msg ||= 'You do not have permission to create or modify this item '
-    if @perm_obj
-      set_perms(@perm_obj)
-      unless (is_modify_action ? @can_modify : @can_control_vms)
-        respond_to do |format|
-          format.html do
-            @title = "Access denied"
-            @errmsg = msg
-            @ajax = params[:ajax]
-            @nolayout = params[:nolayout]
-            if @ajax
-              render :template => 'layouts/popup-error', :layout
=> 'tabs-and-content'
-            elsif @nolayout
-              render :template => 'layouts/popup-error', :layout
=> 'help-and-content'
-            else
-              render :template => 'layouts/popup-error', :layout
=> 'popup'
-            end
-          end
-          format.json do
-            @json_hash ||= {}
-            @json_hash[:success] = false
-            @json_hash[:alert] = msg
-            render :json => @json_hash
-          end
-          format.xml { head :forbidden }
+    authorize_action(Privilege::MODIFY,msg)
+  end
+  def authorize_action(privilege, msg=nil)
+    msg ||= 'You have insufficient privileges to perform action.'
+    unless authorized?(privilege)
+      handle_auth_error(msg)
+      false
+    else
+      true
+    end
+  end
+  def handle_auth_error(msg)
+    respond_to do |format|
+      format.html do
+        @title = "Access denied"
+        @errmsg = msg
+        @ajax = params[:ajax]
+        @nolayout = params[:nolayout]
+        if @ajax
+          render :template => 'layouts/popup-error', :layout =>
'tabs-and-content'
+        elsif @nolayout
+          render :template => 'layouts/popup-error', :layout =>
'help-and-content'
+        else
+          render :template => 'layouts/popup-error', :layout =>
'popup'
         end
-        false
       end
+      format.json do
+        @json_hash ||= {}
+        @json_hash[:success] = false
+        @json_hash[:alert] = msg
+        render :json => @json_hash
+      end
+      format.xml { head :forbidden }
     end
   end
 
@@ -155,6 +175,11 @@ 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
+  end
 
 end
diff --git a/src/app/controllers/hardware_controller.rb
b/src/app/controllers/hardware_controller.rb
index 726e5bd..5615f6e 100644
--- a/src/app/controllers/hardware_controller.rb
+++ b/src/app/controllers/hardware_controller.rb
@@ -70,7 +70,8 @@ class HardwareController < PoolController
     id = params[:id]
     if id
       @pool = Pool.find(id)
-      set_perms(@pool)
+      @perm_obj = @pool
+      set_perms
       unless @pool.has_privilege(@user, privilege)
         flash[:notice] = 'You do not have permission to access this
hardware pool: redirecting to top level'
         redirect_to :controller => "dashboard"
diff --git a/src/app/controllers/host_controller.rb
b/src/app/controllers/host_controller.rb
index f0b8c2b..ad3bde4 100644
--- a/src/app/controllers/host_controller.rb
+++ b/src/app/controllers/host_controller.rb
@@ -53,7 +53,7 @@ class HostController < ApplicationController
 
 
   def show
-    set_perms(@perm_obj)
+    set_perms
     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
@@ -68,7 +68,7 @@ class HostController < ApplicationController
 
   def quick_summary
     pre_show
-    set_perms(@perm_obj)
+    set_perms
     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
diff --git a/src/app/controllers/nic_controller.rb
b/src/app/controllers/nic_controller.rb
index 85d4315..6319dbe 100644
--- a/src/app/controllers/nic_controller.rb
+++ b/src/app/controllers/nic_controller.rb
@@ -23,7 +23,7 @@ class NicController < ApplicationController
          :redirect_to => { :controller => 'dashboard' }
 
   def show
-    set_perms(@perm_obj)
+    set_perms
     unless @can_view
       flash[:notice] = 'You do not have permission to view this NIC:
redirecting to top level'
       redirect_to :controller => 'pool', :action =>
'list'
diff --git a/src/app/controllers/permission_controller.rb
b/src/app/controllers/permission_controller.rb
index 3bfbffa..9914fb7 100644
--- a/src/app/controllers/permission_controller.rb
+++ b/src/app/controllers/permission_controller.rb
@@ -28,7 +28,8 @@ class PermissionController < ApplicationController
 
   def show
     @permission = Permission.find(params[:id])
-    set_perms(@permission.pool)
+    @perm_obj = @permission.pool
+    set_perms
     # admin permission required to view permissions
     unless @can_view_perms
       flash[:notice] = 'You do not have permission to view this permission
record'
@@ -43,7 +44,8 @@ class PermissionController < ApplicationController
     filter = @pool.permissions.collect{ |permission| permission.uid }
     @users = Account.names(filter)
     @roles = Role.find(:all).collect{ |role| [role.name, role.id] }
-    set_perms(@permission.pool)
+    @perm_obj = @permission.pool
+    set_perms
     # admin permission required to view permissions
     unless @can_set_perms
       flash[:notice] = 'You do not have permission to create this
permission record'
@@ -55,7 +57,8 @@ class PermissionController < ApplicationController
 
   def create
     @permission = Permission.new(params[:permission])
-    set_perms(@permission.pool)
+    @perm_obj = @permission.pool
+    set_perms
     unless @can_set_perms
       # FIXME: need to handle proper error messages w/ ajax
       flash[:notice] = 'You do not have permission to create this
permission record'
@@ -117,7 +120,8 @@ class PermissionController < ApplicationController
 
   def destroy
     @permission = Permission.find(params[:id])
-    set_perms(@permission.pool)
+    @perm_obj = @permission.pool
+    set_perms
     unless @can_set_perms
       flash[:notice] = 'You do not have permission to delete this
permission record'
       redirect_to_parent
diff --git a/src/app/controllers/pool_controller.rb
b/src/app/controllers/pool_controller.rb
index 22cf1a4..018f2cd 100644
--- a/src/app/controllers/pool_controller.rb
+++ b/src/app/controllers/pool_controller.rb
@@ -123,7 +123,7 @@ class PoolController < ApplicationController
   def pre_show
     @perm_obj = @pool
     @current_pool_id=@pool.id
-    set_perms(@perm_obj)
+    set_perms
     unless @can_view
       flash[:notice] = 'You do not have permission to view this pool:
redirecting to top level'
       respond_to do |format|
diff --git a/src/app/controllers/quota_controller.rb
b/src/app/controllers/quota_controller.rb
index 17fdc20..220792b 100644
--- a/src/app/controllers/quota_controller.rb
+++ b/src/app/controllers/quota_controller.rb
@@ -28,7 +28,7 @@ class QuotaController < ApplicationController
 
   def show
     @quota = Quota.find(params[:id])
-    set_perms(@quota.pool)
+    set_perms
 
     unless @can_view
       flash[:notice] = 'You do not have permission to view this quota:
redirecting to top level'
@@ -91,7 +91,7 @@ class QuotaController < ApplicationController
   end
   def pre_show
     @quota = Quota.find(params[:id])
-    @perm_obj = @quota
+    @perm_obj = @quota.pool
   end
   def pre_edit
     @quota = Quota.find(params[:id])
diff --git a/src/app/controllers/storage_controller.rb
b/src/app/controllers/storage_controller.rb
index 98ba0f6..2232388 100644
--- a/src/app/controllers/storage_controller.rb
+++ b/src/app/controllers/storage_controller.rb
@@ -46,7 +46,7 @@ class StorageController < ApplicationController
     @attach_to_pool=params[:attach_to_pool]
     if @attach_to_pool
       pool = HardwarePool.find(@attach_to_pool)
-      set_perms(pool)
+      @perm_obj = pool
       unless @can_view
         flash[:notice] = 'You do not have permission to view this storage
pool list: redirecting to top level'
         redirect_to :controller => 'dashboard'
@@ -70,7 +70,8 @@ class StorageController < ApplicationController
 
   def show
     @storage_pool = StoragePool.find(params[:id])
-    set_perms(@storage_pool.hardware_pool)
+    @perm_obj = @storage_pool.hardware_pool
+    set_perms
     unless @can_view
       flash[:notice] = 'You do not have permission to view this storage
pool: redirecting to top level'
       respond_to do |format|
diff --git a/src/app/controllers/storage_volume_controller.rb
b/src/app/controllers/storage_volume_controller.rb
index 9b0b9e0..f78071f 100644
--- a/src/app/controllers/storage_volume_controller.rb
+++ b/src/app/controllers/storage_volume_controller.rb
@@ -92,7 +92,8 @@ class StorageVolumeController < ApplicationController
 
   def show
     @storage_volume = StorageVolume.find(params[:id])
-    set_perms(@storage_volume.storage_pool.hardware_pool)
+    @perm_obj = @storage_volume.storage_pool.hardware_pool
+    set_perms
     @storage_pool = @storage_volume.storage_pool
     unless @can_view
       flash[:notice] = 'You do not have permission to view this storage
volume: redirecting to top level'
@@ -117,7 +118,8 @@ class StorageVolumeController < ApplicationController
 
   def destroy
     @storage_volume = StorageVolume.find(params[:id])
-    set_perms(@storage_volume.storage_pool.hardware_pool)
+    @perm_obj = @storage_volume.storage_pool.hardware_pool
+    set_perms
     unless @can_modify and @storage_volume.storage_pool.user_subdividable
       respond_to do |format|
         format.json { render :json => { :object =>
"storage_volume",
diff --git a/src/app/controllers/task_controller.rb
b/src/app/controllers/task_controller.rb
index 647d69c..618856c 100644
--- a/src/app/controllers/task_controller.rb
+++ b/src/app/controllers/task_controller.rb
@@ -21,14 +21,15 @@ class TaskController < ApplicationController
   def show
     @task = Task.find(params[:id])
     if @task[:type] == VmTask.name
-      set_perms(@task.vm.vm_resource_pool)
+      @perm_obj = @task.vm.vm_resource_pool
     elsif @task[:type] == StorageTask.name 
-      set_perms(@task.storage_pool.hardware_pool)
+      @perm_obj = @task.storage_pool.hardware_pool
     elsif @task[:type] == StorageVolumeTask.name
-      set_perms(@task.storage_volume.storage_pool.hardware_pool)
+      @perm_obj = @task.storage_volume.storage_pool.hardware_pool
     elsif @task[:type] == HostTask.name 
-      set_perms(@task.host.hardware_pool)
+      @perm_obj = @task.host.hardware_pool
     end
+    set_perms
     unless @can_view
       flash[:notice] = 'You do not have permission to view this task:
redirecting to top level'
       redirect_to :controller => 'dashboard'
diff --git a/src/app/controllers/vm_controller.rb
b/src/app/controllers/vm_controller.rb
index a81f6b5..fc55d92 100644
--- a/src/app/controllers/vm_controller.rb
+++ b/src/app/controllers/vm_controller.rb
@@ -17,13 +17,16 @@
 # MA  02110-1301, USA.  A copy of the GNU General Public License is
 # also available at http://www.gnu.org/copyleft/gpl.html.
 require 'socket'
+require 'services/vm_service'
 
 class VmController < ApplicationController
+  include VmService
+
   # GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
   verify :method => :post, :only => [ :destroy, :create, :update ],
          :redirect_to => { :controller => 'dashboard' }
 
-  before_filter :pre_vm_action, :only => [:vm_action, :cancel_queued_tasks,
:console]
+  before_filter :pre_console, :only => [:console]
 
   def index
     @vms = Vm.find(:all,
@@ -38,13 +41,13 @@ class VmController < ApplicationController
   end
 
   def show
-    set_perms(@perm_obj)
-    @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'
+    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
-    render :layout => 'selection'
   end
 
   def add_to_smart_pool
@@ -58,38 +61,15 @@ class VmController < ApplicationController
   end
 
   def create
+    params[:vm][:forward_vnc] = params[:forward_vnc]
     begin
-      Vm.transaction do
-        @vm.save!
-        _setup_vm_provision(params)
-        @task = VmTask.new({ :user        => @user,
-                             :task_target => @vm,
-                             :action      => VmTask::ACTION_CREATE_VM,
-                             :state       => Task::STATE_QUEUED})
-        @task.save!
-      end
-      start_now = params[:start_now]
-      if (start_now)
-        if @vm.get_action_list.include?(VmTask::ACTION_START_VM)
-          @task = VmTask.new({ :user        => @user,
-                               :task_target => @vm,
-                               :action      => VmTask::ACTION_START_VM,
-                               :state       => Task::STATE_QUEUED})
-          @task.save!
-          alert = "VM was successfully created. VM Start action
queued."
-        else
-          alert = "VM was successfully created. Resources are not
available to start VM now."
-        end
-      else
-        alert = "VM was successfully created."
-      end
+      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
-      # FIXME: need to distinguish vm vs. task save errors (but should mostly
be vm)
-      render :json => { :object => "vm", :success => false,
-                        :errors => @vm.errors.localize_error_messages.to_a }
+      json_error("vm", @vm, error)
     end
-
   end
 
   def edit
@@ -98,58 +78,20 @@ class VmController < ApplicationController
   end
 
   def update
+    params[:vm][:forward_vnc] = params[:forward_vnc]
     begin
-      #needs restart if certain fields are changed (since those will only take
effect the next startup)
-      needs_restart = false
-      unless @vm.get_pending_state == Vm::STATE_STOPPED
-        Vm::NEEDS_RESTART_FIELDS.each do |field|
-          unless @vm[field].to_s == params[:vm][field]
-            needs_restart = true
-            break
-          end
-        end
-        current_storage_ids = @vm.storage_volume_ids.sort
-        new_storage_ids = params[:vm][:storage_volume_ids]
-        new_storage_ids = [] unless new_storage_ids
-        new_storage_ids = new_storage_ids.sort.collect {|x| x.to_i }
-        needs_restart = true unless current_storage_ids == new_storage_ids
-      end
-
-      params[:vm][:forward_vnc] = params[:forward_vnc]
-      params[:vm][:needs_restart] = 1 if needs_restart
-      @vm.update_attributes!(params[:vm])
-      _setup_vm_provision(params)
-
-      if (params[:start_now] and
@vm.get_action_list.include?(VmTask::ACTION_START_VM) )
-        @task = VmTask.new({ :user        => @user,
-                             :task_target => @vm,
-                             :action      => VmTask::ACTION_START_VM,
-                             :state       => Task::STATE_QUEUED})
-        @task.save!
-      elsif ( params[:restart_now] and
@vm.get_action_list.include?(VmTask::ACTION_SHUTDOWN_VM) )
-        @task = VmTask.new({ :user        => @user,
-                             :task_target => @vm,
-                             :action      => VmTask::ACTION_SHUTDOWN_VM,
-                             :state       => Task::STATE_QUEUED})
-        @task.save!
-        @task = VmTask.new({ :user    => @user,
-                             :task_target => @vm,
-                             :action  => VmTask::ACTION_START_VM,
-                             :state   => Task::STATE_QUEUED})
-        @task.save!
-      end
-
-
-      render :json => { :object => "vm", :success => true,
-                        :alert => 'Vm was successfully updated.'  }
-    rescue
+      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)
-      render :json => { :object => "vm", :success => false,
-                        :errors => @vm.errors.localize_error_messages.to_a }
+      json_error("vm", @vm, error)
     end
   end
 
   #FIXME: we need permissions checks. user must have permission. Also state
checks
+  # this should probably be implemented as an action on the containing VM pool
once
+  # that service module is defined
   def delete
     vm_ids_str = params[:vm_ids]
     vm_ids = vm_ids_str.split(",").collect {|x| x.to_i}
@@ -181,15 +123,13 @@ class VmController < ApplicationController
   end
 
   def destroy
-    vm_resource_pool = @vm.vm_resource_pool_id
-    if (@vm.is_destroyable?)
-      destroy_cobbler_system(@vm)
-      @vm.destroy
-      render :json => { :object => "vm", :success => true,
-        :alert => "Virtual Machine was successfully deleted." }
-    else
-      render :json => { :object => "vm", :success => false,
-        :alert => "Vm must be stopped to delete it." }
+    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
   end
 
@@ -204,27 +144,23 @@ class VmController < ApplicationController
   end
 
   def vm_action
-    vm_action = params[:vm_action]
-    data = params[:vm_action_data]
     begin
-      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." }
-      end
-    rescue
-      render :json => { :object => "vm", :success => false,
:alert => "Error in queueing #{vm_action}." }
+      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
   end
 
   def cancel_queued_tasks
     begin
-      Task.transaction do
-        @vm.tasks.queued.each { |task| task.cancel}
-      end
-      render :json => { :object => "vm", :success => true,
:alert => "queued tasks were canceled." }
-    rescue
-      render :json => { :object => "vm", :success => true,
:alert => "queued tasks cancel failed." }
+      alert = svc_cancel_queued_tasks(params[:id])
+      render :json => { :object => "vm", :success => true,
:alert => alert  }
+    rescue Exception => error
+      json_error("vm", @vm, error)
     end
   end
 
@@ -268,53 +204,10 @@ class VmController < ApplicationController
     end
   end
 
-  # FIXME: move this to an edit_vm task in taskomatic
-  def _setup_vm_provision(params)
-    # spaces are invalid in the cobbler name
-    name = params[:vm][:uuid]
-    mac = params[:vm][:vnic_mac_addr]
-    provision = params[:vm][:provisioning_and_boot_settings]
-    # determine what type of provisioning was selected for the VM
-    provisioning_type = :pxe_or_hd_type
-    provisioning_type = :image_type  if provision.index
"#{Vm::IMAGE_PREFIX}@#{Vm::COBBLER_PREFIX}"
-    provisioning_type = :system_type if provision.index
"#{Vm::PROFILE_PREFIX}@#{Vm::COBBLER_PREFIX}"
-
-    unless provisioning_type == :pxe_or_hd_type
-      cobbler_name = provision.slice(/(.*):(.*)/, 2)
-      system = Cobbler::System.find_one(name)
-      unless system
-        nic = Cobbler::NetworkInterface.new({'mac_address' => mac})
-
-        case provisioning_type
-        when :image_type:
-            system = Cobbler::System.new("name" => name,
"image"    => cobbler_name)
-        when :system_type:
-            system = Cobbler::System.new("name" => name,
"profile" => cobbler_name)
-        end
-
-        system.interfaces = [nic]
-        system.save
-      end
-    end
-  end
-
   def pre_new
-    # if no vm_resource_pool is passed in, find (or auto-create) it based on
hardware_pool_id
     unless params[:vm_resource_pool_id]
-      unless params[:hardware_pool_id]
-        flash[:notice] = "VM Resource Pool or Hardware Pool is
required."
-        redirect_to :controller => 'dashboard'
-      end
-      @hardware_pool = HardwarePool.find(params[:hardware_pool_id])
-      @user = get_login_user
-      vm_resource_pool = @hardware_pool.sub_vm_resource_pools.select {|pool|
pool.name == @user}.first
-      if vm_resource_pool
-        params[:vm_resource_pool_id] = vm_resource_pool.id
-      else
-        @vm_resource_pool = VmResourcePool.new({:name => vm_resource_pool})
-        @vm_resource_pool.tmp_parent = @hardware_pool
-        @vm_resource_pool_name = @user
-      end
+      flash[:notice] = "VM Resource Pool is required."
+      redirect_to :controller => 'dashboard'
     end
 
     # random MAC
@@ -336,26 +229,6 @@ class VmController < ApplicationController
     @networks = Network.find(:all).collect{ |net| [net.name, net.id] }
     _setup_provisioning_options
   end
-  def pre_create
-    params[:vm][:state] = Vm::STATE_PENDING
-    vm_resource_pool_name = params[:vm_resource_pool_name]
-    hardware_pool_id = params[:hardware_pool_id]
-    if vm_resource_pool_name and hardware_pool_id
-      hardware_pool = HardwarePool.find(hardware_pool_id)
-      vm_resource_pool = VmResourcePool.new({:name =>
vm_resource_pool_name})
-      vm_resource_pool.create_with_parent(hardware_pool)
-      params[:vm][:vm_resource_pool_id] = vm_resource_pool.id
-    end
-    params[:vm][:forward_vnc] = params[:forward_vnc]
-    @vm = Vm.new(params[:vm])
-    @perm_obj = @vm.vm_resource_pool
-    @current_pool_id=@perm_obj.id
-  end
-  def pre_show
-    @vm = Vm.find(params[:id])
-    @perm_obj = @vm.vm_resource_pool
-    @current_pool_id=@perm_obj.id
-  end
   def pre_edit
     @vm = Vm.find(params[:id])
     @perm_obj = @vm.vm_resource_pool
@@ -363,18 +236,15 @@ class VmController < ApplicationController
     @networks = Network.find(:all).collect{ |net| [net.name, net.id] }
     _setup_provisioning_options
   end
-  def pre_vm_action
+  def pre_console
     pre_edit
     authorize_user
   end
-
-  private
-
-  def destroy_cobbler_system(vm)
-    # Destroy the Cobbler system first if it's defined
-    if vm.uses_cobbler?
-      system = Cobbler::System.find_one(vm.cobbler_system_name)
-      system.remove if system
-    end
+  # FIXME: remove these when service transition is complete. these are here
+  # to keep from running permissions checks and other setup steps twice
+  def tmp_pre_update
+  end
+  def tmp_authorize_admin
   end
+
 end
diff --git a/src/app/models/vm.rb b/src/app/models/vm.rb
index c62595a..4c7925c 100644
--- a/src/app/models/vm.rb
+++ b/src/app/models/vm.rb
@@ -220,6 +220,7 @@ class Vm < ActiveRecord::Base
       self[:boot_device]= BOOT_DEV_HD
       self[:provisioning]= nil
     else
+      # FIXME: Should this case trigger an error instead?
       self[:boot_device]= BOOT_DEV_NETWORK
       self[:provisioning]= settings
     end
diff --git a/src/app/services/application_service.rb
b/src/app/services/application_service.rb
new file mode 100644
index 0000000..04f5196
--- /dev/null
+++ b/src/app/services/application_service.rb
@@ -0,0 +1,56 @@
+#
+# Copyright (C) 2009 Red Hat, Inc.
+# Written by Scott Seago <sseago at redhat.com>,
+#            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.
+#
+# Common infrastructure for business logic for WUI and QMF
+#
+# We call objects in the mid-level API 'Service' for lack of a better
name.
+# The Service layer is all in modules that are included by the classes that
+# use them in the WUI and the QMF controllers. They set instance variables,
+# which automatically become instance variables on the controllers that use
+# the Service modules
+
+module ApplicationService
+  class PermissionError < RuntimeError; end
+  class ActionError < RuntimeError; end
+
+  # Including class must provide a GET_LOGIN_USER
+
+  def set_perms
+    @user = get_login_user
+    @can_view = @perm_obj.can_view(@user)
+    @can_control_vms = @perm_obj.can_control_vms(@user)
+    @can_modify = @perm_obj.can_modify(@user)
+    @can_view_perms = @perm_obj.can_view_perms(@user)
+    @can_set_perms = @perm_obj.can_set_perms(@user)
+  end
+
+  def authorized?(privilege)
+    return false unless @perm_obj
+    set_perms
+    return @perm_obj.has_privilege(get_login_user, privilege)
+  end
+  def authorized!(privilege)
+    unless authorized?(privilege)
+      raise PermissionError.new(
+               'You have insufficient privileges to perform action.')
+    end
+  end
+
+end
diff --git a/src/app/services/vm_service.rb b/src/app/services/vm_service.rb
new file mode 100644
index 0000000..0c964de
--- /dev/null
+++ b/src/app/services/vm_service.rb
@@ -0,0 +1,189 @@
+#
+# Copyright (C) 2009 Red Hat, Inc.
+# Written by Scott Seago <sseago at redhat.com>,
+#            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.
+# Mid-level API: Business logic around individual VM's
+module VmService
+
+  include ApplicationService
+
+  def svc_show(vm_id)
+    # from before_filter
+    @vm = Vm.find(vm_id)
+    @perm_obj = @vm.vm_resource_pool
+    @current_pool_id=@perm_obj.id
+    authorized!(Privilege::VIEW)
+  end
+
+  def svc_create(vm_hash, start_now)
+    # from before_filter
+    vm_hash[:state] = Vm::STATE_PENDING
+    @vm = Vm.new(params[:vm])
+    @perm_obj = @vm.vm_resource_pool
+    @current_pool_id=@perm_obj.id
+    authorized!(Privilege::MODIFY)
+
+    alert = "VM was successfully created."
+    Vm.transaction do
+      @vm.save!
+      vm_provision
+      @task = VmTask.new({ :user        => @user,
+                           :task_target => @vm,
+                           :action      => VmTask::ACTION_CREATE_VM,
+                           :state       => Task::STATE_QUEUED})
+      @task.save!
+      if start_now
+        if @vm.get_action_list.include?(VmTask::ACTION_START_VM)
+          @task = VmTask.new({ :user        => @user,
+                               :task_target => @vm,
+                               :action      => VmTask::ACTION_START_VM,
+                               :state       => Task::STATE_QUEUED})
+          @task.save!
+          alert += " VM Start action queued."
+        else
+          alert += " Resources are not available to start VM now."
+        end
+      end
+    end
+    return alert
+  end
+
+  def svc_update(vm_id, vm_hash, start_now, restart_now)
+    # from before_filter
+    @vm = Vm.find(vm_id)
+    @perm_obj = @vm.vm_resource_pool
+    @current_pool_id=@perm_obj.id
+    authorized!(Privilege::MODIFY)
+
+    #needs restart if certain fields are changed
+    # (since those will only take effect the next startup)
+    needs_restart = false
+    unless @vm.get_pending_state == Vm::STATE_STOPPED
+      Vm::NEEDS_RESTART_FIELDS.each do |field|
+        unless @vm[field].to_s == vm_hash[field]
+          needs_restart = true
+          break
+        end
+      end
+      current_storage_ids = @vm.storage_volume_ids.sort
+      new_storage_ids = vm_hash[:storage_volume_ids]
+      new_storage_ids = [] unless new_storage_ids
+      new_storage_ids = new_storage_ids.sort.collect {|x| x.to_i }
+      needs_restart = true unless current_storage_ids == new_storage_ids
+    end
+    vm_hash[:needs_restart] = 1 if needs_restart
+
+
+    alert = "VM was successfully updated."
+    Vm.transaction do
+      @vm.update_attributes!(vm_hash)
+      vm_provision
+      if start_now
+        if @vm.get_action_list.include?(VmTask::ACTION_START_VM)
+          @task = VmTask.new({ :user        => @user,
+                               :task_target => @vm,
+                               :action      => VmTask::ACTION_START_VM,
+                               :state       => Task::STATE_QUEUED})
+          @task.save!
+          alert += " VM Start action queued."
+        else
+          alert += " Resources are not available to start VM now."
+        end
+      elsif restart_now
+        if @vm.get_action_list.include?(VmTask::ACTION_SHUTDOWN_VM)
+          @task = VmTask.new({ :user        => @user,
+                               :task_target => @vm,
+                               :action      => VmTask::ACTION_SHUTDOWN_VM,
+                               :state       => Task::STATE_QUEUED})
+          @task.save!
+          @task = VmTask.new({ :user    => @user,
+                               :task_target => @vm,
+                               :action  => VmTask::ACTION_START_VM,
+                               :state   => Task::STATE_QUEUED})
+          @task.save!
+          alert += " VM Restart action queued."
+        else
+          alert += " Restart action was not available."
+        end
+      end
+    end
+    return alert
+  end
+
+  def svc_destroy(vm_id)
+    # from before_filter
+    @vm = Vm.find(vm_id)
+    @perm_obj = @vm.vm_resource_pool
+    @current_pool_id=@perm_obj.id
+    authorized!(Privilege::MODIFY)
+
+    unless @vm.is_destroyable?
+      raise ActionError.new("Virtual Machine must be stopped to delete
it")
+    end
+    destroy_cobbler_system(@vm)
+    @vm.destroy
+    return "Virtual Machine wa ssuccessfully deleted."
+  end
+
+  def svc_vm_action(vm_id, vm_action, action_args)
+    @vm = Vm.find(vm_id)
+    @perm_obj = @vm.vm_resource_pool
+    @current_pool_id=@perm_obj.id
+    authorized!(Privilege::MODIFY)
+    unless @vm.queue_action(@user, vm_action, action_args)
+      raise ActionError.new("#{vm_action} is an invalid action.")
+    end
+    return "#{vm_action} was successfully queued."
+  end
+
+  def svc_cancel_queued_tasks(vm_id)
+    @vm = Vm.find(vm_id)
+    @perm_obj = @vm.vm_resource_pool
+    @current_pool_id=@perm_obj.id
+    authorized!(Privilege::MODIFY)
+
+    Task.transaction do
+      @vm.tasks.queued.each { |task| task.cancel}
+    end
+    return "Queued tasks were successfully canceled."
+  end
+
+  def vm_provision
+    if @vm.uses_cobbler?
+      # spaces are invalid in the cobbler name
+      name = @vm.uuid
+      system = Cobbler::System.find_one(name)
+      unless system
+        system = Cobbler::System.new("name" => name,
+                                     @vm.cobbler_type => @vm.cobbler_name)
+        system.interfaces = [Cobbler::NetworkInterface.new(
+                                    {'mac_address' =>
@vm.vnic_mac_addr})]
+        system.save
+      end
+    end
+  end
+
+  def destroy_cobbler_system(vm)
+    # Destroy the Cobbler system first if it's defined
+    if vm.uses_cobbler?
+      system = Cobbler::System.find_one(vm.cobbler_system_name)
+      system.remove if system
+    end
+  end
+
+end
diff --git a/src/app/views/vm/_form.rhtml b/src/app/views/vm/_form.rhtml
index 58a3d61..610f2bc 100644
--- a/src/app/views/vm/_form.rhtml
+++ b/src/app/views/vm/_form.rhtml
@@ -3,8 +3,7 @@
 <%  start_resources = @vm.vm_resource_pool.available_resources_for_vm(@vm)
%>
 
 <!--[form:vm]-->
-<%= hidden_field 'vm', 'vm_resource_pool_id' unless
@vm_resource_pool_name %>
-<%= hidden_field_tag 'vm_resource_pool_name', @vm_resource_pool_name
if @vm_resource_pool_name %>
+<%= hidden_field 'vm', 'vm_resource_pool_id' %>
 <%= hidden_field_tag 'hardware_pool_id', @hardware_pool.id if
@hardware_pool %>
 
     <%= text_field_with_label "Name:", "vm",
"description", {:style=>"width:250px;"}  %>
-- 
1.6.0.6

Scott Seago

2009-Apr-20 15:22 UTC

head link

[Ovirt-devel] Re: [PATCH server] First attempt at refactoring the vm_controller using the service layer.

Scott Seago wrote:> This code is based on David Lutterkort's proposal outlined in
> https://www.redhat.com/archives/ovirt-devel/2009-April/msg00086.html
>
> I've included service-layer methods for show, create, update, destroy,
vm_action, and cancel_queued_tasks. These are the concrete actions that make
sense as part of the API. Most of the remaining vm_controller methods are
pure-WUI actions -- for things such as generating the web form for a new VM (the
subsequent 'create' action _is_ part of the API).
>
> There's a certain amount of temporary code that we need in place while
only some of the controllers have been converted, as we still need permissions
pre-filters for non-converted create, etc. methods -- but for the converted
controllers we don't want to perform actions in before_filters that will be
done as part of the services layer. Any temporary methods, etc. that will go
away when the last controller is converted have been marked clearly with FIXME
comments.
>
> Signed-off-by: Scott Seago <sseago at redhat.com>
> ---
>  src/app/controllers/application.rb               |  113 +++++++----
>  src/app/controllers/hardware_controller.rb       |    3 +-
>  src/app/controllers/host_controller.rb           |    4 +-
>  src/app/controllers/nic_controller.rb            |    2 +-
>  src/app/controllers/permission_controller.rb     |   12 +-
>  src/app/controllers/pool_controller.rb           |    2 +-
>  src/app/controllers/quota_controller.rb          |    4 +-
>  src/app/controllers/storage_controller.rb        |    5 +-
>  src/app/controllers/storage_volume_controller.rb |    6 +-
>  src/app/controllers/task_controller.rb           |    9 +-
>  src/app/controllers/vm_controller.rb             |  230
+++++-----------------
>  src/app/models/vm.rb                             |    1 +
>  src/app/services/application_service.rb          |   56 ++++++
>  src/app/services/vm_service.rb                   |  189 ++++++++++++++++++
>  src/app/views/vm/_form.rhtml                     |    3 +-
>  15 files changed, 394 insertions(+), 245 deletions(-)
>  create mode 100644 src/app/services/application_service.rb
>  create mode 100644 src/app/services/vm_service.rb
>
> diff --git a/src/app/controllers/application.rb
b/src/app/controllers/application.rb
> index fa88773..34283ec 100644
> --- a/src/app/controllers/application.rb
> +++ b/src/app/controllers/application.rb
> @@ -20,19 +20,35 @@
>  # Filters added to this controller apply to all controllers in the
application.
>  # Likewise, all the methods added will be available for all controllers.
>  
> +# FIXME: once all controller classes include this, remove here
> +require 'services/vm_service'
> +
>   Oops. that should have been 'application_service' instead of 
'vm_service'. Changing that locally now...

Scott

Ian Main

2009-Apr-20 22:13 UTC

head link

[Ovirt-devel] [PATCH server] First attempt at refactoring the vm_controller using the service layer.

On Mon, 20 Apr 2009 15:13:13 +0000
Scott Seago <sseago at redhat.com> wrote:
> This code is based on David Lutterkort's proposal outlined in
> https://www.redhat.com/archives/ovirt-devel/2009-April/msg00086.html
[SNIP]
> +  def svc_create(vm_hash, start_now)
> +    # from before_filter
> +    vm_hash[:state] = Vm::STATE_PENDING
> +    @vm = Vm.new(params[:vm])
> +    @perm_obj = @vm.vm_resource_pool
> +    @current_pool_id=@perm_obj.id
> +    authorized!(Privilege::MODIFY)
> +
> +    alert = "VM was successfully created."
> +    Vm.transaction do
> +      @vm.save!
> +      vm_provision
> +      @task = VmTask.new({ :user        => @user,
> +                           :task_target => @vm,
> +                           :action      => VmTask::ACTION_CREATE_VM,
> +                           :state       => Task::STATE_QUEUED})
> +      @task.save!
> +      if start_now
> +        if @vm.get_action_list.include?(VmTask::ACTION_START_VM)
> +          @task = VmTask.new({ :user        => @user,
> +                               :task_target => @vm,
> +                               :action      => VmTask::ACTION_START_VM,
> +                               :state       => Task::STATE_QUEUED})
> +          @task.save!
I can see this is a bit odd.. if we're calling this from QMF then
there's going to be issues here..  In the current API I don't have
create VM returning a task, which is kind of beside the point, but if
it did then we would have to have a QMF object of this task figured
out when the command returned (there are other APIs that do return
tasks like start_vm).  I'm not entirely sure how that would work using
this API right now.. we'd have to somehow figure out which task it was
that was put in the DB.. I wonder if there is a better way to approach
this?

I'm not going to argue this hard here because I know this is a first cut
and just a refactoring but I do think there should be no tasks
associated with creating a VM definition for the API.  I think we've
talked about this before.. not a big deal, just wanted to bring it up
for future reference :).

	Ian

Ian Main

2009-Apr-20 22:22 UTC

head link

[Ovirt-devel] [PATCH server] First attempt at refactoring the vm_controller using the service layer.

On Mon, 20 Apr 2009 15:13:13 +0000
Scott Seago <sseago at redhat.com> wrote:
> This code is based on David Lutterkort's proposal outlined in
> https://www.redhat.com/archives/ovirt-devel/2009-April/msg00086.html
> [SNIP]
> +
> +  def svc_create(vm_hash, start_now)
> +    # from before_filter
> +    vm_hash[:state] = Vm::STATE_PENDING
> +    @vm = Vm.new(params[:vm])
> +    @perm_obj = @vm.vm_resource_pool
> +    @current_pool_id=@perm_obj.id
> +    authorized!(Privilege::MODIFY)
> +
> +    alert = "VM was successfully created."
> +    Vm.transaction do
> +      @vm.save!
> +      vm_provision
> +      @task = VmTask.new({ :user        => @user,
> +                           :task_target => @vm,
> +                           :action      => VmTask::ACTION_CREATE_VM,
> +                           :state       => Task::STATE_QUEUED})
> +      @task.save!
> +      if start_now
> +        if @vm.get_action_list.include?(VmTask::ACTION_START_VM)
> +          @task = VmTask.new({ :user        => @user,
> +                               :task_target => @vm,
> +                               :action      => VmTask::ACTION_START_VM,
> +                               :state       => Task::STATE_QUEUED})
> +          @task.save!
> +          alert += " VM Start action queued."
> +        else
> +          alert += " Resources are not available to start VM
now."
> +        end
> +      end
> +    end
> +    return alert
> +  end

I also just realized that we return the newly created VM definition in
the QMF API too.. so again we'd have to somehow figure out which new VM
was created when making this call..  I think it would be good to return
the VM ID?  Isn't this some kind of odd design where you have all these
side effects from calling a single method?
> +  def svc_update(vm_id, vm_hash, start_now, restart_now)
> +    # from before_filter
> +    @vm = Vm.find(vm_id)
> +    @perm_obj = @vm.vm_resource_pool
> +    @current_pool_id=@perm_obj.id
> +    authorized!(Privilege::MODIFY)
> +
> +    #needs restart if certain fields are changed
> +    # (since those will only take effect the next startup)
> +    needs_restart = false
> +    unless @vm.get_pending_state == Vm::STATE_STOPPED
> +      Vm::NEEDS_RESTART_FIELDS.each do |field|
> +        unless @vm[field].to_s == vm_hash[field]
> +          needs_restart = true
> +          break
> +        end
> +      end
> +      current_storage_ids = @vm.storage_volume_ids.sort
> +      new_storage_ids = vm_hash[:storage_volume_ids]
> +      new_storage_ids = [] unless new_storage_ids
> +      new_storage_ids = new_storage_ids.sort.collect {|x| x.to_i }
> +      needs_restart = true unless current_storage_ids == new_storage_ids
> +    end
> +    vm_hash[:needs_restart] = 1 if needs_restart
> +
> +
> +    alert = "VM was successfully updated."
> +    Vm.transaction do
> +      @vm.update_attributes!(vm_hash)
> +      vm_provision
> +      if start_now
> +        if @vm.get_action_list.include?(VmTask::ACTION_START_VM)
> +          @task = VmTask.new({ :user        => @user,
> +                               :task_target => @vm,
> +                               :action      => VmTask::ACTION_START_VM,
> +                               :state       => Task::STATE_QUEUED})
> +          @task.save!
> +          alert += " VM Start action queued."
> +        else
> +          alert += " Resources are not available to start VM
now."
> +        end
> +      elsif restart_now
> +        if @vm.get_action_list.include?(VmTask::ACTION_SHUTDOWN_VM)
> +          @task = VmTask.new({ :user        => @user,
> +                               :task_target => @vm,
> +                               :action      =>
VmTask::ACTION_SHUTDOWN_VM,
> +                               :state       => Task::STATE_QUEUED})
> +          @task.save!
> +          @task = VmTask.new({ :user    => @user,
> +                               :task_target => @vm,
> +                               :action  => VmTask::ACTION_START_VM,
> +                               :state   => Task::STATE_QUEUED})
> +          @task.save!
> +          alert += " VM Restart action queued."
> +        else
> +          alert += " Restart action was not available."
> +        end
> +      end
> +    end
> +    return alert
> +  end
Is this something we want in the API?  It seems a little odd to me from
a usage standpoint for the QMF implementation.  It will require that we
map the info from the active record into a hash, then pass it to this
method with the property changed, and then deal with side effects from
tasks starting?
> +  def svc_destroy(vm_id)
> +    # from before_filter
> +    @vm = Vm.find(vm_id)
> +    @perm_obj = @vm.vm_resource_pool
> +    @current_pool_id=@perm_obj.id
> +    authorized!(Privilege::MODIFY)
> +
> +    unless @vm.is_destroyable?
> +      raise ActionError.new("Virtual Machine must be stopped to
delete it")
> +    end
> +    destroy_cobbler_system(@vm)
> +    @vm.destroy
> +    return "Virtual Machine wa ssuccessfully deleted."
> +  end
> +
> +  def svc_vm_action(vm_id, vm_action, action_args)
> +    @vm = Vm.find(vm_id)
> +    @perm_obj = @vm.vm_resource_pool
> +    @current_pool_id=@perm_obj.id
> +    authorized!(Privilege::MODIFY)
> +    unless @vm.queue_action(@user, vm_action, action_args)
> +      raise ActionError.new("#{vm_action} is an invalid
action.")
> +    end
> +    return "#{vm_action} was successfully queued."
> +  end
> +
> +  def svc_cancel_queued_tasks(vm_id)
> +    @vm = Vm.find(vm_id)
> +    @perm_obj = @vm.vm_resource_pool
> +    @current_pool_id=@perm_obj.id
> +    authorized!(Privilege::MODIFY)
> +
> +    Task.transaction do
> +      @vm.tasks.queued.each { |task| task.cancel}
> +    end
> +    return "Queued tasks were successfully canceled."
> +  end
So I'm guessing things like this we would just ignore?  I'm thinking we
may end up using only a portion of the calls here..  In QMF I would
think you would just do a search of open tasks where the target is the
VM in question and then call cancel on them.
> +
> +  def vm_provision
> +    if @vm.uses_cobbler?
> +      # spaces are invalid in the cobbler name
> +      name = @vm.uuid
> +      system = Cobbler::System.find_one(name)
> +      unless system
> +        system = Cobbler::System.new("name" => name,
> +                                     @vm.cobbler_type =>
@vm.cobbler_name)
> +        system.interfaces = [Cobbler::NetworkInterface.new(
> +                                    {'mac_address' =>
@vm.vnic_mac_addr})]
> +        system.save
> +      end
> +    end
> +  end
I'm not sure how these fit in either..
> +  def destroy_cobbler_system(vm)
> +    # Destroy the Cobbler system first if it's defined
> +    if vm.uses_cobbler?
> +      system = Cobbler::System.find_one(vm.cobbler_system_name)
> +      system.remove if system
> +    end
> +  end
    Ian

Ovirt devel - Apr 2009 - [PATCH server] First attempt at refactoring the vm_controller using the service layer.

[Ovirt-devel] [PATCH server] First attempt at refactoring the vm_controller using the service layer.

[Ovirt-devel] Re: [PATCH server] First attempt at refactoring the vm_controller using the service layer.

[Ovirt-devel] [PATCH server] First attempt at refactoring the vm_controller using the service layer.

[Ovirt-devel] [PATCH server] First attempt at refactoring the vm_controller using the service layer.