Ovirt devel - May 2008 - PATCH: constrain storage_pool to avoid duplicate targets

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.

Hope this helps,
Steve Linabery
-------------- next part --------------
A non-text attachment was scrubbed...
Name: storage_pools.patch
Type: text/x-patch
Size: 2900 bytes
Desc: not available
URL:
<http://listman.redhat.com/archives/ovirt-devel/attachments/20080430/781eb4d9/attachment.bin>

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