Scott Seago
2009-May-04 18:47 UTC
[Ovirt-devel] Revised pool controller service layer refactoring patches
This replaces the earlier patch sent for pool controller service layer changes. The storage volume controller patch fixes a prior bug in authentication for that controller, and the final patch caches the set_perms results for multi-object operations when the perm_obj remains constant.
Scott Seago
2009-May-04 18:47 UTC
[Ovirt-devel] [PATCH server] controller service layer refactoring for pools. (revised)
This is the second round of controller/service layer refactoring changes -- in
this case for the pool-related controllers (HW, VM, and Smart pools)
Revised for feedback from David Lutterkort
Signed-off-by: Scott Seago <sseago at redhat.com>
---
src/app/controllers/application.rb | 28 ++--
src/app/controllers/hardware_controller.rb | 213 +++++--------------------
src/app/controllers/pool_controller.rb | 130 +++++++++++++---
src/app/controllers/resources_controller.rb | 121 +++++----------
src/app/controllers/smart_pools_controller.rb | 119 ++++++--------
src/app/models/hardware_pool.rb | 48 ------
src/app/models/host.rb | 4 +
src/app/models/pool.rb | 3 +
src/app/models/smart_pool.rb | 19 ---
src/app/models/storage_pool.rb | 4 +
src/app/models/vm.rb | 6 +
src/app/models/vm_resource_pool.rb | 4 +
src/app/models/vm_task.rb | 8 +-
src/app/services/application_service.rb | 7 +
src/app/services/hardware_pool_service.rb | 127 +++++++++++++++
src/app/services/pool_service.rb | 60 +++++++
src/app/services/smart_pool_service.rb | 79 +++++++++
src/app/services/vm_resource_pool_service.rb | 76 +++++++++
src/app/views/resources/quick_summary.rhtml | 2 +-
src/app/views/resources/vm_actions.rhtml | 6 +-
src/app/views/smart_pools/_form.rhtml | 2 +-
21 files changed, 635 insertions(+), 431 deletions(-)
create mode 100644 src/app/services/hardware_pool_service.rb
create mode 100644 src/app/services/pool_service.rb
create mode 100644 src/app/services/smart_pool_service.rb
create mode 100644 src/app/services/vm_resource_pool_service.rb
diff --git a/src/app/controllers/application.rb
b/src/app/controllers/application.rb
index e5f4d4b..bdd5b64 100644
--- a/src/app/controllers/application.rb
+++ b/src/app/controllers/application.rb
@@ -20,8 +20,6 @@
# 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/application_service'
class ApplicationController < ActionController::Base
# FIXME: once all controller classes include this, remove here
@@ -118,17 +116,7 @@ class ApplicationController < ActionController::Base
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
+ html_error_page(msg)
end
format.json do
@json_hash ||= {}
@@ -139,7 +127,19 @@ class ApplicationController < ActionController::Base
format.xml { head :forbidden }
end
end
-
+ def html_error_page(msg)
+ @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
# 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
diff --git a/src/app/controllers/hardware_controller.rb
b/src/app/controllers/hardware_controller.rb
index 2158e08..384b4aa 100644
--- a/src/app/controllers/hardware_controller.rb
+++ b/src/app/controllers/hardware_controller.rb
@@ -19,6 +19,7 @@
#
class HardwareController < PoolController
+ include HardwarePoolService
EQ_ATTRIBUTES = [ :name, :parent_id ]
@@ -27,10 +28,7 @@ class HardwareController < PoolController
verify :method => [:post, :delete], :only => :destroy,
:redirect_to => { :action => :list }
- before_filter :pre_modify, :only => [:add_hosts, :move_hosts,
- :add_storage, :move_storage,
- :create_storage, :delete_storage,
- :move, :removestorage]
+ before_filter :pre_modify, :only => [:move, :removestorage]
def index
if params[:path]
@@ -113,8 +111,14 @@ class HardwareController < PoolController
end
def show_storage
- @storage_tree = @pool.storage_tree(:filter_unavailable => false,
:include_used => true).to_json
- show
+ 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
end
def show_tasks
@@ -124,12 +128,12 @@ class HardwareController < PoolController
def hosts_json
if params[:exclude_host]
- pre_show
+ pre_show_pool
hosts = @pool.hosts
find_opts = {:conditions => ["id != ?",
params[:exclude_host]]}
include_pool = false
elsif params[:id]
- pre_show
+ pre_show_pool
hosts = @pool.hosts
find_opts = {}
include_pool = false
@@ -154,7 +158,7 @@ class HardwareController < PoolController
def storage_pools_json
if params[:id]
- pre_show
+ pre_show_pool
storage_pools = @pool.storage_pools
find_opts = {:conditions => "type !=
'LvmStoragePool'"}
include_pool = false
@@ -190,144 +194,48 @@ class HardwareController < PoolController
super
end
- def create
- resource_type = params[:resource_type]
- resource_ids_str = params[:resource_ids]
- resource_ids = []
- resource_ids = resource_ids_str.split(",").collect {|x| x.to_i}
if resource_ids_str
- begin
- @pool.create_with_resources(@parent, resource_type, resource_ids)
- respond_to do |format|
- format.html {
- reply = { :object => "pool", :success => true,
- :alert => "Hardware Pool was successfully created." }
- reply[:resource_type] = resource_type if resource_type
- render :json => reply
- }
- format.xml {
- render :xml => @pool.to_xml(XML_OPTS),
- :status => :created,
- :location => hardware_pool_url(@pool)
- }
- end
- rescue
- respond_to do |format|
- format.json {
- render :json => { :object => "pool", :success =>
false,
- :errors => @pool.errors.localize_error_messages.to_a }
- }
- format.xml { render :xml => @pool.errors,
- :status => :unprocessable_entity }
- end
- end
- end
-
- def update
- if params[:hardware_pool]
- # FIXME: For the REST API, we allow moving hosts/storage through
- # update. It makes that operation convenient for clients, though makes
- # the implementation here somewhat ugly.
- [:hosts, :storage_pools].each do |k|
- objs = params[:hardware_pool].delete(k)
- ids = objs.reject{ |obj| obj[:hardware_pool_id] == @pool.id}.
- collect{ |obj| obj[:id] }
- if ids.size > 0
- # FIXME: use self.move_hosts/self.move_storage
- if k == :hosts
- @pool.move_hosts(ids, @pool.id)
- else
- @pool.move_storage(ids, @pool.id)
- end
- end
- end
- # FIXME: HTML views should use :hardware_pool
- params[:pool] = params.delete(:hardware_pool)
- end
-
- begin
- @pool.update_attributes!(params[:pool])
- respond_to do |format|
- format.json {
- render :json => { :object => "pool", :success =>
true,
- :alert => "Hardware Pool was successfully modified." }
- }
- format.xml {
- render :xml => @pool.to_xml(XML_OPTS),
- :status => :created,
- :location => hardware_pool_url(@pool)
- }
- end
- rescue
- respond_to do |format|
- format.json {
- render :json => { :object => "pool", :success =>
false,
- :errors => @pool.errors.localize_error_messages.to_a}
- }
- format.xml {
- render :xml => @pool.errors,
- :status => :unprocessable_entity
- }
- end
- end
+ def additional_create_params
+ {:resource_type => params[:resource_type],
+ :resource_ids => params[:resource_ids],
+ :parent_id => (params[:hardware_pool] ?
+ params[:hardware_pool][:parent_id] :
+ params[:parent_id])}
end
def add_hosts
- edit_items(Host, :move_hosts, @pool.id, :add)
+ edit_items(@pool.id, :svc_move_hosts, :add)
end
def move_hosts
- edit_items(Host, :move_hosts, params[:target_pool_id], :move)
+ edit_items(params[:target_pool_id], :svc_move_hosts, :move)
end
def add_storage
- edit_items(StoragePool, :move_storage, @pool.id, :add)
+ edit_items(@pool.id, :svc_move_storage, :add)
end
def move_storage
- edit_items(StoragePool, :move_storage, params[:target_pool_id], :move)
+ edit_items(params[:target_pool_id], :svc_move_storage, :move)
end
- #FIXME: we need permissions checks. user must have permission on src pool
- # in addition to the current pool (which is checked). We also need to fail
- # for storage that aren't currently empty
- def edit_items(item_class, item_method, target_pool_id, item_action)
- resource_ids_str = params[:resource_ids]
- resource_ids = resource_ids_str.split(",").collect {|x| x.to_i}
-
- # if user doesn't have modify permission on both source and destination
- unless @pool.can_modify(@user) and
Pool.find(target_pool_id).can_modify(@user)
- render :json => { :success => false,
- :alert => "Cannot #{item_action.to_s}
#{item_class.table_name.humanize} without admin permissions on both pools"
}
- return
- end
-
- # relay error message if movable check fails for any resource
- success = true
- failed_resources = ""
- resource_ids.each {|x|
- unless item_class.find(x).movable?
- success = false
- failed_resources += x.to_s + " "
- end
- }
- resource_ids.delete_if { |x| ! item_class.find(x).movable? }
-
+ def edit_items(svc_method, target_pool_id, item_action)
begin
- @pool.transaction do
- @pool.send(item_method, resource_ids, target_pool_id)
- end
- rescue
- success = false
- end
-
- if success
- render :json => { :success => true,
- :alert => "#{item_action.to_s}
#{item_class.table_name.humanize} successful.",
- :storage => @pool.storage_tree({:filter_unavailable => false,
:include_used => true, :state => item_action.to_s})}
- else
- render :json => { :success => false,
- :alert => "#{item_action.to_s}
#{item_class.table_name.humanize} failed" +
- (failed_resources == "" ? "." : "
for " + failed_resources) }
+ 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
end
@@ -335,33 +243,6 @@ class HardwareController < PoolController
render :layout => 'popup'
end
- def destroy
- parent = @pool.parent
- if not(parent)
- alert="You can't delete the top level Hardware pool."
- success=false
- status=:method_not_allowed
- elsif not(@pool.children.empty?)
- alert = "You can't delete a Pool without first deleting its
children."
- success=false
- status=:conflict
- else
- if @pool.move_contents_and_destroy
- alert="Hardware Pool was successfully deleted."
- success=true
- status=:ok
- else
- alert="Failed to delete hardware pool."
- success=false
- status=:internal_server_error
- end
- end
- respond_to do |format|
- format.json { render :json => { :object => "pool",
:success => success,
- :alert => alert } }
- format.xml { head status }
- end
- end
protected
#filter methods
@@ -369,25 +250,11 @@ class HardwareController < PoolController
@pool = HardwarePool.new
super
end
- def pre_create
- # FIXME: REST and browsers send params differently. Should be fixed
- # in the views
- if params[:pool]
- @pool = HardwarePool.new(params[:pool])
- else
- @pool = HardwarePool.new(params[:hardware_pool])
- end
- super
- end
def pre_edit
@pool = HardwarePool.find(params[:id])
@parent = @pool.parent
set_perms(@pool)
end
- def pre_show
- @pool = HardwarePool.find(params[:id])
- super
- end
def pre_modify
pre_edit
authorize_admin
diff --git a/src/app/controllers/pool_controller.rb
b/src/app/controllers/pool_controller.rb
index 06f8768..911bc92 100644
--- a/src/app/controllers/pool_controller.rb
+++ b/src/app/controllers/pool_controller.rb
@@ -20,12 +20,9 @@
class PoolController < ApplicationController
- before_filter :pre_show_pool, :only => [:show_vms, :show_users,
- :show_hosts, :show_storage,
- :users_json, :show_tasks, :tasks,
+ before_filter :pre_show_pool, :only => [:users_json, :show_tasks, :tasks,
:vm_pools_json,
- :pools_json, :show_pools,
- :storage_volumes_json,
:quick_summary]
+ :pools_json, :storage_volumes_json]
XML_OPTS = {
:include => [ :storage_pools, :hosts, :quota ]
@@ -40,6 +37,15 @@ class PoolController < ApplicationController
end
def show
+ begin
+ svc_show(params[:id])
+ render_show
+ rescue PermissionError => perm_error
+ handle_auth_error(perm_error.message)
+ end
+ end
+
+ def render_show
respond_to do |format|
format.html {
render :layout => 'tabs-and-content' if params[:ajax]
@@ -49,10 +55,15 @@ class PoolController < ApplicationController
render :xml => @pool.to_xml(XML_OPTS)
}
end
- end
+ end
def quick_summary
- render :layout => 'selection'
+ begin
+ svc_show(params[:id])
+ render :layout => 'selection'
+ rescue PermissionError => perm_error
+ handle_auth_error(perm_error.message)
+ end
end
# resource's users list page
@@ -97,30 +108,111 @@ class PoolController < ApplicationController
render :layout => 'popup'
end
+ 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
+ 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
+ end
+ end
+
+ def additional_create_params
+ {}
+ end
+
def edit
render :layout => 'popup'
end
+ def destroy
+ alert = nil
+ success = true
+ status = :ok
+ begin
+ alert = svc_destroy(params[:id])
+ rescue ActionError => error
+ alert = error.message
+ success = false
+ status = :conflict
+ rescue PermissionError => error
+ alert = error.message
+ success = false
+ status = :forbidden
+ rescue Exception => error
+ alert = error.message
+ success = false
+ status = :method_not_allowed
+ end
+ respond_to do |format|
+ format.json { render :json => { :object => "pool",
:success => success,
+ :alert => alert } }
+ format.xml { head status }
+ end
+ end
+
protected
def pre_new
@parent = Pool.find(params[:parent_id])
set_perms(@parent)
end
- def pre_create
- #this is currently only true for the rest API for hardware pools
- if params[:hardware_pool]
- @parent = Pool.find(params[:hardware_pool][:parent_id])
- else
- @parent = Pool.find(params[:parent_id])
- end
- set_perms(@parent)
- end
def pre_show_pool
- pre_show
- end
- def pre_show
+ @pool = Pool.find(params[:id])
set_perms(@pool)
authorize_view
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/controllers/resources_controller.rb
b/src/app/controllers/resources_controller.rb
index c61ef46..7ad79ca 100644
--- a/src/app/controllers/resources_controller.rb
+++ b/src/app/controllers/resources_controller.rb
@@ -18,13 +18,12 @@
# also available at http://www.gnu.org/copyleft/gpl.html.
class ResourcesController < PoolController
+ include VmResourcePoolService
def index
list
render :action => 'list'
end
- before_filter :pre_vm_actions, :only => [:vm_actions]
-
# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }
@@ -69,89 +68,62 @@ class ResourcesController < PoolController
end
def vms_json
- pre_show
+ pre_show_pool
super(:full_items => @pool.vms, :find_opts => {}, :include_pool =>
:true)
end
- def create
- begin
- @pool.create_with_parent(@parent)
- render :json => { :object => "vm_resource_pool", :success
=> true,
- :alert => "Virtual Machine Pool was
successfully created." }
- rescue
- render :json => { :object => "vm_resource_pool", :success
=> false,
- :errors =>
@pool.errors.localize_error_messages.to_a}
- end
- end
-
- def update
- begin
- @pool.update_attributes!(params[:pool])
- render :json => { :object => "vm_resource_pool", :success
=> true,
- :alert => "Virtual Machine Pool was
successfully modified." }
- rescue
- render :json => { :object => "vm_resource_pool", :success
=> false,
- :errors =>
@pool.errors.localize_error_messages.to_a}
- end
+ def additional_create_params
+ {:parent_id => (params[:hardware_pool] ?
+ params[:hardware_pool][:parent_id] :
+ params[:parent_id])}
end
- #FIXME: we need permissions checks. user must have permission. We also need
to fail
+ #FIXME: we need permissions checks. user must have permission. We also need
to fail
# for pools that aren't currently empty
def delete
- vm_pool_ids_str = params[:vm_pool_ids]
- vm_pool_ids = vm_pool_ids_str.split(",").collect {|x| x.to_i}
- vm_pool_names = []
- begin
- VmResourcePool.transaction do
- pools = VmResourcePool.find(:all, :conditions => "id in
(#{vm_pool_ids.join(', ')})")
- pools.each do |pool|
- vm_pool_names << pool.name
- pool.destroy
- end
+ vm_pool_ids = params[:vm_pool_ids].split(",")
+ successes = []
+ failures = {}
+ vm_pool_ids.each do |pool_id|
+ begin
+ svc_destroy(pool_id)
+ successes << @pool
+ rescue PermissionError => perm_error
+ failures[@pool] = perm_error.message
+ rescue Exception => ex
+ failures[@pool] = ex.message
end
- render :json => { :object => "vm_resource_pool", :success
=> true,
- :alert => "Virtual Machine Pools
#{vm_pool_names.join(', ')} were successfully deleted." }
- rescue
- render :json => { :object => "vm_resource_pool", :success
=> false,
- :alert => "Error in deleting Virtual Machine
Pools."}
end
- end
-
- def destroy
- if @pool.destroy
- alert="Virtual Machine Pool was successfully deleted."
- success=true
- else
- alert="Failed to delete virtual machine pool."
- success=false
+ success = failures.empty?
+ alert = ""
+ if !successes.empty?
+ alert = "Virtual Machine Pools #{successes.collect{|pool|
pool.name}.join(', ')} were successfully deleted."
+ end
+ if !failures.empty?
+ alert += " Errors in deleting VM Pools #{failures.collect{|pool,err|
"#{pool.name}: #{err}"}.join(', ')}."
end
- render :json => { :object => "vm_resource_pool", :success
=> success, :alert => alert }
+ render :json => { :object => "vm_resource_pool", :success
=> success,
+ :alert => alert }
end
def vm_actions
- @action = params[:vm_action]
- @action_label = VmTask.action_label(@action)
- vms_str = params[:vm_ids]
- @vms = vms_str.split(",").collect {|x| Vm.find(x.to_i)}
- @success_list = []
- @failure_list = []
begin
- @pool.transaction do
- @vms.each do |vm|
- if vm.queue_action(@user, @action)
- @success_list << vm
- print vm.description, vm.id, "\n"
- else
- @failure_list << vm
- end
- end
- end
- rescue
+ alert = svc_vm_actions_hosts(params[:id], params[:vm_action],
+ params[:vm_ids].split(","))
+ @success_list = @vms
+ @failures = {}
+ render :layout => 'confirmation'
+ rescue PermissionError => perm_error
+ handle_auth_error(perm_error.message)
+ rescue PartialSuccessError => error
+ @success_list = error.successes
+ @failures = error.failures
+ render :layout => 'confirmation'
+ rescue Exeption => ex
flash[:errmsg] = 'Error queueing VM actions.'
@success_list = []
@failure_list = []
end
- render :layout => 'confirmation'
end
protected
@@ -159,26 +131,11 @@ class ResourcesController < PoolController
@pool = VmResourcePool.new
super
end
- def pre_create
- @pool = VmResourcePool.new(params[:pool])
- super
- end
def pre_edit
@pool = VmResourcePool.find(params[:id])
@parent = @pool.parent
@current_pool_id=@pool.id
set_perms(@pool.parent)
end
- def pre_show
- @pool = VmResourcePool.find(params[:id])
- super
- @is_hwpool_admin = @pool.parent.can_modify(@user)
- end
- def pre_vm_actions
- @pool = VmResourcePool.find(params[:id])
- @parent = @pool.parent
- set_perms(@pool)
- authorize_user
- end
end
diff --git a/src/app/controllers/smart_pools_controller.rb
b/src/app/controllers/smart_pools_controller.rb
index fb6ccb5..993d285 100644
--- a/src/app/controllers/smart_pools_controller.rb
+++ b/src/app/controllers/smart_pools_controller.rb
@@ -19,12 +19,9 @@
#
class SmartPoolsController < PoolController
+ include SmartPoolService
- before_filter :pre_modify, :only => [:add_hosts, :remove_hosts,
- :add_storage, :remove_storage,
- :add_vms, :remove_vms,
- :add_pools, :remove_pools,
- :add_items, :add_pool_dialog]
+ before_filter :pre_modify, :only => [:add_pool_dialog]
def show_vms
show
end
@@ -38,30 +35,19 @@ class SmartPoolsController < PoolController
end
def show_storage
- @storage_tree = @pool.storage_tree(:filter_unavailable => false,
:include_used => true).to_json
- show
- end
-
- def create
begin
- @pool.create_with_parent(@parent)
- render :json => { :object => "smart_pool", :success =>
true,
- :alert => "Smart Pool was successfully
created." }
- rescue
- render :json => { :object => "smart_pool", :success =>
false,
- :errors =>
@pool.errors.localize_error_messages.to_a}
+ 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
end
- def update
- begin
- @pool.update_attributes!(params[:smart_pool])
- render :json => { :object => "smart_pool", :success =>
true,
- :alert => "Smart Pool was successfully
modified." }
- rescue
- render :json => { :object => "smart_pool", :success =>
false,
- :errors =>
@pool.errors.localize_error_messages.to_a}
- end
+ def additional_create_params
+ {:parent_id => (params[:hardware_pool] ?
+ params[:hardware_pool][:parent_id] :
+ params[:parent_id])}
end
def add_pool_dialog
@@ -98,7 +84,7 @@ class SmartPoolsController < PoolController
def items_json_internal(item_class, item_assoc)
if params[:id]
- pre_show
+ pre_show_pool
full_items = @pool.send(item_assoc)
find_opts = {}
include_pool = false
@@ -120,80 +106,82 @@ class SmartPoolsController < PoolController
end
def add_hosts
- edit_items(Host, :add_items, :add)
+ add_or_remove_items(Host, :add)
end
def remove_hosts
- edit_items(Host, :remove_items, :remove)
+ add_or_remove_items(Host, :remove)
end
def add_storage
- edit_items(StoragePool, :add_items, :add)
+ add_or_remove_items(StoragePool, :add)
end
def remove_storage
- edit_items(StoragePool, :remove_items, :remove)
+ add_or_remove_items(StoragePool, :remove)
end
def add_vms
- edit_items(Vm, :add_items, :add)
+ add_or_remove_items(Vm, :add)
end
def remove_vms
- edit_items(Vm, :remove_items, :remove)
+ add_or_remove_items(Vm, :remove)
end
def add_pools
- edit_items(Pool, :add_items, :add)
+ add_or_remove_items(Pool, :add)
end
def remove_pools
- edit_items(Pool, :remove_items, :remove)
+ add_or_remove_items(Pool, :remove)
end
- def edit_items(item_class, item_method, item_action)
- resource_ids_str = params[:resource_ids]
- resource_ids = resource_ids_str.split(",").collect {|x| x.to_i}
+ def add_or_remove_items(item_class, item_action)
begin
- @pool.send(item_method,item_class, resource_ids)
- render :json => { :success => true,
- :alert => "#{item_action.to_s}
#{item_class.table_name.humanize} successful." }
+ 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
end
def add_items
class_and_ids_str = params[:class_and_ids]
- class_and_ids = class_and_ids_str.split(",").collect {|x|
x.split("_")}
+ class_and_ids = class_and_ids_str.split(",").collect do
|class_and_id_str|
+ class_and_id = class_and_id_str.split("_")
+ class_and_id[0] = class_and_id[0].constantize
+ class_and_id[1] = class_and_id[1].to_a
+ end
begin
- @pool.transaction do
- class_and_ids.each do |class_and_id|
-
@pool.add_item(class_and_id[0].constantize.find(class_and_id[1].to_i))
- end
- end
- render :json => { :success => true,
- :alert => "Add items to smart pool successful." }
- rescue => ex
+ alert = svc_add_remove_items(params[:id], nil, :add, class_and_ids)
+ render :json => { :success => true, :alert => alert}
+ rescue
render :json => { :success => false,
- :alert => "Add items to smart pool failed: " +
ex.message }
+ :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
end
- def destroy
- if @pool.destroy
- alert="Smart Pool was successfully deleted."
- success=true
- else
- alert="Failed to delete Smart pool."
- success=false
- end
- render :json => { :object => "smart_pool", :success =>
success, :alert => alert }
- end
-
protected
#filter methods
def pre_new
@@ -201,20 +189,11 @@ class SmartPoolsController < PoolController
@parent = DirectoryPool.get_or_create_user_root(get_login_user)
set_perms(@parent)
end
- def pre_create
- @pool = SmartPool.new(params[:smart_pool])
- @parent = DirectoryPool.get_or_create_user_root(get_login_user)
- set_perms(@parent)
- end
def pre_edit
@pool = SmartPool.find(params[:id])
@parent = @pool.parent
set_perms(@pool)
end
- def pre_show
- @pool = SmartPool.find(params[:id])
- super
- end
def pre_modify
pre_edit
authorize_admin
diff --git a/src/app/models/hardware_pool.rb b/src/app/models/hardware_pool.rb
index 7015854..0e7e745 100644
--- a/src/app/models/hardware_pool.rb
+++ b/src/app/models/hardware_pool.rb
@@ -40,54 +40,6 @@ class HardwarePool < Pool
hw_root ? hw_root.named_child(DEFAULT_POOL_NAME) : nil
end
- def create_with_resources(parent, resource_type= nil, resource_ids=[])
- create_with_parent(parent) do
- if resource_type == "hosts"
- move_hosts(resource_ids, id)
- elsif resource_type == "storage"
- move_storage(resource_ids, id)
- end
- end
- end
-
- def move_hosts(host_ids, target_pool_id)
- hosts = Host.find(:all, :conditions => "id in
(#{host_ids.join(', ')})")
- transaction do
- hosts.each do |host|
- host.hardware_pool = HardwarePool.find(target_pool_id)
- host.save!
- end
- end
- end
-
- def move_storage(storage_pool_ids, target_pool_id)
- storage_pools = StoragePool.find(:all, :conditions => "id in
(#{storage_pool_ids.join(', ')})")
- transaction do
- storage_pools.each do |storage_pool|
- storage_pool.hardware_pool_id = target_pool_id
- storage_pool.save!
- end
- end
- end
-
-
- # todo: does this method still make sense? or should we just enforce
"empty" pools?
- def move_contents_and_destroy
- transaction do
- parent_id = parent.id
- hosts.each do |host|
- host.hardware_pool_id=parent_id
- host.save
- end
- storage_pools.each do |vol|
- vol.hardware_pool_id=parent_id
- vol.save
- end
- # what about quotas -- for now they're deleted
- destroy
- end
- end
-
def total_storage_volumes
storage_pools.inject(0) { |sum, pool| sum += pool.storage_volumes.size}
end
diff --git a/src/app/models/host.rb b/src/app/models/host.rb
index 06d7388..0665c3f 100644
--- a/src/app/models/host.rb
+++ b/src/app/models/host.rb
@@ -134,6 +134,10 @@ class Host < ActiveRecord::Base
hardware_pool.search_users
end
+ def permission_obj
+ hardware_pool
+ end
+
def movable?
return vms.size == 0
end
diff --git a/src/app/models/pool.rb b/src/app/models/pool.rb
index 6f2a086..2979fcb 100644
--- a/src/app/models/pool.rb
+++ b/src/app/models/pool.rb
@@ -306,6 +306,9 @@ class Pool < ActiveRecord::Base
def class_and_id
self.class.name + "_" + self.id.to_s
end
+ def permission_obj
+ self
+ end
protected
def traverse_parents
if id
diff --git a/src/app/models/smart_pool.rb b/src/app/models/smart_pool.rb
index dba000d..1f718a8 100644
--- a/src/app/models/smart_pool.rb
+++ b/src/app/models/smart_pool.rb
@@ -53,25 +53,6 @@ class SmartPool < Pool
:tagged_id=>item.id}).destroy
end
- def add_items(item_class, item_ids)
- items = item_class.find(:all, :conditions => "id in
(#{item_ids.join(', ')})")
- transaction do
- items.each { |item| add_item(item)}
- end
- end
-
- def remove_items(item_class, item_ids)
- tags = smart_pool_tags.find(:all,
- :conditions => "tagged_id in
- (#{item_ids.join(',
')})
- and
tagged_type='#{item_class.name}'")
- transaction do
- tags.each do |tag|
- tag.destroy
- end
- end
- end
-
def self.smart_pools_for_user(user)
nested_pools = DirectoryPool.get_smart_root.full_set_nested(
:privilege => Privilege::MODIFY, :user => user,
diff --git a/src/app/models/storage_pool.rb b/src/app/models/storage_pool.rb
index fbe5954..92548bd 100644
--- a/src/app/models/storage_pool.rb
+++ b/src/app/models/storage_pool.rb
@@ -155,6 +155,10 @@ class StoragePool < ActiveRecord::Base
return_hash
end
+ def permission_obj
+ hardware_pool
+ end
+
def movable?
storage_volumes.each{ |x|
return false unless x.movable?
diff --git a/src/app/models/vm.rb b/src/app/models/vm.rb
index c62595a..6880c22 100644
--- a/src/app/models/vm.rb
+++ b/src/app/models/vm.rb
@@ -177,11 +177,17 @@ class Vm < ActiveRecord::Base
:in => EFFECTIVE_STATE.keys
+ def get_vm_pool
+ vm_resource_pool
+ end
def get_hardware_pool
pool = vm_resource_pool
pool = pool.get_hardware_pool if pool
pool
end
+ def permission_obj
+ vm_resource_pool
+ end
def storage_volume_ids
storage_volumes.collect {|x| x.id }
end
diff --git a/src/app/models/vm_resource_pool.rb
b/src/app/models/vm_resource_pool.rb
index 1ab4ab1..e41dcab 100644
--- a/src/app/models/vm_resource_pool.rb
+++ b/src/app/models/vm_resource_pool.rb
@@ -32,6 +32,10 @@ class VmResourcePool < Pool
traverse_parents { |pool| pool if pool[:type] == HardwarePool.name}
end
+ def get_vm_pool
+ self
+ end
+
def allocated_resources(exclude_vm = nil)
pending_cpus = 0
pending_memory = 0
diff --git a/src/app/models/vm_task.rb b/src/app/models/vm_task.rb
index ed33564..d7afe54 100644
--- a/src/app/models/vm_task.rb
+++ b/src/app/models/vm_task.rb
@@ -36,7 +36,7 @@ class VmTask < Task
# for migrate VM action, args provides the optional target host
ACTION_MIGRATE_VM = "migrate_vm"
- PRIV_OBJECT_VM_POOL = "vm_resource_pool"
+ PRIV_OBJECT_VM_POOL = "get_vm_pool"
PRIV_OBJECT_HW_POOL = "get_hardware_pool"
@@ -145,6 +145,12 @@ class VmTask < Task
actions
end
+ def self.action_privilege(action)
+ return ACTIONS[action][:privilege][0]
+ end
+ def self.action_privilege_object(action, obj)
+ return obj.send(ACTIONS[action][:privilege][1])
+ end
def self.action_label(action)
return ACTIONS[action][:label]
end
diff --git a/src/app/services/application_service.rb
b/src/app/services/application_service.rb
index 4dc5eba..bd0a308 100644
--- a/src/app/services/application_service.rb
+++ b/src/app/services/application_service.rb
@@ -29,6 +29,13 @@
module ApplicationService
class PermissionError < RuntimeError; end
class ActionError < RuntimeError; end
+ class PartialSuccessError < RuntimeError
+ attr_reader :failures, :successes
+ def initialize(msg, failures={}, successes=[])
+ @failures = failures
+ @successes = successes
+ end
+ end
# Including class must provide a GET_LOGIN_USER
diff --git a/src/app/services/hardware_pool_service.rb
b/src/app/services/hardware_pool_service.rb
new file mode 100644
index 0000000..c9aa70b
--- /dev/null
+++ b/src/app/services/hardware_pool_service.rb
@@ -0,0 +1,127 @@
+#
+# 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 HW pools
+module HardwarePoolService
+
+ include PoolService
+
+ def svc_create(pool_hash, other_args)
+ # from before_filter
+ @pool = HardwarePool.new(pool_hash)
+ @parent = Pool.find(other_args[:parent_id])
+ authorized!(Privilege::MODIFY, at parent)
+
+ alert = "Hardware Pool was successfully created."
+ Pool.transaction do
+ @pool.create_with_parent(@parent)
+ begin
+ if other_args[:resource_type] == "hosts"
+ svc_move_hosts(@pool.id,
other_args[:resource_ids].split(","), @pool.id)
+ elsif other_args[:resource_type] == "storage"
+ svc_move_storage(@pool.id,
other_args[:resource_ids].split(","), @pool.id)
+ end
+ # wrapped in a transaction, so fail on partial success
+ rescue PartialSuccessError => ex
+ # Raising ActionError here since we're aborting the transaction.
Errors
+ # on creation here result in no persistent changes to the database.
+ raise ActionError.new("Could not move all hosts or storage to this
pool")
+ end
+ end
+ return alert
+ end
+
+ def svc_move_hosts(pool_id, host_ids, target_pool_id)
+ svc_move_items_internal(pool_id, Host, host_ids, target_pool_id)
+ end
+ def svc_move_storage(pool_id, storage_pool_ids, target_pool_id)
+ svc_move_items_internal(pool_id, StoragePool, storage_pool_ids,
target_pool_id)
+ end
+ def svc_move_items_internal(pool_id, item_class, resource_ids,
target_pool_id)
+ # from before_filter
+ @pool = HardwarePool.find(pool_id)
+ target_pool = Pool.find(target_pool_id)
+ authorized!(Privilege::MODIFY,target_pool)
+ authorized!(Privilege::MODIFY, at pool) unless @pool == target_pool
+
+ resources = item_class.find(resource_ids)
+
+ # relay error message if movable check fails for any resource
+ success = true
+ failed_resources = {}
+ successful_resources = []
+ resources.each do |resource|
+ begin
+ if !resource.movable?
+ failed_resources[resource] = "Not Movable"
+ elsif ! resource.hardware_pool.can_modify(@user)
+ failed_resources[resource] = "Failed permission check"
+ else
+ resource.hardware_pool = target_pool
+ resource.save!
+ successful_resources << resource
+ end
+ rescue Exception => ex
+ failed_resources[resource] = ex.message
+ end
+ end
+ unless failed_resources.empty?
+ raise PartialSuccessError.new("Move
#{item_class.table_name.humanize} only partially successful",
+ failed_resources, successful_resources)
+ end
+ return "Move #{item_class.table_name.humanize} successful."
+ end
+
+ def additional_update_actions(pool, pool_hash)
+ # FIXME: For the REST API, we allow moving hosts/storage through
+ # update. It makes that operation convenient for clients, though makes
+ # the implementation here somewhat ugly.
+ begin
+ [:hosts, :storage_pools].each do |k|
+ objs = pool_hash.delete(k)
+ ids = objs.reject{ |obj| obj[:hardware_pool_id] == @pool.id}.
+ collect{ |obj| obj[:id] }
+ if ids.size > 0
+ if k == :hosts
+ svc_move_hosts(pool.id, ids, pool.id)
+ else
+ svc_move_storage(pool.id, ids, pool.id)
+ end
+ end
+ end
+ # wrapped in a transaction, so fail on partial success
+ rescue PartialSuccessError => ex
+ raise ActionError.new("Could not move all hosts or storage to this
pool")
+ end
+ end
+
+ def check_destroy_preconditions
+ msg = nil
+ if @pool == HardwarePool.get_default_pool
+ msg = "You can't delete the top level Hardware pool"
+ elsif not(@pool.children.empty?)
+ msg = "You can't delete a Pool without first deleting its
children."
+ elsif not(@pool.hosts.empty?)
+ msg = "You can't delete a Pool without first moving its
hosts."
+ elsif not(@pool.storage_pools.empty?)
+ msg = "You can't delete a Pool without first moving its
storage."
+ end
+ raise ActionError.new(msg) if msg
+ end
+end
diff --git a/src/app/services/pool_service.rb b/src/app/services/pool_service.rb
new file mode 100644
index 0000000..2377094
--- /dev/null
+++ b/src/app/services/pool_service.rb
@@ -0,0 +1,60 @@
+#
+# 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 pools
+module PoolService
+
+ include ApplicationService
+
+ def svc_show(pool_id)
+ # from before_filter
+ @pool = Pool.find(pool_id)
+ authorized!(Privilege::VIEW, at pool)
+ end
+
+ def update_perms
+ set_perms(@pool)
+ end
+ def additional_update_actions(pool, pool_hash)
+ end
+
+ def svc_update(pool_id, pool_hash)
+ # from before_filter
+ @pool = Pool.find(params[:id])
+ @parent = @pool.parent
+ update_perms
+ authorized!(Privilege::MODIFY)
+ Pool.transaction do
+ additional_update_actions(@pool, pool_hash)
+ @pool.update_attributes!(pool_hash)
+ end
+ end
+
+ def svc_destroy(pool_id)
+ # from before_filter
+ @pool = Pool.find(pool_id)
+ authorized!(Privilege::MODIFY, @pool)
+ check_destroy_preconditions
+ @pool.destroy
+ return "Pool was successfully deleted."
+ end
+
+ def check_destroy_preconditions
+ end
+end
diff --git a/src/app/services/smart_pool_service.rb
b/src/app/services/smart_pool_service.rb
new file mode 100644
index 0000000..344b9e8
--- /dev/null
+++ b/src/app/services/smart_pool_service.rb
@@ -0,0 +1,79 @@
+#
+# 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 smart pools
+module SmartPoolService
+
+ include PoolService
+
+ def svc_create(pool_hash, other_args)
+ # from before_filter
+ @pool = SmartPool.new(pool_hash)
+ @parent = DirectoryPool.get_or_create_user_root(get_login_user)
+ authorized!(Privilege::MODIFY, at parent)
+
+ alert = "Smart Pool was successfully created."
+ @pool.create_with_parent(@parent)
+ return alert
+ end
+
+ # if item_class is nil, resource_ids is an array of [class, id] pairs
+ def svc_add_remove_items(pool_id, item_class, item_action, resource_ids)
+ # from before_filter
+ @pool = SmartPool.find(pool_id)
+ @parent = @pool.parent
+ authorized!(Privilege::MODIFY, at pool)
+ unless [:add, :remove].include?(item_action)
+ raise ActionError.new("Invalid action #{item_action}")
+ end
+ if item_class
+ resources = item_class.find(resource_ids)
+ else
+ resources = resource_ids.collect {|the_class,id| the_class.find(id)}
+ end
+
+ # relay error message if movable check fails for any resource
+ success = true
+ failed_resources = {}
+ successful_resources = []
+ resources.each do |resource|
+ begin
+ if item_action == :add
+ if ! resource.permission_obj.can_view(@user)
+ failed_resources[resource] = "Failed permission check"
+ else
+ @pool.add_item(resource)
+ successful_resources << resource
+ end
+ elsif item_action == :remove
+ @pool.remove_item(resource)
+ successful_resources << resource
+ end
+ rescue Exception => ex
+ failed_resources[resource] = ex.message
+ end
+ end
+ unless failed_resources.empty?
+ raise PartialSuccessError.new("#{item_action.to_s}
#{item_class.table_name.humanize if item_class} only partially successful",
+ failed_resources, successful_resources)
+ end
+ return "#{item_action.to_s} #{item_class.table_name.humanize}
successful."
+ end
+
+end
diff --git a/src/app/services/vm_resource_pool_service.rb
b/src/app/services/vm_resource_pool_service.rb
new file mode 100644
index 0000000..30f7106
--- /dev/null
+++ b/src/app/services/vm_resource_pool_service.rb
@@ -0,0 +1,76 @@
+#
+# 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 VM pools
+module VmResourcePoolService
+
+ include PoolService
+
+ def svc_create(pool_hash, other_args)
+ # from before_filter
+ @pool = VmResourcePool.new(pool_hash)
+ @parent = Pool.find(other_args[:parent_id])
+ authorized!(Privilege::MODIFY, at parent)
+
+ alert = "VM Pool was successfully created."
+ @pool.create_with_parent(@parent)
+ return alert
+ end
+
+ def update_perms
+ @current_pool_id=@pool.id
+ set_perms(@pool.parent)
+ end
+
+ def svc_vm_actions(pool_id, vm_action, vm_ids)
+ # from before_filter
+ @pool = VmResourcePool.find(pool_id)
+ @parent = @pool.parent
+ @action = vm_action
+ @action_label = VmTask.action_label(@action)
+ authorized!(VmTask.action_privilege(@action),
+ VmTask.action_privilege_object(@action, at pool))
+
+ @vms = Vm.find(vm_ids)
+
+ successful_vms = []
+ failed_vms = {}
+ @vms.each do |vm|
+ begin
+ if vm.vm_resource_pool != @pool
+ failed_vms[vm] = "VM #{vm.description} does not belong to the
current pool."
+ elsif vm.queue_action(@user, @action)
+ successful_vms << vm
+ else
+ failed_vms[vm] = "unavailable action"
+ end
+ rescue Exception => ex
+ failed_vms[vm] = ex.message
+ end
+ end
+ unless failed_vms.empty?
+ raise PartialSuccessError.new("#{@action} only partially
successful",
+ failed_vms, successful_vms)
+ end
+ return "Action #{@action} successful."
+ end
+
+
+
+end
diff --git a/src/app/views/resources/quick_summary.rhtml
b/src/app/views/resources/quick_summary.rhtml
index 31c4033..70b8df6 100644
--- a/src/app/views/resources/quick_summary.rhtml
+++ b/src/app/views/resources/quick_summary.rhtml
@@ -2,7 +2,7 @@
<%=h @pool.name %> quota
<%- end -%>
<%- content_for :action_links do -%>
- <%if @is_hwpool_admin -%>
+ <%if @pool.parent.can_modify(@user) -%>
<%= link_to image_tag("icon_edit.png") + " Edit",
{:controller => 'resources', :action =>
'edit', :id => @pool},
:rel=>"facebox[.bolder]",
:class=>"selection_facebox" %>
diff --git a/src/app/views/resources/vm_actions.rhtml
b/src/app/views/resources/vm_actions.rhtml
index d556b94..b3fa1ad 100644
--- a/src/app/views/resources/vm_actions.rhtml
+++ b/src/app/views/resources/vm_actions.rhtml
@@ -25,10 +25,10 @@ Action succeeded for these VMs:
<% end %>
</ul>
-Action invalid for these VMs:
+Action failed for these VMs:
<ul>
- <% for vm in @failure_list %>
- <li><%= vm.description %></li>
+ <% for vm, msg in @failures %>
+ <li><%= vm.description %>: <%= msg %></li>
<% end %>
</ul>
</div>
diff --git a/src/app/views/smart_pools/_form.rhtml
b/src/app/views/smart_pools/_form.rhtml
index 2f2156a..c1ec6bd 100644
--- a/src/app/views/smart_pools/_form.rhtml
+++ b/src/app/views/smart_pools/_form.rhtml
@@ -1,6 +1,6 @@
<%= error_messages_for 'vm_resource_pool' %>
<!--[form:vm_resource_pool]-->
-<%= text_field_with_label "Name", 'smart_pool',
'name' %>
+<%= text_field_with_label "Name", 'pool', 'name'
%>
<!--[eoform:vm_resource_pool]-->
--
1.6.0.6
Scott Seago
2009-May-04 18:47 UTC
[Ovirt-devel] [PATCH server] fixed some problems with create/delete storage volume functionality.
One of the recent refactoring rounds seems to have broken the 'new storage
volume' functionality. It looks like the necessary before_filters
weren't being executed.
Signed-off-by: Scott Seago <sseago at redhat.com>
---
src/app/controllers/storage_volume_controller.rb | 39 ++++++++++++---------
1 files changed, 22 insertions(+), 17 deletions(-)
diff --git a/src/app/controllers/storage_volume_controller.rb
b/src/app/controllers/storage_volume_controller.rb
index b6b0593..d4a2561 100644
--- a/src/app/controllers/storage_volume_controller.rb
+++ b/src/app/controllers/storage_volume_controller.rb
@@ -22,19 +22,16 @@ class StorageVolumeController < ApplicationController
def new
@return_to_workflow = params[:return_to_workflow] || false
if params[:storage_pool_id]
- @storage_pool = StoragePool.find(params[:storage_pool_id])
unless @storage_pool.user_subdividable
- #fixme: proper error page for popups
- redirect_to :controller => 'dashboard'
+ html_error_page("User-created storage volumes are not supported on
this pool")
return
end
- new_volume_internal(@storage_pool,
- { :storage_pool_id => params[:storage_pool_id]})
+ @storage_volume = StorageVolume.factory(@storage_pool.get_type_label,
+ { :storage_pool_id =>
+ params[:storage_pool_id]})
else
- @source_volume = StorageVolume.find(params[:source_volume_id])
unless @source_volume.supports_lvm_subdivision
- #fixme: proper error page for popups
- redirect_to :controller => 'dashboard'
+ html_error_page("LVM is not supported for this storage
volume")
return
end
lvm_pool = @source_volume.lvm_storage_pool
@@ -47,7 +44,8 @@ class StorageVolumeController < ApplicationController
lvm_pool.source_volumes << @source_volume
lvm_pool.save!
end
- new_volume_internal(lvm_pool, { :storage_pool_id => lvm_pool.id})
+ @storage_volume = StorageVolume.factory(lvm_pool.get_type_label,
+ { :storage_pool_id =>
lvm_pool.id})
@storage_volume.lv_owner_perms='0744'
@storage_volume.lv_group_perms='0744'
@storage_volume.lv_mode_perms='0744'
@@ -99,7 +97,7 @@ class StorageVolumeController < ApplicationController
format.html { render :layout => 'selection' }
format.json do
attr_list = []
- attr_list << :id if (@storage_pool.user_subdividable and
authorized?(Privilege::MODIFY)
+ attr_list << :id if (@storage_pool.user_subdividable and
authorized?(Privilege::MODIFY))
attr_list += [:display_name, :size_in_gb, :get_type_label]
json_list(@storage_pool.storage_volumes, attr_list)
end
@@ -109,8 +107,6 @@ class StorageVolumeController < ApplicationController
end
def destroy
- @storage_volume = StorageVolume.find(params[:id])
- set_perms(@storage_volume.storage_pool.hardware_pool)
unless authorized?(Privilege::MODIFY) and
@storage_volume.storage_pool.user_subdividable
handle_auth_error("You do not have permission to delete this storage
volume.")
else
@@ -123,6 +119,16 @@ class StorageVolumeController < ApplicationController
end
end
+ def pre_new
+ if params[:storage_pool_id]
+ @storage_pool = StoragePool.find(params[:storage_pool_id])
+ set_perms(@storage_pool.hardware_pool)
+ else
+ @source_volume = StorageVolume.find(params[:source_volume_id])
+ set_perms(@source_volume.storage_pool.hardware_pool)
+ end
+ end
+
def pre_create
volume = params[:storage_volume]
unless type = params[:storage_type]
@@ -132,14 +138,13 @@ class StorageVolumeController < ApplicationController
set_perms(@storage_volume.storage_pool.hardware_pool)
authorize_admin
end
-
- private
- def new_volume_internal(storage_pool, new_params)
- @storage_volume = StorageVolume.factory(storage_pool.get_type_label,
new_params)
+ # will go away w/ svc layer
+ def pre_edit
+ @storage_volume = StorageVolume.find(params[:id])
set_perms(@storage_volume.storage_pool.hardware_pool)
- authorize_admin
end
+ private
def delete_volume_internal(volume)
begin
name = volume.display_name
--
1.6.0.6