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

[steve linabery]

On Thu, May 1, 2008 at 8:05 AM, Hugh O. Brock <hbrock at redhat.com> wrote:
>
> 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
>

The 'FIXME' allows storage/new.rhtml and storage/new2.rhtml to be
served. Those pages are currently broken because they are referencing
an attribute called 'parent' in the HardwarePool object that does not
exist.

Since I don't know how where the model is headed on this particular
point, I thought it best to just put a FIXME placeholder in those
rhtml files to allow mongrel to serve them.

I should have split this out into a different patch, I guess, since it
doesn't have anything to do with the uniqueness constraint issue, no?

Hope this answers your question.

Thanks
Steve