[Ovirt-devel] [PATCH server 01/10] converted storage volume controller to use the service layer

Jason Guiditta jason.guiditta at gmail.com
Tue May 19 16:25:39 UTC 2009


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.

On Tue, May 19, 2009 at 10:23 AM, Scott Seago <sseago at redhat.com> wrote:

>
> Signed-off-by: Scott Seago <sseago at redhat.com>
> ---
>  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 http://www.gnu.org/copyleft/gpl.html.
>
>  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_#{@source_volume.id}
> -        new_params = { :vg_name => "ovirt_vg_#{@source_volume.id}",
> -          :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 =>
> lvm_pool.id})
> -      @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 http://www.gnu.org/copyleft/gpl.html.
>
>  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 <sseago 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 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, at 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_#{@source_volume.id}
> +        new_params = { :vg_name => "ovirt_vg_#{@source_volume.id}",
> +          :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 =>
> +                                              @storage_pool.id})
> +  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, at 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, at storage_pool.hardware_pool)
> +  end
> +
> +end
> --
> 1.6.0.6
>
> _______________________________________________
> Ovirt-devel mailing list
> Ovirt-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/ovirt-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/ovirt-devel/attachments/20090519/8aa05995/attachment.htm>


More information about the ovirt-devel mailing list