[Ovirt-devel] PATCH: constrain storage_pool to avoid duplicate targets

[Hugh O. Brock]

On Wed, Apr 30, 2008 at 07:42:30PM -0500, steve linabery wrote:
> Hi Ovirt,
> 
> This little patch attempts to do the following:
> 0) Add a flash notice when storage pool creation fails and redirect to
> storage/show
> 1) Use validates_uniqueness_of in isci and nfs storage pool classes
> 2) Add a "FIXME" to storage/new.rhtml and storage/new2.rhtml so that
> the tip will stop breaking my page flow :)
> 
> I think 1) is sufficient for avoiding both duplicate storage pool
> targets as well as duplicate storage volumes, since afaict volumes are
> only detected/created after pools.
 
Looks good... but what's the "FIXME" for below?
--H

> diff --git a/wui/src/app/controllers/storage_controller.rb b/wui/src/app/controllers/storage_controller.rb
> index c977fcb..4784581 100644
> --- a/wui/src/app/controllers/storage_controller.rb
> +++ b/wui/src/app/controllers/storage_controller.rb
> @@ -104,6 +104,9 @@ class StorageController < ApplicationController
>        else
>          render :action => 'new'
>        end
> +    else 
> +      flash[:notice] = 'Storage Pool creation failed.'
> +      redirect_to :controller => @storage_pool.hardware_pool.get_controller, :action => 'show', :id => @storage_pool.hardware_pool_id
>      end
>    end
>  
> diff --git a/wui/src/app/models/iscsi_storage_pool.rb b/wui/src/app/models/iscsi_storage_pool.rb
> index 8164e7c..9cc104d 100644
> --- a/wui/src/app/models/iscsi_storage_pool.rb
> +++ b/wui/src/app/models/iscsi_storage_pool.rb
> @@ -18,6 +18,9 @@
>  # also available at http://www.gnu.org/copyleft/gpl.html.
>  
>  class IscsiStoragePool < StoragePool
> +  
> +  validates_uniqueness_of :ip_addr, :scope => [:port, :target]
> +  
>    def label_components
>      "#{target}"
>    end
> diff --git a/wui/src/app/models/nfs_storage_pool.rb b/wui/src/app/models/nfs_storage_pool.rb
> index 30608cd..3704cc0 100644
> --- a/wui/src/app/models/nfs_storage_pool.rb
> +++ b/wui/src/app/models/nfs_storage_pool.rb
> @@ -18,6 +18,9 @@
>  # also available at http://www.gnu.org/copyleft/gpl.html.
>  
>  class NfsStoragePool < StoragePool
> +
> +  validates_uniqueness_of :ip_addr, :scope => :export_path
> +
>    def label_components
>      "#{export_path}"
>    end
> diff --git a/wui/src/app/views/storage/new.rhtml b/wui/src/app/views/storage/new.rhtml
> index 9dbc240..80d4941 100644
> --- a/wui/src/app/views/storage/new.rhtml
> +++ b/wui/src/app/views/storage/new.rhtml
> @@ -22,7 +22,9 @@
>                  </div>
>  
>                  <div class="data-table-column">
> -                  <strong>Choose From Storage in <%= @hardware_pool.parent.name %></strong>
> +<strong>FIX ME: hardware_pool parent attribute</strong>
> +<!--                  <strong>Choose From Storage in < @hardware_pool.parent.name ></strong>
> +-->

What are we missing here?

Thanks,
--Hugh