ACK. This works fine for me (once I remembered to restart tasko and db-omatic). Not that I could not test the lvm change, as I do not have lvm currently in my test deployment. <div class="gmail_quote">On Tue, May 19, 2009 at 10:23 AM, Scott Seago <<a href="mailto:sseago@redhat.com">sseago@redhat.com</a>> wrote: <blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> Signed-off-by: Scott Seago <<a href="mailto:sseago@redhat.com">sseago@redhat.com</a>> --- src/app/controllers/storage_volume_controller.rb | 164 +++++----------------- src/app/models/lvm_storage_volume.rb | 8 + src/app/services/storage_volume_service.rb | 145 +++++++++++++++++++ 3 files changed, 187 insertions(+), 130 deletions(-) create mode 100644 src/app/services/storage_volume_service.rb diff --git a/src/app/controllers/storage_volume_controller.rb b/src/app/controllers/storage_volume_controller.rb index 6bdbbdc..b87f9b6 100644 --- a/src/app/controllers/storage_volume_controller.rb +++ b/src/app/controllers/storage_volume_controller.rb @@ -18,156 +18,60 @@ # also available at <a href="http://www.gnu.org/copyleft/gpl.html" target="_blank">http://www.gnu.org/copyleft/gpl.html</a>. class StorageVolumeController < ApplicationController + include StorageVolumeService def new + svc_new(params[:storage_pool_id], params[:source_volume_id]) @return_to_workflow = params[:return_to_workflow] || false - if params[:storage_pool_id] - unless @storage_pool.user_subdividable - html_error_page("User-created storage volumes are not supported on this pool") - return - end - @storage_volume = StorageVolume.factory(@storage_pool.get_type_label, - { :storage_pool_id => - params[:storage_pool_id]}) - else - unless @source_volume.supports_lvm_subdivision - html_error_page("LVM is not supported for this storage volume") - return - end - lvm_pool = @source_volume.lvm_storage_pool - unless lvm_pool - # FIXME: what should we do about VG/LV names? - # for now auto-create VG name as ovirt_vg_#{@<a href="http://source_volume.id" target="_blank">source_volume.id</a>} - new_params = { :vg_name => "ovirt_vg_#{@<a href="http://source_volume.id" target="_blank">source_volume.id</a>}", - :hardware_pool_id => @source_volume.storage_pool.hardware_pool_id} - lvm_pool = StoragePool.factory(StoragePool::LVM, new_params) - lvm_pool.source_volumes << @source_volume - lvm_pool.save! - end - @storage_volume = StorageVolume.factory(lvm_pool.get_type_label, - { :storage_pool_id => <a href="http://lvm_pool.id" target="_blank">lvm_pool.id</a>}) - @storage_volume.lv_owner_perms='0744' - @storage_volume.lv_group_perms='0744' - @storage_volume.lv_mode_perms='0744' - end render :layout => 'popup' end def create - begin - StorageVolume.transaction do - @storage_volume.save! - @task = StorageVolumeTask.new({ :user => @user, - :task_target => @storage_volume, - :action => StorageVolumeTask::ACTION_CREATE_VOLUME, - :state => Task::STATE_QUEUED}) - @task.save! - end - respond_to do |format| - format.json { render :json => { :object => "storage_volume", - :success => true, - :alert => "Storage Volume was successfully created." , - :new_volume => @storage_volume.storage_tree_element({:filter_unavailable => false, :state => 'new'})} } - format.xml { render :xml => @storage_volume, - :status => :created, - # FIXME: create storage_volume_url method if relevant - :location => storage_pool_url(@storage_volume) - } - end - rescue => ex - # FIXME: need to distinguish volume vs. task save errors - respond_to do |format| - format.json { - json_hash = { :object => "storage_volume", :success => false, - :errors => @storage_volume.errors.localize_error_messages.to_a } - json_hash[:message] = ex.message if json_hash[:errors].empty? - render :json => json_hash } - format.xml { render :xml => @storage_volume.errors, - :status => :unprocessable_entity } - end + volume = params[:storage_volume] + unless type = params[:storage_type] + type = volume.delete(:storage_type) + end + alert = svc_create(type, volume) + respond_to do |format| + format.json { render :json => { :object => "storage_volume", + :success => true, :alert => alert, + :new_volume => @storage_volume.storage_tree_element( + {:filter_unavailable => false, :state => 'new'})} } + format.xml { render :xml => @storage_volume, + :status => :created, + :location => storage_pool_url(@storage_volume) + } end end def show - @storage_volume = StorageVolume.find(params[:id]) - set_perms(@storage_volume.storage_pool.hardware_pool) - @storage_pool = @storage_volume.storage_pool - if authorize_view - respond_to do |format| - format.html { render :layout => 'selection' } - format.json do - attr_list = [] - 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 - format.xml { render :xml => @storage_volume.to_xml } + svc_show(params[:id]) + respond_to do |format| + format.html { render :layout => 'selection' } + format.json do + attr_list = [] + 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 + format.xml { render :xml => @storage_volume.to_xml } end end def destroy - unless authorized?(Privilege::MODIFY) and @storage_volume.storage_pool.user_subdividable - handle_error(:message => - "You do not have permission to delete this storage volume.", - :status => :forbidden, - :title => "Access Denied") - else - alert, success = delete_volume_internal(@storage_volume) - respond_to do |format| - format.json { render :json => { :object => "storage_volume", - :success => success, :alert => alert } } - format.xml { head(success ? :ok : :method_not_allowed) } - end + alert = svc_destroy(params[:id]) + respond_to do |format| + format.json { render :json => { :object => "storage_volume", + :success => true, :alert => alert } } + format.xml { head(:ok) } 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 + # 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 pre_create - volume = params[:storage_volume] - unless type = params[:storage_type] - type = volume.delete(:storage_type) - end - @storage_volume = StorageVolume.factory(type, volume) - set_perms(@storage_volume.storage_pool.hardware_pool) - authorize_admin - end - # will go away w/ svc layer - def pre_edit - @storage_volume = StorageVolume.find(params[:id]) - set_perms(@storage_volume.storage_pool.hardware_pool) - end - - private - def delete_volume_internal(volume) - begin - name = volume.display_name - if !volume.vms.empty? - vm_list = volume.vms.collect {|vm| vm.description}.join(", ") - ["Storage Volume #{name} must be unattached from VMs (#{vm_list}) before deleting it.", - false] - else - volume.state=StorageVolume::STATE_PENDING_DELETION - volume.save! - @task = StorageVolumeTask.new({ :user => @user, - :task_target => volume, - :action => StorageVolumeTask::ACTION_DELETE_VOLUME, - :state => Task::STATE_QUEUED}) - @task.save! - ["Storage Volume #{name} deletion was successfully queued.", true] - end - rescue => ex - ["Failed to delete storage volume #{name} (#{ex.message}.",false] - end + def tmp_authorize_admin end end diff --git a/src/app/models/lvm_storage_volume.rb b/src/app/models/lvm_storage_volume.rb index 5b6177b..8fd1bac 100644 --- a/src/app/models/lvm_storage_volume.rb +++ b/src/app/models/lvm_storage_volume.rb @@ -18,6 +18,14 @@ # also available at <a href="http://www.gnu.org/copyleft/gpl.html" target="_blank">http://www.gnu.org/copyleft/gpl.html</a>. class LvmStorageVolume < StorageVolume + + def initialize(params) + super + self.lv_owner_perms='0744' unless self.lv_owner_perms + self.lv_group_perms='0744' unless self.lv_group_perms + self.lv_mode_perms='0744' unless self.lv_mode_perms + end + def display_name "#{get_type_label}: #{storage_pool.vg_name}:#{lv_name}" end diff --git a/src/app/services/storage_volume_service.rb b/src/app/services/storage_volume_service.rb new file mode 100644 index 0000000..6d338a5 --- /dev/null +++ b/src/app/services/storage_volume_service.rb @@ -0,0 +1,145 @@ +# +# Copyright (C) 2009 Red Hat, Inc. +# Written by Scott Seago <<a href="mailto:sseago@redhat.com">sseago@redhat.com</a>>, +# +# 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 <a href="http://www.gnu.org/copyleft/gpl.html" target="_blank">http://www.gnu.org/copyleft/gpl.html</a>. +# Mid-level API: Business logic around storage volumes +module StorageVolumeService + + include ApplicationService + + # Load the StorageVolume with +id+ for viewing + # + # === Instance variables + # [<tt>@storage_volume</tt>] stores the Storage Volume with +id+ + # [<tt>@storage_pool</tt>] stores the Storage Volume's Storage Pool + # === Required permissions + # [<tt>Privilege::VIEW</tt>] on StorageVolume's HardwarePool + def svc_show(id) + lookup(id,Privilege::VIEW) + end + + # Load the StorageVolume with +id+ for editing + # + # === Instance variables + # [<tt>@storage_volume</tt>] stores the Storage Volume with +id+ + # [<tt>@storage_pool</tt>] stores the Storage Volume's Storage Pool + # === Required permissions + # [<tt>Privilege::MODIFY</tt>] on StorageVolume's HardwarePool + def svc_modify(id) + lookup(id,Privilege::MODIFY) + end + + # Load a new StorageVolume for creating + # + # === Instance variables + # [<tt>@storage_volume</tt>] loads a new StorageVolume object into memory + # [<tt>@storage_pool</tt>] Storage pool containing <tt>@storage_volume</tt> + # [<tt>@source_volume</tt>] Storage volume containing the LVM + # <tt>@storage_pool</tt> if storage type is LVM + # === Required permissions + # [<tt>Privilege::MODIFY</tt>] for the storage volume's HardwarePool + def svc_new(storage_pool_id, source_volume_id) + if storage_pool_id + @storage_pool = StoragePool.find(storage_pool_id) + unless @storage_pool.user_subdividable + raise ActionError.new("Unsupported action for " + + "#{@storage_pool.get_type_label} volumes.") + end + else + @source_volume = StorageVolume.find(source_volume_id) + @storage_pool = @source_volume.storage_pool + unless @source_volume.supports_lvm_subdivision + raise ActionError.new("LVM is not supported for this storage volume") + end + end + authorized!(Privilege::MODIFY,@storage_pool.hardware_pool) + + if source_volume_id + @storage_pool = @source_volume.lvm_storage_pool + unless @storage_pool + # FIXME: what should we do about VG/LV names? + # for now auto-create VG name as ovirt_vg_#{@<a href="http://source_volume.id" target="_blank">source_volume.id</a>} + new_params = { :vg_name => "ovirt_vg_#{@<a href="http://source_volume.id" target="_blank">source_volume.id</a>}", + :hardware_pool_id => @source_volume.storage_pool.hardware_pool_id} + @storage_pool = StoragePool.factory(StoragePool::LVM, new_params) + @storage_pool.source_volumes << @source_volume + @storage_pool.save! + end + end + @storage_volume = StorageVolume.factory(@storage_pool.get_type_label, + { :storage_pool_id => + @<a href="http://storage_pool.id" target="_blank">storage_pool.id</a>}) + end + + # Create a new StorageVolume + # + # === Instance variables + # [<tt>@storage_volume</tt>] the newly-created StorageVolume + # === Required permissions + # [<tt>Privilege::MODIFY</tt>] for the storage volume's HardwarePool + def svc_create(storage_type, storage_volume_hash) + @storage_volume = StorageVolume.factory(storage_type, storage_volume_hash) + authorized!(Privilege::MODIFY,@storage_volume.storage_pool.hardware_pool) + StorageVolume.transaction do + @storage_volume.save! + @task = StorageVolumeTask.new({ :user => @user, + :task_target => @storage_volume, + :action => StorageVolumeTask::ACTION_CREATE_VOLUME, + :state => Task::STATE_QUEUED}) + @task.save! + end + return "Storage Volume was successfully created." + end + + # Queues StorageVolume with +id+ for deletion + # + # === Instance variables + # [<tt>@storage_volume</tt>] stores the Storage Volume with +id+ + # [<tt>@storage_pool</tt>] stores the Storage Volume's Storage Pool + # === Required permissions + # [<tt>Privilege::MODIFY</tt>] on StorageVolume's HardwarePool + def svc_destroy(id) + lookup(id, Privilege::MODIFY) + unless @storage_pool.user_subdividable + raise ActionError.new("Unsupported action for " + + "#{@storage_volume.get_type_label} volumes.") + end + unless @storage_volume.vms.empty? + vms = @storage_volume.vms.collect {|vm| vm.description}.join(", ") + raise ActionError.new("Cannot delete storage assigned to VMs (#{vms})") + end + name = @storage_volume.display_name + StorageVolume.transaction do + @storage_volume.state=StorageVolume::STATE_PENDING_DELETION + @storage_volume.save! + @task = StorageVolumeTask.new({ :user => @user, + :task_target => @storage_volume, + :action => StorageVolumeTask::ACTION_DELETE_VOLUME, + :state => Task::STATE_QUEUED}) + @task.save! + end + return "Storage Volume #{name} deletion was successfully queued." + end + + private + def lookup(id, priv) + @storage_volume = StorageVolume.find(id) + @storage_pool = @storage_volume.storage_pool + authorized!(priv,@storage_pool.hardware_pool) + end + +end -- 1.6.0.6 _______________________________________________ Ovirt-devel mailing list <a href="mailto:Ovirt-devel@redhat.com">Ovirt-devel@redhat.com</a> <a href="https://www.redhat.com/mailman/listinfo/ovirt-devel" target="_blank">https://www.redhat.com/mailman/listinfo/ovirt-devel</a> </blockquote></div>