[Ovirt-devel] [PATCH server 2/3] Additional vm_controller refactoring:
Scott Seago
sseago at redhat.com
Fri May 8 19:34:10 UTC 2009
1) Moved all remaining auth/before_filter action into service layer
2) Added rdoc comments
3) delete VMs action now uses svc_destroy action
Signed-off-by: Scott Seago <sseago at redhat.com>
---
src/app/controllers/vm_controller.rb | 89 +++++++-------------------
src/app/services/vm_service.rb | 103 ++++++++++++++++++++++++++----
src/app/views/resources/vm_actions.rhtml | 2 +-
3 files changed, 114 insertions(+), 80 deletions(-)
diff --git a/src/app/controllers/vm_controller.rb b/src/app/controllers/vm_controller.rb
index 29c0f16..a40fc5c 100644
--- a/src/app/controllers/vm_controller.rb
+++ b/src/app/controllers/vm_controller.rb
@@ -17,7 +17,6 @@
# 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
@@ -26,8 +25,6 @@ class VmController < ApplicationController
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :controller => 'dashboard' }
- before_filter :pre_console, :only => [:console]
-
def index
@vms = Vm.find(:all,
:include => [{:vm_resource_pool =>
@@ -52,7 +49,10 @@ class VmController < ApplicationController
end
def new
+ alert = svc_new(params[:vm_resource_pool_id])
+ _setup_provisioning_options
@storage_tree = VmResourcePool.find(params[:vm_resource_pool_id]).get_hardware_pool.storage_tree.to_json
+ @networks = Network.find(:all).collect{ |net| [net.name, net.id] }
render :layout => 'popup'
end
@@ -63,6 +63,9 @@ class VmController < ApplicationController
end
def edit
+ svc_modify(params[:id])
+ _setup_provisioning_options
+ @networks = Network.find(:all).collect{ |net| [net.name, net.id] }
@storage_tree = @vm.vm_resource_pool.get_hardware_pool.storage_tree(:vm_to_include => @vm).to_json
render :layout => 'popup'
end
@@ -74,37 +77,26 @@ class VmController < ApplicationController
render :json => { :object => "vm", :success => true, :alert => alert }
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}
- failure_list = []
- success = false
- begin
- Vm.transaction do
- vms = Vm.find(:all, :conditions => "id in (#{vm_ids.join(', ')})")
- vms.each do |vm|
- if vm.is_destroyable?
- destroy_cobbler_system(vm)
- vm.destroy
- else
- failure_list << vm.description
- end
- end
- end
- if failure_list.empty?
- success = true
- alert = "Virtual Machines were successfully deleted."
- else
- alert = "The following Virtual Machines were not deleted (a VM must be stopped to delete it): "
- alert+= failure_list.join(', ')
+ vm_ids = params[:vm_ids].split(",")
+ successes = []
+ failures = {}
+ vm_ids.each do |vm_id|
+ begin
+ svc_destroy(vm_id)
+ successes << @vm
+ rescue PermissionError => perm_error
+ failures[@vm] = perm_error.message
+ rescue Exception => ex
+ failures[@vm] = ex.message
end
- rescue
- alert = "Error deleting virtual machines."
end
- render :json => { :object => "vm", :success => success, :alert => alert }
+ unless failures.empty?
+ raise PartialSuccessError.new("Delete failed for some VMs",
+ failures, successes)
+ end
+ render :json => { :object => "vm", :success => success,
+ :alert => "VM Pools were successfully deleted." }
end
def destroy
@@ -142,6 +134,7 @@ class VmController < ApplicationController
end
def console
+ svc_modify(params[:id])
@show_vnc_error = "Console is unavailable for VM #{@vm.description}" unless @vm.has_console
if @vm.host.hostname.match("priv\.ovirt\.org$")
@vnc_hostname = IPSocket.getaddress(@vm.host.hostname)
@@ -172,40 +165,6 @@ class VmController < ApplicationController
end
end
- def pre_new
- unless params[:vm_resource_pool_id]
- flash[:notice] = "VM Resource Pool is required."
- redirect_to :controller => 'dashboard'
- end
-
- # random MAC
- mac = [ 0x00, 0x16, 0x3e, rand(0x7f), rand(0xff), rand(0xff) ]
- # random uuid
- uuid = ["%02x" * 4, "%02x" * 2, "%02x" * 2, "%02x" * 2, "%02x" * 6].join("-") %
- Array.new(16) {|x| rand(0xff) }
- newargs = {
- :vm_resource_pool_id => params[:vm_resource_pool_id],
- :vnic_mac_addr => mac.collect {|x| "%02x" % x}.join(":"),
- :uuid => uuid
- }
- @vm = Vm.new( newargs )
- unless params[:vm_resource_pool_id]
- @vm.vm_resource_pool = @vm_resource_pool
- end
- set_perms(@vm.vm_resource_pool)
- @networks = Network.find(:all).collect{ |net| [net.name, net.id] }
- _setup_provisioning_options
- end
- def pre_edit
- @vm = Vm.find(params[:id])
- set_perms(@vm.vm_resource_pool)
- @networks = Network.find(:all).collect{ |net| [net.name, net.id] }
- _setup_provisioning_options
- end
- def pre_console
- pre_edit
- authorize_user
- 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
diff --git a/src/app/services/vm_service.rb b/src/app/services/vm_service.rb
index 8256730..ff1a919 100644
--- a/src/app/services/vm_service.rb
+++ b/src/app/services/vm_service.rb
@@ -22,12 +22,54 @@ module VmService
include ApplicationService
- def svc_show(vm_id)
- # from before_filter
- @vm = Vm.find(vm_id)
- authorized!(Privilege::VIEW, at vm.vm_resource_pool)
+ # Load the Vm with +id+ for viewing
+ #
+ # === Instance variables
+ # [<tt>@vm</tt>] stores the Vm with +id+
+ # === Required permissions
+ # [<tt>Privilege::VIEW</tt>] on vm's VmResourcePool
+ def svc_show(id)
+ lookup(id,Privilege::VIEW)
+ end
+
+ # Load the Vm with +id+ for editing
+ #
+ # === Instance variables
+ # [<tt>@vm</tt>] stores the Vm with +id+
+ # === Required permissions
+ # [<tt>Privilege::MODIFY</tt>] on vm's VmResourcePool
+ def svc_modify(id)
+ lookup(id,Privilege::MODIFY)
end
+ # Load a new Vm for creating
+ #
+ # === Instance variables
+ # [<tt>@vm</tt>] loads a new Vm object into memory
+ # === Required permissions
+ # [<tt>Privilege::MODIFY</tt>] for the vm's VmResourcePool as specified by
+ # +vm_resource_pool_id+
+ def svc_new(vm_resource_pool_id)
+ raise ActionError.new("VM Resource Pool is required.") unless vm_resource_pool_id
+
+ # random MAC
+ mac = [ 0x00, 0x16, 0x3e, rand(0x7f), rand(0xff), rand(0xff) ]
+ # random uuid
+ uuid = ["%02x"*4, "%02x"*2, "%02x"*2, "%02x"*2, "%02x"*6].join("-") %
+ Array.new(16) {|x| rand(0xff) }
+
+ @vm = Vm.new({:vm_resource_pool_id => vm_resource_pool_id,
+ :vnic_mac_addr => mac.collect {|x| "%02x" % x}.join(":"),
+ :uuid => uuid })
+ authorized!(Privilege::MODIFY, @vm.vm_resource_pool)
+ end
+
+ # Save a new Vm
+ #
+ # === Instance variables
+ # [<tt>@vm</tt>] the newly saved Vm
+ # === Required permissions
+ # [<tt>Privilege::MODIFY</tt>] for the vm's VmResourcePool
def svc_create(vm_hash, start_now)
# from before_filter
vm_hash[:state] = Vm::STATE_PENDING
@@ -59,9 +101,15 @@ module VmService
return alert
end
- def svc_update(vm_id, vm_hash, start_now, restart_now)
+ # Update attributes for the Vm with +id+
+ #
+ # === Instance variables
+ # [<tt>@vm</tt>] stores the Vm with +id+
+ # === Required permissions
+ # [<tt>Privilege::MODIFY</tt>] for the Vm's VmResourcePool
+ def svc_update(id, vm_hash, start_now, restart_now)
# from before_filter
- @vm = Vm.find(vm_id)
+ @vm = Vm.find(id)
authorized!(Privilege::MODIFY, @vm.vm_resource_pool)
#needs restart if certain fields are changed
@@ -119,9 +167,15 @@ module VmService
return alert
end
- def svc_destroy(vm_id)
+ # Destroys for the Vm with +id+
+ #
+ # === Instance variables
+ # [<tt>@vm</tt>] stores the Vm with +id+
+ # === Required permissions
+ # [<tt>Privilege::MODIFY</tt>] for the Vm's VmResourcePool
+ def svc_destroy(id)
# from before_filter
- @vm = Vm.find(vm_id)
+ @vm = Vm.find(id)
authorized!(Privilege::MODIFY, @vm.vm_resource_pool)
unless @vm.is_destroyable?
@@ -129,20 +183,34 @@ module VmService
end
destroy_cobbler_system(@vm)
@vm.destroy
- return "Virtual Machine wa ssuccessfully deleted."
+ return "Virtual Machine was successfully deleted."
end
- def svc_vm_action(vm_id, vm_action, action_args)
- @vm = Vm.find(vm_id)
- authorized!(Privilege::MODIFY, @vm.vm_resource_pool)
+ # Queues action +vm_action+ for Vm with +id+
+ #
+ # === Instance variables
+ # [<tt>@vm</tt>] stores the Vm with +id+
+ # === Required permissions
+ # permission is action-specific as determined by
+ # <tt>VmTask.action_privilege(@action)</tt>
+ def svc_vm_action(id, vm_action, action_args)
+ @vm = Vm.find(id)
+ authorized!(VmTask.action_privilege(vm_action),
+ VmTask.action_privilege_object(vm_action, at vm))
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)
+ # Cancels queued tasks for for Vm with +id+
+ #
+ # === Instance variables
+ # [<tt>@vm</tt>] stores the Vm with +id+
+ # === Required permissions
+ # [<tt>Privilege::MODIFY</tt>] for the Vm's VmResourcePool
+ def svc_cancel_queued_tasks(id)
+ @vm = Vm.find(id)
authorized!(Privilege::MODIFY, @vm.vm_resource_pool)
Task.transaction do
@@ -151,6 +219,7 @@ module VmService
return "Queued tasks were successfully canceled."
end
+ protected
def vm_provision
if @vm.uses_cobbler?
# spaces are invalid in the cobbler name
@@ -174,4 +243,10 @@ module VmService
end
end
+ private
+ def lookup(id, priv)
+ @vm = Vm.find(id)
+ authorized!(priv, at vm.vm_resource_pool)
+ end
+
end
diff --git a/src/app/views/resources/vm_actions.rhtml b/src/app/views/resources/vm_actions.rhtml
index b3fa1ad..2a50a31 100644
--- a/src/app/views/resources/vm_actions.rhtml
+++ b/src/app/views/resources/vm_actions.rhtml
@@ -20,7 +20,7 @@
<% end %>
Action succeeded for these VMs:
<ul>
- <% for vm in @success_list %>
+ <% for vm in @successes %>
<li><%= vm.description %></li>
<% end %>
</ul>
--
1.6.0.6
More information about the ovirt-devel
mailing list