Mohammed Morsi
2008-Dec-09 16:00 UTC
[Ovirt-devel] [PATCH server] missing ovirt wui validations
this patch is now syntactically correct and will run when invoking
db:migrate or when running the tests. several tests now fail though
and need to be debugged / possibly changed
Attached is additional validations, added to the rails model
and controller layers verifying:
bondings: arp related fields
boot_types: label uniqueness and proto inclusion
cpus: various fields are set and min values
help_sections: presence and uniqueness of fields
hosts: presence and format of various fields
ip_addresses: fk existance / integrity
nics: mac address format
permissions: presence / inclusion of various fields
pools: fields requied for tree present
quotas: minimum values
smart_pool_tags: tagged_id / type fk present
storage_pools: type, state field inclusion, min values
storage_volumes: various fields, subclasses w/ various fields
tasks: various fields present and consistensy maintained
usages: label, other fields present, unique
vms: uuid of correct format, min values, various fields present
can only add / move host and storage volumes and pools to / from pools
when you have sufficient permissions and target entities don't have
associated vms
can only destroy storage volumes / pools if there are no attached vms
---
src/app/controllers/hardware_controller.rb | 28 +++++++++++++++++++++-
src/app/controllers/storage_controller.rb | 7 +++++
src/app/models/bonding.rb | 11 +++++++++
src/app/models/boot_type.rb | 5 ++++
src/app/models/cpu.rb | 23 +++++++++++++++++++
src/app/models/help_section.rb | 34 ++++++++++++++++++++++++++++
src/app/models/host.rb | 21 +++++++++++++++++
src/app/models/ip_address.rb | 4 +++
src/app/models/iscsi_storage_volume.rb | 2 +
src/app/models/lvm_storage_volume.rb | 6 +++++
src/app/models/nfs_storage_pool.rb | 3 ++
src/app/models/nic.rb | 11 +++++++++
src/app/models/permission.rb | 7 +++++
src/app/models/pool.rb | 7 +++++
src/app/models/quota.rb | 22 ++++++++++++++++++
src/app/models/smart_pool_tag.rb | 5 ++++
src/app/models/storage_pool.rb | 17 ++++++++++++++
src/app/models/storage_volume.rb | 21 +++++++++++++++++
src/app/models/task.rb | 15 ++++++++++++
src/app/models/usage.rb | 6 +++++
src/app/models/vm.rb | 34 ++++++++++++++++++++++++++-
21 files changed, 285 insertions(+), 4 deletions(-)
diff --git a/src/app/controllers/hardware_controller.rb
b/src/app/controllers/hardware_controller.rb
index 4dda736..cf45c49 100644
--- a/src/app/controllers/hardware_controller.rb
+++ b/src/app/controllers/hardware_controller.rb
@@ -303,15 +303,39 @@ class HardwareController < PoolController
resource_ids_str = params[:resource_ids]
resource_ids = resource_ids_str.split(",").collect {|x| x.to_i}
+ # if user doesn't have modify permission on both source and destination
+ unless @pool.can_modify(@user) and
Pool.find(params[:target_pool_id]).can_modify(@user)
+ render :json => { :success => false,
+ :alert => "Cannot #{item_action.to_s}
#{item_class.table_name.humanize} without admin permissions on both pools"
}
+ return
+ end
+
+ # relay error message if movable check fails for any resource
+ success = true
+ failed_resources = ""
+ resource_ids.each {|x|
+ unless item_class.find(x).movable?
+ success = false
+ failed_resources += x.to_s + " "
+ end
+ }
+ resource_ids.delete_if { |x| ! item_class.find(x).movable? }
+
begin
@pool.transaction do
@pool.send(item_method, resource_ids, target_pool_id)
end
+ rescue
+ success = false
+ end
+
+ if success
render :json => { :success => true,
:alert => "#{item_action.to_s}
#{item_class.table_name.humanize} successful." }
- rescue
+ else
render :json => { :success => false,
- :alert => "#{item_action.to_s}
#{item_class.table_name.humanize} failed." }
+ :alert => "#{item_action.to_s}
#{item_class.table_name.humanize} failed" +
+ (failed_resources == "" ? "." : "
for " + failed_resources) }
end
end
diff --git a/src/app/controllers/storage_controller.rb
b/src/app/controllers/storage_controller.rb
index 148d1be..47e9e14 100644
--- a/src/app/controllers/storage_controller.rb
+++ b/src/app/controllers/storage_controller.rb
@@ -310,6 +310,13 @@ class StorageController < ApplicationController
end
def destroy
+ unless @storage_pool.movable?
+ format.json { render :json => { :object => "storage_pool",
+ :success => false,
+ :alert => "Cannot delete storage with associated vms" } }
+ return
+ end
+
pool = @storage_pool.hardware_pool
if @storage_pool.destroy
alert="Storage Pool was successfully deleted."
diff --git a/src/app/models/bonding.rb b/src/app/models/bonding.rb
index c9af38c..fabdb67 100644
--- a/src/app/models/bonding.rb
+++ b/src/app/models/bonding.rb
@@ -30,6 +30,8 @@
# interface. They can be ignored if not used.
#
class Bonding < ActiveRecord::Base
+
+
belongs_to :host
belongs_to :bonding_type
belongs_to :vlan
@@ -58,6 +60,15 @@ class Bonding < ActiveRecord::Base
validates_presence_of :vlan_id,
:message => 'A vlan must be specified.'
+ # verify arp ping address to be ipv4 if set
+ validates_format_of :arp_ping_address,
+ :with => %r{^(\d{1,3}\.){3}\d{1,3}$},
+ :unless => Proc.new { |bonding| bonding.arp_ping_address.nil? }
+
+ validates_numericality_of :arp_interval,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new { |bonding| bonding.arp_interval.nil? }
+
protected
def validate
if vlan.boot_type.proto == 'static' and ip_addresses.size == 0
diff --git a/src/app/models/boot_type.rb b/src/app/models/boot_type.rb
index 8ffbe67..6cfdb04 100644
--- a/src/app/models/boot_type.rb
+++ b/src/app/models/boot_type.rb
@@ -18,4 +18,9 @@
# also available at http://www.gnu.org/copyleft/gpl.html.
class BootType < ActiveRecord::Base
+ validates_presence_of :label
+ validates_uniqueness_of :label,
+ :message => 'Label must be unique'
+ validates_inclusion_of :proto,
+ :in => %w( static dhcp bootp )
end
diff --git a/src/app/models/cpu.rb b/src/app/models/cpu.rb
index 1a7d3cf..714a8ed 100644
--- a/src/app/models/cpu.rb
+++ b/src/app/models/cpu.rb
@@ -21,4 +21,27 @@
#
class Cpu < ActiveRecord::Base
belongs_to :host
+
+ validates_presence_of :host_id,
+ :message => 'A host must be specified.'
+
+ validates_numericality_of :cpu_number,
+ :greater_than_or_equal_to => 0
+
+ validates_numericality_of :core_number,
+ :greater_than_or_equal_to => 0
+
+ validates_numericality_of :number_of_cores,
+ :greater_than_or_equal_to => 1
+
+ validates_numericality_of :cpuid_level,
+ :greater_than_or_equal_to => 0
+
+ validates_numericality_of :speed,
+ :greater_than => 0
+ # also verify speed in MHz ?
+
+ validates_presence_of :vendor
+ validates_presence_of :model
+ validates_presence_of :family
end
diff --git a/src/app/models/help_section.rb b/src/app/models/help_section.rb
index a891383..1de1e5a 100644
--- a/src/app/models/help_section.rb
+++ b/src/app/models/help_section.rb
@@ -1,2 +1,36 @@
+# Copyright (C) 2008 Red Hat, Inc.
+# Written by Mohammed Morsi <mmorsi 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.
+#
+# +HelpSection+ defines a section of the web help document to be
+# available for a specific controller / action
+#
class HelpSection < ActiveRecord::Base
+
+ validates_uniqueness_of :action,
+ :scope => :controller
+ :message => 'Controller / Action must be unique'
+
+ validates_presence_of :controller,
+ :message => 'A controller must be specified.'
+
+ validates_presence_of :action,
+ :message => 'An action must be specified.'
+
+ validates_presence_of :section,
+ :message => 'A section must be specified.'
end
diff --git a/src/app/models/host.rb b/src/app/models/host.rb
index 640782d..ef495ff 100644
--- a/src/app/models/host.rb
+++ b/src/app/models/host.rb
@@ -51,6 +51,17 @@ class Host < ActiveRecord::Base
[ :search_users, 'U',
"search_users" ] ],
:eager_load => :smart_pools
+ validates_presence_of :hardware_pool_id,
+ :message => 'A hardware pool id must be specified.'
+
+ validates_presence_of :uuid,
+ :message => 'A uuid must be specified.'
+
+ validates_presence_of :hostname,
+ :message => 'A hostname must be specified.'
+
+ validates_presence_of :arch,
+ :message => 'An arch must be specified.'
KVM_HYPERVISOR_TYPE = "KVM"
HYPERVISOR_TYPES = [KVM_HYPERVISOR_TYPE]
@@ -58,6 +69,12 @@ class Host < ActiveRecord::Base
STATE_AVAILABLE = "available"
STATES = [STATE_UNAVAILABLE, STATE_AVAILABLE]
+ validates_inclusion_of :hypervisor_type,
+ :in => HYPERVISOR_TYPES
+
+ validates_inclusion_of :state,
+ :in => STATES + Task::COMPLETED_STATES + TASK::WORKING_STATES
+
def memory_in_mb
kb_to_mb(memory)
end
@@ -99,4 +116,8 @@ class Host < ActiveRecord::Base
hardware_pool.search_users
end
+ def movable?
+ return vms.size == 0
+ end
+
end
diff --git a/src/app/models/ip_address.rb b/src/app/models/ip_address.rb
index 5d2e6af..3f246b1 100644
--- a/src/app/models/ip_address.rb
+++ b/src/app/models/ip_address.rb
@@ -24,4 +24,8 @@ class IpAddress < ActiveRecord::Base
belongs_to :network
belongs_to :nic
belongs_to :bonding
+
+ def validate
+ errors.add("id", "ip must be associated with foreign
entity") if network_id.nil? and nic_id.nil? and bonding_id.nil?
+ end
end
diff --git a/src/app/models/iscsi_storage_volume.rb
b/src/app/models/iscsi_storage_volume.rb
index 48edbd8..fe2cbf5 100644
--- a/src/app/models/iscsi_storage_volume.rb
+++ b/src/app/models/iscsi_storage_volume.rb
@@ -31,4 +31,6 @@ class IscsiStorageVolume < StorageVolume
return true
end
+ validates_presence_of :lun
+
end
diff --git a/src/app/models/lvm_storage_volume.rb
b/src/app/models/lvm_storage_volume.rb
index 8eb1f0e..38949ce 100644
--- a/src/app/models/lvm_storage_volume.rb
+++ b/src/app/models/lvm_storage_volume.rb
@@ -29,4 +29,10 @@ class LvmStorageVolume < StorageVolume
def volume_create_params
return lv_name, size, lv_owner_perms, lv_group_perms, lv_mode_perms
end
+
+ validates_presence_of :lv_name
+ validates_presence_of :lv_owner_perms
+ validates_presence_of :lv_group_perms
+ validates_presence_of :lv_mode_perms
+
end
diff --git a/src/app/models/nfs_storage_pool.rb
b/src/app/models/nfs_storage_pool.rb
index 398e3f9..da0d4b4 100644
--- a/src/app/models/nfs_storage_pool.rb
+++ b/src/app/models/nfs_storage_pool.rb
@@ -29,4 +29,7 @@ class NfsStoragePool < StoragePool
def user_subdividable
true
end
+
+ validates_presence_of :filename
+
end
diff --git a/src/app/models/nic.rb b/src/app/models/nic.rb
index 5649763..aa9c740 100644
--- a/src/app/models/nic.rb
+++ b/src/app/models/nic.rb
@@ -24,12 +24,23 @@ class Nic < ActiveRecord::Base
has_and_belongs_to_many :bondings, :join_table => 'bondings_nics'
+ validates_presence_of :mac,
+ :message => 'A mac must be specified.'
+
+ validates_format_of :mac,
+ :with => %r{^([0-9a-f]{2}([:-]|$)){6}$}
+
validates_presence_of :host_id,
:message => 'A host must be specified.'
validates_presence_of :physical_network_id,
:message => 'A network must be specified.'
+ validates_numericality_of :bandwidth,
+ :greater_than_or_equal_to => 0
+
+ # validate 'bridge' or 'usage_type' attribute ?
+
protected
def validate
if physical_network.boot_type.proto == 'static' and
ip_addresses.size == 0
diff --git a/src/app/models/permission.rb b/src/app/models/permission.rb
index ece3da5..b3929ad 100644
--- a/src/app/models/permission.rb
+++ b/src/app/models/permission.rb
@@ -24,9 +24,12 @@ class Permission < ActiveRecord::Base
has_many :child_permissions, :dependent => :destroy,
:class_name => "Permission", :foreign_key =>
"inherited_from_id"
+ validates_presence_of :pool_id
+ validates_presence_of :uid
validates_uniqueness_of :uid, :scope => [:pool_id, :inherited_from_id]
+
ROLE_SUPER_ADMIN = "Super Admin"
ROLE_ADMIN = "Administrator"
ROLE_USER = "User"
@@ -44,6 +47,10 @@ class Permission < ActiveRecord::Base
ROLE_USER => [PRIV_VIEW, PRIV_VM_CONTROL],
ROLE_MONITOR => [PRIV_VIEW]}
+
+ validates_inclusion_of :user_role,
+ :in => ROLES.keys
+
def self.invert_roles
return_hash = {}
ROLES.each do |role, privs|
diff --git a/src/app/models/pool.rb b/src/app/models/pool.rb
index 7034e79..5cfc947 100644
--- a/src/app/models/pool.rb
+++ b/src/app/models/pool.rb
@@ -52,6 +52,13 @@ class Pool < ActiveRecord::Base
validates_presence_of :name
validates_uniqueness_of :name, :scope => :parent_id
+ validates_presence_of :parent_id
+ validates_presence_of :lft
+ validates_presence_of :rgt
+
+ validates_inclusion_of :type,
+ :in => %w( DirectoryPool HardwarePool VmResourcePool SmartPool )
+
# overloading this method such that we can use permissions.admins to get all
the admins for an object
has_many :permissions, :dependent => :destroy, :order => "id
ASC" do
def super_admins
diff --git a/src/app/models/quota.rb b/src/app/models/quota.rb
index fc52177..2a9e0f0 100644
--- a/src/app/models/quota.rb
+++ b/src/app/models/quota.rb
@@ -22,6 +22,28 @@ require 'util/ovirt'
class Quota < ActiveRecord::Base
belongs_to :pool
+ validates_presence_of :pool_id
+
+
+ validates_numericality_of :total_vcpus,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new{ |quota| quota.total_vcpus.nil? }
+
+ validates_numericality_of :total_vmemory,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new{ |quota| quota.total_vmemory.nil? }
+
+ validates_numericality_of :total_vnics,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new{ |quota| quota.total_vnics.nil? }
+
+ validates_numericality_of :total_storage,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new{ |quota| quota.total_storage.nil? }
+
+ validates_numericality_of :total_vms,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new{ |quota| quota.total_vms.nil? }
def total_vmemory_in_mb
kb_to_mb(total_vmemory)
diff --git a/src/app/models/smart_pool_tag.rb b/src/app/models/smart_pool_tag.rb
index e135ddf..00bb5a7 100644
--- a/src/app/models/smart_pool_tag.rb
+++ b/src/app/models/smart_pool_tag.rb
@@ -31,6 +31,11 @@ class SmartPoolTag < ActiveRecord::Base
validates_uniqueness_of :smart_pool_id, :scope => [:tagged_id,
:tagged_type]
+ validates_presence_of :tagged_id
+
+ validates_inclusion_of :tagged_type,
+ :in => %w( Pool StoragePool Host Vm )
+
def tagged_type=(sType)
super(sType.to_s.classify.constantize.base_class.to_s)
end
diff --git a/src/app/models/storage_pool.rb b/src/app/models/storage_pool.rb
index bab7031..f9abb55 100644
--- a/src/app/models/storage_pool.rb
+++ b/src/app/models/storage_pool.rb
@@ -39,6 +39,13 @@ class StoragePool < ActiveRecord::Base
validates_presence_of :hardware_pool_id
+ validates_inclusion_of :type,
+ :in => %w( IscsiStoragePool LvmStoragePool NfsStoragePool )
+
+
+ validates_numericality_of :capacity,
+ :greater_than_or_equal_to => 0
+
acts_as_xapian :texts => [ :ip_addr, :target, :export_path, :type ],
:terms => [ [ :search_users, 'U',
"search_users" ] ],
:eager_load => :smart_pools
@@ -54,6 +61,9 @@ class StoragePool < ActiveRecord::Base
STATE_PENDING_DELETION = "pending_deletion"
STATE_AVAILABLE = "available"
+ validates_inclusion_of :state,
+ :in => [ STATE_PENDING_SETUP, STATE_PENDING_DELETION, STATE_AVAILABLE]
+
def self.factory(type, params = {})
params[:state] = STATE_PENDING_SETUP unless params[:state]
case type
@@ -121,4 +131,11 @@ class StoragePool < ActiveRecord::Base
end
return_hash
end
+
+ def movable?
+ storage_volumes.each{ |x|
+ return false unless x.movable?
+ }
+ return true
+ end
end
diff --git a/src/app/models/storage_volume.rb b/src/app/models/storage_volume.rb
index 39b72d5..385c126 100644
--- a/src/app/models/storage_volume.rb
+++ b/src/app/models/storage_volume.rb
@@ -32,10 +32,23 @@ class StorageVolume < ActiveRecord::Base
end
end
+ validates_presence_of :storage_pool_id
+
+
+ validates_numericality_of :size,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new { |storage_volume| storage_volume.nil? }
+
+ validates_inclusion_of :type,
+ :in => %w( IscsiStorageVolume LvmStorageVolume NfsStorageVolume )
+
STATE_PENDING_SETUP = "pending_setup"
STATE_PENDING_DELETION = "pending_deletion"
STATE_AVAILABLE = "available"
+ validates_inclusion_of :state,
+ :in => [ STATE_PENDING_SETUP, STATE_PENDING_DELETION, STATE_AVAILABLE]
+
def self.factory(type, params = {})
params[:state] = STATE_PENDING_SETUP unless params[:state]
case type
@@ -131,4 +144,12 @@ class StorageVolume < ActiveRecord::Base
return_hash
end
+ def movable?
+ if vms.size > 0 or
+ (not lvm_storage_pool.nil? and not lvm_stoage_pool.movable?)
+ return false
+ end
+ return true
+ end
+
end
diff --git a/src/app/models/task.rb b/src/app/models/task.rb
index f231c18..e6b9079 100644
--- a/src/app/models/task.rb
+++ b/src/app/models/task.rb
@@ -45,6 +45,16 @@ class Task < ActiveRecord::Base
COMPLETED_STATES = [STATE_FINISHED, STATE_FAILED, STATE_CANCELED]
WORKING_STATES = [STATE_QUEUED, STATE_RUNNING, STATE_PAUSED]
+ validates_inclusion_of :type,
+ :in => %w( HostTask StorageTask StorageVolumeTask VmTask )
+
+ validates_inclusion_of :state,
+ :in => COMPLETED_STATES + WORKING_STATES
+
+ # FIXME validate action depending on type / subclass
+ # validate task_target_id, task_type_id, arg, message
+ # depending on subclass, action, state
+
def cancel
self[:state] = STATE_CANCELED
save!
@@ -71,4 +81,9 @@ class Task < ActiveRecord::Base
""
end
+ def validate
+ errors.add("time_ended", "Tasks ends before its
started") unless time_ended > time_started
+ errors.add("time_started", "Tasks starts before its
created") unless time_started > time_created
+ end
+
end
diff --git a/src/app/models/usage.rb b/src/app/models/usage.rb
index 353e8f4..59b0e48 100644
--- a/src/app/models/usage.rb
+++ b/src/app/models/usage.rb
@@ -18,4 +18,10 @@
class Usage < ActiveRecord::Base
has_and_belongs_to_many :networks, :join_table =>
'networks_usages'
+
+ validates_presence_of :label
+ validates_presence_of :usage
+
+ validates_uniqueness_of :usage
+
end
diff --git a/src/app/models/vm.rb b/src/app/models/vm.rb
index 227f343..4d66c63 100644
--- a/src/app/models/vm.rb
+++ b/src/app/models/vm.rb
@@ -34,7 +34,32 @@ class Vm < ActiveRecord::Base
validates_presence_of :uuid, :description, :num_vcpus_allocated,
:boot_device, :memory_allocated_in_mb,
- :memory_allocated, :vnic_mac_addr
+ :memory_allocated, :vnic_mac_addr,
+ :vm_resource_pool_id
+
+ validates_format_of :uuid,
+ :with =>
%r([a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12})
+
+ validates_numericality_of :needs_restart,
+ :greater_than_or_equal_to => 0,
+ :less_than_or_equal_to => 1,
+ :unless => Proc.new{ |vm| vm.needs_restart.nil? }
+
+ validates_numericality_of :memory_used,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new{ |vm| vm.memory_used.nil? }
+
+ validates_numericality_of :vnc_port,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new{ |vm| vm.vnc_port.nil? }
+
+ validates_numericality_of :num_vcpus_allocated,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new{ |vm| vm.num_vcpus_allocated.nil? }
+
+ validates_numericality_of :memory_allocated,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new{ |vm| vm.memory_allocated.nil? }
acts_as_xapian :texts => [ :uuid, :description, :vnic_mac_addr, :state ],
:terms => [ [ :search_users, 'U',
"search_users" ] ],
@@ -117,9 +142,14 @@ class Vm < ActiveRecord::Base
STATE_SAVED => STATE_SAVED,
STATE_RESTORING => STATE_RUNNING,
STATE_MIGRATING => STATE_RUNNING,
- STATE_CREATE_FAILED => STATE_CREATE_FAILED}
+ STATE_CREATE_FAILED => STATE_CREATE_FAILED,
+ STATE_INVALID => STATE_INVALID}
TASK_STATE_TRANSITIONS = []
+ validates_inclusion_of :state,
+ :in => EFFECTIVE_STATE.keys
+
+
def get_hardware_pool
pool = vm_resource_pool
pool = pool.get_hardware_pool if pool
--
1.6.0.4
Jason Guiditta
2008-Dec-10 03:49 UTC
[Ovirt-devel] [PATCH server] missing ovirt wui validations
On Tue, 2008-12-09 at 11:00 -0500, Mohammed Morsi wrote: Mo, I'll review this tomorrow since I think Scott is off or at least limited availability. I am assuming this is still just looking for feedback as your last one was, since I cannot ack if it breaks tests (and depending what the new assertions are, really should be _adding_ tests, if anything). -j
Jason Guiditta
2008-Dec-10 22:25 UTC
[Ovirt-devel] [PATCH server] missing ovirt wui validations
Coming along nicely, some comments/corrections inline.
There are some whitespace errors with the patch:
Applying missing ovirt wui validations
.dotest/patch:44: trailing whitespace.
unless item_class.find(x).movable?
.dotest/patch:81: trailing whitespace.
:success => false,
.dotest/patch:474: trailing whitespace.
validates_numericality_of :size,
.dotest/patch:496: trailing whitespace.
if vms.size > 0 or
.dotest/patch:498: trailing whitespace.
return false
warning: 5 lines add whitespace errors.
On Tue, 2008-12-09 at 11:00 -0500, Mohammed Morsi wrote:> this patch is now syntactically correct and will run when invoking
> db:migrate or when running the tests. several tests now fail though
> and need to be debugged / possibly changed
>
> Attached is additional validations, added to the rails model
> and controller layers verifying:
> bondings: arp related fields
> boot_types: label uniqueness and proto inclusion
> cpus: various fields are set and min values
> help_sections: presence and uniqueness of fields
> hosts: presence and format of various fields
> ip_addresses: fk existance / integrity
> nics: mac address format
> permissions: presence / inclusion of various fields
> pools: fields requied for tree present
> quotas: minimum values
> smart_pool_tags: tagged_id / type fk present
> storage_pools: type, state field inclusion, min values
> storage_volumes: various fields, subclasses w/ various fields
> tasks: various fields present and consistensy maintained
> usages: label, other fields present, unique
> vms: uuid of correct format, min values, various fields present
>
> can only add / move host and storage volumes and pools to / from pools
> when you have sufficient permissions and target entities don't have
> associated vms
>
> can only destroy storage volumes / pools if there are no attached vms
> ---
> src/app/controllers/hardware_controller.rb | 28 +++++++++++++++++++++-
> src/app/controllers/storage_controller.rb | 7 +++++
> src/app/models/bonding.rb | 11 +++++++++
> src/app/models/boot_type.rb | 5 ++++
> src/app/models/cpu.rb | 23 +++++++++++++++++++
> src/app/models/help_section.rb | 34
++++++++++++++++++++++++++++
> src/app/models/host.rb | 21 +++++++++++++++++
> src/app/models/ip_address.rb | 4 +++
> src/app/models/iscsi_storage_volume.rb | 2 +
> src/app/models/lvm_storage_volume.rb | 6 +++++
> src/app/models/nfs_storage_pool.rb | 3 ++
> src/app/models/nic.rb | 11 +++++++++
> src/app/models/permission.rb | 7 +++++
> src/app/models/pool.rb | 7 +++++
> src/app/models/quota.rb | 22 ++++++++++++++++++
> src/app/models/smart_pool_tag.rb | 5 ++++
> src/app/models/storage_pool.rb | 17 ++++++++++++++
> src/app/models/storage_volume.rb | 21 +++++++++++++++++
> src/app/models/task.rb | 15 ++++++++++++
> src/app/models/usage.rb | 6 +++++
> src/app/models/vm.rb | 34
++++++++++++++++++++++++++-
> 21 files changed, 285 insertions(+), 4 deletions(-)
>
> diff --git a/src/app/controllers/hardware_controller.rb
b/src/app/controllers/hardware_controller.rb
> index 4dda736..cf45c49 100644
> --- a/src/app/controllers/hardware_controller.rb
> +++ b/src/app/controllers/hardware_controller.rb
> @@ -303,15 +303,39 @@ class HardwareController < PoolController
> resource_ids_str = params[:resource_ids]
> resource_ids = resource_ids_str.split(",").collect {|x|
x.to_i}
>
> + # if user doesn't have modify permission on both source and
destination
> + unless @pool.can_modify(@user) and
Pool.find(params[:target_pool_id]).can_modify(@user)
> + render :json => { :success => false,
> + :alert => "Cannot #{item_action.to_s}
#{item_class.table_name.humanize} without admin permissions on both pools"
}
> + return
> + end
> +
> + # relay error message if movable check fails for any resource
> + success = true
> + failed_resources = ""
> + resource_ids.each {|x|
> + unless item_class.find(x).movable?
> + success = false
> + failed_resources += x.to_s + " "
> + end
> + }
> + resource_ids.delete_if { |x| ! item_class.find(x).movable? }
> +
> begin
> @pool.transaction do
> @pool.send(item_method, resource_ids, target_pool_id)
> end
> + rescue
> + success = false
> + end
> +
> + if success
> render :json => { :success => true,
> :alert => "#{item_action.to_s}
#{item_class.table_name.humanize} successful." }
> - rescue
> + else
> render :json => { :success => false,
> - :alert => "#{item_action.to_s}
#{item_class.table_name.humanize} failed." }
> + :alert => "#{item_action.to_s}
#{item_class.table_name.humanize} failed" +
> + (failed_resources == "" ? "." :
" for " + failed_resources) }
> end
> end
Very nice, I like this much better than the first rev.
>
> diff --git a/src/app/controllers/storage_controller.rb
b/src/app/controllers/storage_controller.rb
> index 148d1be..47e9e14 100644
> --- a/src/app/controllers/storage_controller.rb
> +++ b/src/app/controllers/storage_controller.rb
> @@ -310,6 +310,13 @@ class StorageController < ApplicationController
> end
>
> def destroy
> + unless @storage_pool.movable?
> + format.json { render :json => { :object =>
"storage_pool",
> + :success => false,
> + :alert => "Cannot delete storage with associated vms"
} }
> + return
> + end
> +
> pool = @storage_pool.hardware_pool
> if @storage_pool.destroy
> alert="Storage Pool was successfully deleted."
> diff --git a/src/app/models/bonding.rb b/src/app/models/bonding.rb
> index c9af38c..fabdb67 100644
> --- a/src/app/models/bonding.rb
> +++ b/src/app/models/bonding.rb
> @@ -30,6 +30,8 @@
> # interface. They can be ignored if not used.
> #
> class Bonding < ActiveRecord::Base
> +
> +
> belongs_to :host
> belongs_to :bonding_type
> belongs_to :vlan
> @@ -58,6 +60,15 @@ class Bonding < ActiveRecord::Base
> validates_presence_of :vlan_id,
> :message => 'A vlan must be specified.'
>
> + # verify arp ping address to be ipv4 if set
> + validates_format_of :arp_ping_address,
> + :with => %r{^(\d{1,3}\.){3}\d{1,3}$},
> + :unless => Proc.new { |bonding| bonding.arp_ping_address.nil? }
> +
> + validates_numericality_of :arp_interval,
> + :greater_than_or_equal_to => 0,
> + :unless => Proc.new { |bonding| bonding.arp_interval.nil? }
> +
> protected
> def validate
> if vlan.boot_type.proto == 'static' and ip_addresses.size == 0
> diff --git a/src/app/models/boot_type.rb b/src/app/models/boot_type.rb
> index 8ffbe67..6cfdb04 100644
> --- a/src/app/models/boot_type.rb
> +++ b/src/app/models/boot_type.rb
> @@ -18,4 +18,9 @@
> # also available at http://www.gnu.org/copyleft/gpl.html.
>
> class BootType < ActiveRecord::Base
> + validates_presence_of :label
> + validates_uniqueness_of :label,
> + :message => 'Label must be unique'
> + validates_inclusion_of :proto,
> + :in => %w( static dhcp bootp )
> end
> diff --git a/src/app/models/cpu.rb b/src/app/models/cpu.rb
> index 1a7d3cf..714a8ed 100644
> --- a/src/app/models/cpu.rb
> +++ b/src/app/models/cpu.rb
> @@ -21,4 +21,27 @@
> #
> class Cpu < ActiveRecord::Base
> belongs_to :host
> +
> + validates_presence_of :host_id,
> + :message => 'A host must be specified.'
> +
> + validates_numericality_of :cpu_number,
> + :greater_than_or_equal_to => 0
> +
> + validates_numericality_of :core_number,
> + :greater_than_or_equal_to => 0
> +
> + validates_numericality_of :number_of_cores,
> + :greater_than_or_equal_to => 1
> +
> + validates_numericality_of :cpuid_level,
> + :greater_than_or_equal_to => 0
> +
> + validates_numericality_of :speed,
> + :greater_than => 0
> + # also verify speed in MHz ?
> +
> + validates_presence_of :vendor
> + validates_presence_of :model
> + validates_presence_of :family
> end
> diff --git a/src/app/models/help_section.rb
b/src/app/models/help_section.rb
> index a891383..1de1e5a 100644
> --- a/src/app/models/help_section.rb
> +++ b/src/app/models/help_section.rb
> @@ -1,2 +1,36 @@
> +# Copyright (C) 2008 Red Hat, Inc.
> +# Written by Mohammed Morsi <mmorsi 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.
> +#
> +# +HelpSection+ defines a section of the web help document to be
> +# available for a specific controller / action
> +#
> class HelpSection < ActiveRecord::Base
> +
> + validates_uniqueness_of :action,
> + :scope => :controller
Missing ',' on above line> + :message => 'Controller / Action must be unique'
> +
> + validates_presence_of :controller,
> + :message => 'A controller must be specified.'
> +
> + validates_presence_of :action,
> + :message => 'An action must be specified.'
> +
> + validates_presence_of :section,
> + :message => 'A section must be specified.'
> end
> diff --git a/src/app/models/host.rb b/src/app/models/host.rb
> index 640782d..ef495ff 100644
> --- a/src/app/models/host.rb
> +++ b/src/app/models/host.rb
> @@ -51,6 +51,17 @@ class Host < ActiveRecord::Base
> [ :search_users, 'U',
"search_users" ] ],
> :eager_load => :smart_pools
>
> + validates_presence_of :hardware_pool_id,
> + :message => 'A hardware pool id must be specified.'
> +
> + validates_presence_of :uuid,
> + :message => 'A uuid must be specified.'
> +
> + validates_presence_of :hostname,
> + :message => 'A hostname must be specified.'
> +
> + validates_presence_of :arch,
> + :message => 'An arch must be specified.'
>
> KVM_HYPERVISOR_TYPE = "KVM"
> HYPERVISOR_TYPES = [KVM_HYPERVISOR_TYPE]
> @@ -58,6 +69,12 @@ class Host < ActiveRecord::Base
> STATE_AVAILABLE = "available"
> STATES = [STATE_UNAVAILABLE, STATE_AVAILABLE]
>
> + validates_inclusion_of :hypervisor_type,
> + :in => HYPERVISOR_TYPES
> +
> + validates_inclusion_of :state,
> + :in => STATES + Task::COMPLETED_STATES + TASK::WORKING_STATES
2nd 'Task' should not be all caps> +
> def memory_in_mb
> kb_to_mb(memory)
> end
> @@ -99,4 +116,8 @@ class Host < ActiveRecord::Base
> hardware_pool.search_users
> end
>
> + def movable?
> + return vms.size == 0
> + end
> +
> end
> diff --git a/src/app/models/ip_address.rb b/src/app/models/ip_address.rb
> index 5d2e6af..3f246b1 100644
> --- a/src/app/models/ip_address.rb
> +++ b/src/app/models/ip_address.rb
> @@ -24,4 +24,8 @@ class IpAddress < ActiveRecord::Base
> belongs_to :network
> belongs_to :nic
> belongs_to :bonding
> +
> + def validate
> + errors.add("id", "ip must be associated with foreign
entity") if network_id.nil? and nic_id.nil? and bonding_id.nil?
> + end
> end
> diff --git a/src/app/models/iscsi_storage_volume.rb
b/src/app/models/iscsi_storage_volume.rb
> index 48edbd8..fe2cbf5 100644
> --- a/src/app/models/iscsi_storage_volume.rb
> +++ b/src/app/models/iscsi_storage_volume.rb
> @@ -31,4 +31,6 @@ class IscsiStorageVolume < StorageVolume
> return true
> end
>
> + validates_presence_of :lun
> +
> end
> diff --git a/src/app/models/lvm_storage_volume.rb
b/src/app/models/lvm_storage_volume.rb
> index 8eb1f0e..38949ce 100644
> --- a/src/app/models/lvm_storage_volume.rb
> +++ b/src/app/models/lvm_storage_volume.rb
> @@ -29,4 +29,10 @@ class LvmStorageVolume < StorageVolume
> def volume_create_params
> return lv_name, size, lv_owner_perms, lv_group_perms, lv_mode_perms
> end
> +
> + validates_presence_of :lv_name
> + validates_presence_of :lv_owner_perms
> + validates_presence_of :lv_group_perms
> + validates_presence_of :lv_mode_perms
> +
> end
> diff --git a/src/app/models/nfs_storage_pool.rb
b/src/app/models/nfs_storage_pool.rb
> index 398e3f9..da0d4b4 100644
> --- a/src/app/models/nfs_storage_pool.rb
> +++ b/src/app/models/nfs_storage_pool.rb
> @@ -29,4 +29,7 @@ class NfsStoragePool < StoragePool
> def user_subdividable
> true
> end
> +
> + validates_presence_of :filename
> +
> end
> diff --git a/src/app/models/nic.rb b/src/app/models/nic.rb
> index 5649763..aa9c740 100644
> --- a/src/app/models/nic.rb
> +++ b/src/app/models/nic.rb
> @@ -24,12 +24,23 @@ class Nic < ActiveRecord::Base
>
> has_and_belongs_to_many :bondings, :join_table =>
'bondings_nics'
>
> + validates_presence_of :mac,
> + :message => 'A mac must be specified.'
> +
> + validates_format_of :mac,
> + :with => %r{^([0-9a-f]{2}([:-]|$)){6}$}
> +
> validates_presence_of :host_id,
> :message => 'A host must be specified.'
>
> validates_presence_of :physical_network_id,
> :message => 'A network must be specified.'
>
> + validates_numericality_of :bandwidth,
> + :greater_than_or_equal_to => 0
> +
> + # validate 'bridge' or 'usage_type' attribute ?
> +
> protected
> def validate
> if physical_network.boot_type.proto == 'static' and
ip_addresses.size == 0
> diff --git a/src/app/models/permission.rb b/src/app/models/permission.rb
> index ece3da5..b3929ad 100644
> --- a/src/app/models/permission.rb
> +++ b/src/app/models/permission.rb
> @@ -24,9 +24,12 @@ class Permission < ActiveRecord::Base
> has_many :child_permissions, :dependent => :destroy,
> :class_name => "Permission", :foreign_key =>
"inherited_from_id"
>
> + validates_presence_of :pool_id
>
> + validates_presence_of :uid
> validates_uniqueness_of :uid, :scope => [:pool_id,
:inherited_from_id]
>
> +
> ROLE_SUPER_ADMIN = "Super Admin"
> ROLE_ADMIN = "Administrator"
> ROLE_USER = "User"
> @@ -44,6 +47,10 @@ class Permission < ActiveRecord::Base
> ROLE_USER => [PRIV_VIEW, PRIV_VM_CONTROL],
> ROLE_MONITOR => [PRIV_VIEW]}
>
> +
> + validates_inclusion_of :user_role,
> + :in => ROLES.keys
> +
> def self.invert_roles
> return_hash = {}
> ROLES.each do |role, privs|
> diff --git a/src/app/models/pool.rb b/src/app/models/pool.rb
> index 7034e79..5cfc947 100644
> --- a/src/app/models/pool.rb
> +++ b/src/app/models/pool.rb
> @@ -52,6 +52,13 @@ class Pool < ActiveRecord::Base
> validates_presence_of :name
> validates_uniqueness_of :name, :scope => :parent_id
>
> + validates_presence_of :parent_id
> + validates_presence_of :lft
> + validates_presence_of :rgt
> +
> + validates_inclusion_of :type,
> + :in => %w( DirectoryPool HardwarePool VmResourcePool SmartPool )
> +
> # overloading this method such that we can use permissions.admins to get
all the admins for an object
> has_many :permissions, :dependent => :destroy, :order => "id
ASC" do
> def super_admins
> diff --git a/src/app/models/quota.rb b/src/app/models/quota.rb
> index fc52177..2a9e0f0 100644
> --- a/src/app/models/quota.rb
> +++ b/src/app/models/quota.rb
> @@ -22,6 +22,28 @@ require 'util/ovirt'
> class Quota < ActiveRecord::Base
> belongs_to :pool
>
> + validates_presence_of :pool_id
> +
> +
> + validates_numericality_of :total_vcpus,
> + :greater_than_or_equal_to => 0,
> + :unless => Proc.new{ |quota| quota.total_vcpus.nil? }
> +
> + validates_numericality_of :total_vmemory,
> + :greater_than_or_equal_to => 0,
> + :unless => Proc.new{ |quota| quota.total_vmemory.nil? }
> +
> + validates_numericality_of :total_vnics,
> + :greater_than_or_equal_to => 0,
> + :unless => Proc.new{ |quota| quota.total_vnics.nil? }
> +
> + validates_numericality_of :total_storage,
> + :greater_than_or_equal_to => 0,
> + :unless => Proc.new{ |quota| quota.total_storage.nil? }
> +
> + validates_numericality_of :total_vms,
> + :greater_than_or_equal_to => 0,
> + :unless => Proc.new{ |quota| quota.total_vms.nil? }
>
> def total_vmemory_in_mb
> kb_to_mb(total_vmemory)
> diff --git a/src/app/models/smart_pool_tag.rb
b/src/app/models/smart_pool_tag.rb
> index e135ddf..00bb5a7 100644
> --- a/src/app/models/smart_pool_tag.rb
> +++ b/src/app/models/smart_pool_tag.rb
> @@ -31,6 +31,11 @@ class SmartPoolTag < ActiveRecord::Base
>
> validates_uniqueness_of :smart_pool_id, :scope => [:tagged_id,
:tagged_type]
>
> + validates_presence_of :tagged_id
> +
> + validates_inclusion_of :tagged_type,
> + :in => %w( Pool StoragePool Host Vm )
> +
> def tagged_type=(sType)
> super(sType.to_s.classify.constantize.base_class.to_s)
> end
> diff --git a/src/app/models/storage_pool.rb
b/src/app/models/storage_pool.rb
> index bab7031..f9abb55 100644
> --- a/src/app/models/storage_pool.rb
> +++ b/src/app/models/storage_pool.rb
> @@ -39,6 +39,13 @@ class StoragePool < ActiveRecord::Base
>
> validates_presence_of :hardware_pool_id
>
> + validates_inclusion_of :type,
> + :in => %w( IscsiStoragePool LvmStoragePool NfsStoragePool )
> +
> +
> + validates_numericality_of :capacity,
> + :greater_than_or_equal_to => 0
> +
> acts_as_xapian :texts => [ :ip_addr, :target, :export_path, :type ],
> :terms => [ [ :search_users, 'U',
"search_users" ] ],
> :eager_load => :smart_pools
> @@ -54,6 +61,9 @@ class StoragePool < ActiveRecord::Base
> STATE_PENDING_DELETION = "pending_deletion"
> STATE_AVAILABLE = "available"
>
> + validates_inclusion_of :state,
> + :in => [ STATE_PENDING_SETUP, STATE_PENDING_DELETION,
STATE_AVAILABLE]
> +
> def self.factory(type, params = {})
> params[:state] = STATE_PENDING_SETUP unless params[:state]
> case type
> @@ -121,4 +131,11 @@ class StoragePool < ActiveRecord::Base
> end
> return_hash
> end
> +
> + def movable?
> + storage_volumes.each{ |x|
> + return false unless x.movable?
> + }
> + return true
> + end
> end
> diff --git a/src/app/models/storage_volume.rb
b/src/app/models/storage_volume.rb
> index 39b72d5..385c126 100644
> --- a/src/app/models/storage_volume.rb
> +++ b/src/app/models/storage_volume.rb
> @@ -32,10 +32,23 @@ class StorageVolume < ActiveRecord::Base
> end
> end
>
> + validates_presence_of :storage_pool_id
> +
> +
> + validates_numericality_of :size,
> + :greater_than_or_equal_to => 0,
> + :unless => Proc.new { |storage_volume| storage_volume.nil? }
> +
> + validates_inclusion_of :type,
> + :in => %w( IscsiStorageVolume LvmStorageVolume NfsStorageVolume )
> +
> STATE_PENDING_SETUP = "pending_setup"
> STATE_PENDING_DELETION = "pending_deletion"
> STATE_AVAILABLE = "available"
>
> + validates_inclusion_of :state,
> + :in => [ STATE_PENDING_SETUP, STATE_PENDING_DELETION,
STATE_AVAILABLE]
> +
> def self.factory(type, params = {})
> params[:state] = STATE_PENDING_SETUP unless params[:state]
> case type
> @@ -131,4 +144,12 @@ class StorageVolume < ActiveRecord::Base
> return_hash
> end
>
> + def movable?
> + if vms.size > 0 or
> + (not lvm_storage_pool.nil? and not lvm_stoage_pool.movable?)
typo lvm_stoage_pool [unless that was an elmer fudd ref? ;) ]
and shouldn't it be '!' rather than
'not'?> + return false
> + end
> + return true
> + end
> +
> end
> diff --git a/src/app/models/task.rb b/src/app/models/task.rb
> index f231c18..e6b9079 100644
> --- a/src/app/models/task.rb
> +++ b/src/app/models/task.rb
> @@ -45,6 +45,16 @@ class Task < ActiveRecord::Base
> COMPLETED_STATES = [STATE_FINISHED, STATE_FAILED, STATE_CANCELED]
> WORKING_STATES = [STATE_QUEUED, STATE_RUNNING, STATE_PAUSED]
>
> + validates_inclusion_of :type,
> + :in => %w( HostTask StorageTask StorageVolumeTask VmTask )
> +
> + validates_inclusion_of :state,
> + :in => COMPLETED_STATES + WORKING_STATES
> +
> + # FIXME validate action depending on type / subclass
> + # validate task_target_id, task_type_id, arg, message
> + # depending on subclass, action, state
> +
> def cancel
> self[:state] = STATE_CANCELED
> save!
> @@ -71,4 +81,9 @@ class Task < ActiveRecord::Base
> ""
> end
>
> + def validate
> + errors.add("time_ended", "Tasks ends before its
started") unless time_ended > time_started
> + errors.add("time_started", "Tasks starts before its
created") unless time_started > time_created
> + end
> +
> end
> diff --git a/src/app/models/usage.rb b/src/app/models/usage.rb
> index 353e8f4..59b0e48 100644
> --- a/src/app/models/usage.rb
> +++ b/src/app/models/usage.rb
> @@ -18,4 +18,10 @@
>
> class Usage < ActiveRecord::Base
> has_and_belongs_to_many :networks, :join_table =>
'networks_usages'
> +
> + validates_presence_of :label
> + validates_presence_of :usage
> +
> + validates_uniqueness_of :usage
> +
> end
> diff --git a/src/app/models/vm.rb b/src/app/models/vm.rb
> index 227f343..4d66c63 100644
> --- a/src/app/models/vm.rb
> +++ b/src/app/models/vm.rb
> @@ -34,7 +34,32 @@ class Vm < ActiveRecord::Base
>
> validates_presence_of :uuid, :description, :num_vcpus_allocated,
> :boot_device, :memory_allocated_in_mb,
> - :memory_allocated, :vnic_mac_addr
> + :memory_allocated, :vnic_mac_addr,
> + :vm_resource_pool_id
> +
> + validates_format_of :uuid,
> + :with =>
%r([a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12})
> +
> + validates_numericality_of :needs_restart,
> + :greater_than_or_equal_to => 0,
> + :less_than_or_equal_to => 1,
> + :unless => Proc.new{ |vm| vm.needs_restart.nil? }
> +
> + validates_numericality_of :memory_used,
> + :greater_than_or_equal_to => 0,
> + :unless => Proc.new{ |vm| vm.memory_used.nil? }
> +
> + validates_numericality_of :vnc_port,
> + :greater_than_or_equal_to => 0,
> + :unless => Proc.new{ |vm| vm.vnc_port.nil? }
> +
> + validates_numericality_of :num_vcpus_allocated,
> + :greater_than_or_equal_to => 0,
> + :unless => Proc.new{ |vm| vm.num_vcpus_allocated.nil? }
> +
> + validates_numericality_of :memory_allocated,
> + :greater_than_or_equal_to => 0,
> + :unless => Proc.new{ |vm| vm.memory_allocated.nil? }
>
> acts_as_xapian :texts => [ :uuid, :description, :vnic_mac_addr,
:state ],
> :terms => [ [ :search_users, 'U',
"search_users" ] ],
> @@ -117,9 +142,14 @@ class Vm < ActiveRecord::Base
> STATE_SAVED => STATE_SAVED,
> STATE_RESTORING => STATE_RUNNING,
> STATE_MIGRATING => STATE_RUNNING,
> - STATE_CREATE_FAILED => STATE_CREATE_FAILED}
> + STATE_CREATE_FAILED => STATE_CREATE_FAILED,
> + STATE_INVALID => STATE_INVALID}
> TASK_STATE_TRANSITIONS = []
>
> + validates_inclusion_of :state,
> + :in => EFFECTIVE_STATE.keys
> +
> +
> def get_hardware_pool
> pool = vm_resource_pool
> pool = pool.get_hardware_pool if pool
Mohammed Morsi
2008-Dec-10 22:40 UTC
[Ovirt-devel] [PATCH server] missing ovirt wui validations
This patch now contains all the validations and relevant
fixes to the test suite. Additional tests asserting the
new validations has not been written yet.
Attached is additional validations, added to the rails model
and controller layers verifying:
bondings: arp related fields
boot_types: label uniqueness and proto inclusion
cpus: various fields are set and min values
help_sections: presence and uniqueness of fields
hosts: presence and format of various fields
ip_addresses: fk existance / integrity
nics: mac address format
permissions: presence / inclusion of various fields
pools: type inclusion
quotas: minimum values
smart_pool_tags: tagged_id / type fk present
storage_pools: type, state field inclusion, min values
storage_volumes: various fields, subclasses w/ various fields
tasks: various fields present and consistensy maintained
usages: label, other fields present, unique
vms: uuid of correct format, min values, various fields present
pools lft and rgt are not verified as they are added by
acts_as_nested_set _after_ the pool is inserted into the db
can only add / move host and storage volumes and pools to / from pools
when you have sufficient permissions and target entities don't have
associated vms
can only destroy storage volumes / pools if there are no attached vms
---
src/app/controllers/hardware_controller.rb | 28 ++++++++++++-
src/app/controllers/storage_controller.rb | 7 +++
src/app/models/bonding.rb | 11 +++++
src/app/models/boot_type.rb | 5 ++
src/app/models/cpu.rb | 23 +++++++++++
src/app/models/help_section.rb | 34 ++++++++++++++++
src/app/models/host.rb | 21 ++++++++++
src/app/models/ip_address.rb | 4 ++
src/app/models/iscsi_storage_volume.rb | 2 +
src/app/models/lvm_storage_volume.rb | 6 +++
src/app/models/nfs_storage_pool.rb | 1 +
src/app/models/nic.rb | 11 +++++
src/app/models/permission.rb | 7 +++
src/app/models/pool.rb | 3 +
src/app/models/quota.rb | 22 +++++++++++
src/app/models/smart_pool_tag.rb | 5 ++
src/app/models/storage_pool.rb | 17 ++++++++
src/app/models/storage_volume.rb | 21 ++++++++++
src/app/models/task.rb | 15 +++++++
src/app/models/usage.rb | 6 +++
src/app/models/vm.rb | 34 +++++++++++++++-
src/test/fixtures/hosts.yml | 45 ++++++++++++++--------
src/test/fixtures/storage_pools.yml | 4 ++
src/test/functional/resources_controller_test.rb | 2 +-
src/test/functional/storage_controller_test.rb | 4 +-
25 files changed, 315 insertions(+), 23 deletions(-)
diff --git a/src/app/controllers/hardware_controller.rb
b/src/app/controllers/hardware_controller.rb
index 4dda736..cf45c49 100644
--- a/src/app/controllers/hardware_controller.rb
+++ b/src/app/controllers/hardware_controller.rb
@@ -303,15 +303,39 @@ class HardwareController < PoolController
resource_ids_str = params[:resource_ids]
resource_ids = resource_ids_str.split(",").collect {|x| x.to_i}
+ # if user doesn't have modify permission on both source and destination
+ unless @pool.can_modify(@user) and
Pool.find(params[:target_pool_id]).can_modify(@user)
+ render :json => { :success => false,
+ :alert => "Cannot #{item_action.to_s}
#{item_class.table_name.humanize} without admin permissions on both pools"
}
+ return
+ end
+
+ # relay error message if movable check fails for any resource
+ success = true
+ failed_resources = ""
+ resource_ids.each {|x|
+ unless item_class.find(x).movable?
+ success = false
+ failed_resources += x.to_s + " "
+ end
+ }
+ resource_ids.delete_if { |x| ! item_class.find(x).movable? }
+
begin
@pool.transaction do
@pool.send(item_method, resource_ids, target_pool_id)
end
+ rescue
+ success = false
+ end
+
+ if success
render :json => { :success => true,
:alert => "#{item_action.to_s}
#{item_class.table_name.humanize} successful." }
- rescue
+ else
render :json => { :success => false,
- :alert => "#{item_action.to_s}
#{item_class.table_name.humanize} failed." }
+ :alert => "#{item_action.to_s}
#{item_class.table_name.humanize} failed" +
+ (failed_resources == "" ? "." : "
for " + failed_resources) }
end
end
diff --git a/src/app/controllers/storage_controller.rb
b/src/app/controllers/storage_controller.rb
index 148d1be..47e9e14 100644
--- a/src/app/controllers/storage_controller.rb
+++ b/src/app/controllers/storage_controller.rb
@@ -310,6 +310,13 @@ class StorageController < ApplicationController
end
def destroy
+ unless @storage_pool.movable?
+ format.json { render :json => { :object => "storage_pool",
+ :success => false,
+ :alert => "Cannot delete storage with associated vms" } }
+ return
+ end
+
pool = @storage_pool.hardware_pool
if @storage_pool.destroy
alert="Storage Pool was successfully deleted."
diff --git a/src/app/models/bonding.rb b/src/app/models/bonding.rb
index c9af38c..fabdb67 100644
--- a/src/app/models/bonding.rb
+++ b/src/app/models/bonding.rb
@@ -30,6 +30,8 @@
# interface. They can be ignored if not used.
#
class Bonding < ActiveRecord::Base
+
+
belongs_to :host
belongs_to :bonding_type
belongs_to :vlan
@@ -58,6 +60,15 @@ class Bonding < ActiveRecord::Base
validates_presence_of :vlan_id,
:message => 'A vlan must be specified.'
+ # verify arp ping address to be ipv4 if set
+ validates_format_of :arp_ping_address,
+ :with => %r{^(\d{1,3}\.){3}\d{1,3}$},
+ :unless => Proc.new { |bonding| bonding.arp_ping_address.nil? }
+
+ validates_numericality_of :arp_interval,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new { |bonding| bonding.arp_interval.nil? }
+
protected
def validate
if vlan.boot_type.proto == 'static' and ip_addresses.size == 0
diff --git a/src/app/models/boot_type.rb b/src/app/models/boot_type.rb
index 8ffbe67..6cfdb04 100644
--- a/src/app/models/boot_type.rb
+++ b/src/app/models/boot_type.rb
@@ -18,4 +18,9 @@
# also available at http://www.gnu.org/copyleft/gpl.html.
class BootType < ActiveRecord::Base
+ validates_presence_of :label
+ validates_uniqueness_of :label,
+ :message => 'Label must be unique'
+ validates_inclusion_of :proto,
+ :in => %w( static dhcp bootp )
end
diff --git a/src/app/models/cpu.rb b/src/app/models/cpu.rb
index 1a7d3cf..714a8ed 100644
--- a/src/app/models/cpu.rb
+++ b/src/app/models/cpu.rb
@@ -21,4 +21,27 @@
#
class Cpu < ActiveRecord::Base
belongs_to :host
+
+ validates_presence_of :host_id,
+ :message => 'A host must be specified.'
+
+ validates_numericality_of :cpu_number,
+ :greater_than_or_equal_to => 0
+
+ validates_numericality_of :core_number,
+ :greater_than_or_equal_to => 0
+
+ validates_numericality_of :number_of_cores,
+ :greater_than_or_equal_to => 1
+
+ validates_numericality_of :cpuid_level,
+ :greater_than_or_equal_to => 0
+
+ validates_numericality_of :speed,
+ :greater_than => 0
+ # also verify speed in MHz ?
+
+ validates_presence_of :vendor
+ validates_presence_of :model
+ validates_presence_of :family
end
diff --git a/src/app/models/help_section.rb b/src/app/models/help_section.rb
index a891383..bd3e27c 100644
--- a/src/app/models/help_section.rb
+++ b/src/app/models/help_section.rb
@@ -1,2 +1,36 @@
+# Copyright (C) 2008 Red Hat, Inc.
+# Written by Mohammed Morsi <mmorsi 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.
+#
+# +HelpSection+ defines a section of the web help document to be
+# available for a specific controller / action
+#
class HelpSection < ActiveRecord::Base
+
+ validates_uniqueness_of :action,
+ :scope => :controller,
+ :message => 'Controller / Action must be unique'
+
+ validates_presence_of :controller,
+ :message => 'A controller must be specified.'
+
+ validates_presence_of :action,
+ :message => 'An action must be specified.'
+
+ validates_presence_of :section,
+ :message => 'A section must be specified.'
end
diff --git a/src/app/models/host.rb b/src/app/models/host.rb
index 640782d..08f9282 100644
--- a/src/app/models/host.rb
+++ b/src/app/models/host.rb
@@ -51,6 +51,17 @@ class Host < ActiveRecord::Base
[ :search_users, 'U',
"search_users" ] ],
:eager_load => :smart_pools
+ validates_presence_of :hardware_pool_id,
+ :message => 'A hardware pool id must be specified.'
+
+ validates_presence_of :uuid,
+ :message => 'A uuid must be specified.'
+
+ validates_presence_of :hostname,
+ :message => 'A hostname must be specified.'
+
+ validates_presence_of :arch,
+ :message => 'An arch must be specified.'
KVM_HYPERVISOR_TYPE = "KVM"
HYPERVISOR_TYPES = [KVM_HYPERVISOR_TYPE]
@@ -58,6 +69,12 @@ class Host < ActiveRecord::Base
STATE_AVAILABLE = "available"
STATES = [STATE_UNAVAILABLE, STATE_AVAILABLE]
+ validates_inclusion_of :hypervisor_type,
+ :in => HYPERVISOR_TYPES
+
+ validates_inclusion_of :state,
+ :in => STATES + Task::COMPLETED_STATES + Task::WORKING_STATES
+
def memory_in_mb
kb_to_mb(memory)
end
@@ -99,4 +116,8 @@ class Host < ActiveRecord::Base
hardware_pool.search_users
end
+ def movable?
+ return vms.size == 0
+ end
+
end
diff --git a/src/app/models/ip_address.rb b/src/app/models/ip_address.rb
index 5d2e6af..3f246b1 100644
--- a/src/app/models/ip_address.rb
+++ b/src/app/models/ip_address.rb
@@ -24,4 +24,8 @@ class IpAddress < ActiveRecord::Base
belongs_to :network
belongs_to :nic
belongs_to :bonding
+
+ def validate
+ errors.add("id", "ip must be associated with foreign
entity") if network_id.nil? and nic_id.nil? and bonding_id.nil?
+ end
end
diff --git a/src/app/models/iscsi_storage_volume.rb
b/src/app/models/iscsi_storage_volume.rb
index 48edbd8..fe2cbf5 100644
--- a/src/app/models/iscsi_storage_volume.rb
+++ b/src/app/models/iscsi_storage_volume.rb
@@ -31,4 +31,6 @@ class IscsiStorageVolume < StorageVolume
return true
end
+ validates_presence_of :lun
+
end
diff --git a/src/app/models/lvm_storage_volume.rb
b/src/app/models/lvm_storage_volume.rb
index 8eb1f0e..38949ce 100644
--- a/src/app/models/lvm_storage_volume.rb
+++ b/src/app/models/lvm_storage_volume.rb
@@ -29,4 +29,10 @@ class LvmStorageVolume < StorageVolume
def volume_create_params
return lv_name, size, lv_owner_perms, lv_group_perms, lv_mode_perms
end
+
+ validates_presence_of :lv_name
+ validates_presence_of :lv_owner_perms
+ validates_presence_of :lv_group_perms
+ validates_presence_of :lv_mode_perms
+
end
diff --git a/src/app/models/nfs_storage_pool.rb
b/src/app/models/nfs_storage_pool.rb
index 398e3f9..3b438c8 100644
--- a/src/app/models/nfs_storage_pool.rb
+++ b/src/app/models/nfs_storage_pool.rb
@@ -29,4 +29,5 @@ class NfsStoragePool < StoragePool
def user_subdividable
true
end
+
end
diff --git a/src/app/models/nic.rb b/src/app/models/nic.rb
index 5649763..aa9c740 100644
--- a/src/app/models/nic.rb
+++ b/src/app/models/nic.rb
@@ -24,12 +24,23 @@ class Nic < ActiveRecord::Base
has_and_belongs_to_many :bondings, :join_table => 'bondings_nics'
+ validates_presence_of :mac,
+ :message => 'A mac must be specified.'
+
+ validates_format_of :mac,
+ :with => %r{^([0-9a-f]{2}([:-]|$)){6}$}
+
validates_presence_of :host_id,
:message => 'A host must be specified.'
validates_presence_of :physical_network_id,
:message => 'A network must be specified.'
+ validates_numericality_of :bandwidth,
+ :greater_than_or_equal_to => 0
+
+ # validate 'bridge' or 'usage_type' attribute ?
+
protected
def validate
if physical_network.boot_type.proto == 'static' and
ip_addresses.size == 0
diff --git a/src/app/models/permission.rb b/src/app/models/permission.rb
index ece3da5..b3929ad 100644
--- a/src/app/models/permission.rb
+++ b/src/app/models/permission.rb
@@ -24,9 +24,12 @@ class Permission < ActiveRecord::Base
has_many :child_permissions, :dependent => :destroy,
:class_name => "Permission", :foreign_key =>
"inherited_from_id"
+ validates_presence_of :pool_id
+ validates_presence_of :uid
validates_uniqueness_of :uid, :scope => [:pool_id, :inherited_from_id]
+
ROLE_SUPER_ADMIN = "Super Admin"
ROLE_ADMIN = "Administrator"
ROLE_USER = "User"
@@ -44,6 +47,10 @@ class Permission < ActiveRecord::Base
ROLE_USER => [PRIV_VIEW, PRIV_VM_CONTROL],
ROLE_MONITOR => [PRIV_VIEW]}
+
+ validates_inclusion_of :user_role,
+ :in => ROLES.keys
+
def self.invert_roles
return_hash = {}
ROLES.each do |role, privs|
diff --git a/src/app/models/pool.rb b/src/app/models/pool.rb
index 7034e79..f7cc774 100644
--- a/src/app/models/pool.rb
+++ b/src/app/models/pool.rb
@@ -52,6 +52,9 @@ class Pool < ActiveRecord::Base
validates_presence_of :name
validates_uniqueness_of :name, :scope => :parent_id
+ validates_inclusion_of :type,
+ :in => %w( DirectoryPool HardwarePool VmResourcePool SmartPool )
+
# overloading this method such that we can use permissions.admins to get all
the admins for an object
has_many :permissions, :dependent => :destroy, :order => "id
ASC" do
def super_admins
diff --git a/src/app/models/quota.rb b/src/app/models/quota.rb
index fc52177..2a9e0f0 100644
--- a/src/app/models/quota.rb
+++ b/src/app/models/quota.rb
@@ -22,6 +22,28 @@ require 'util/ovirt'
class Quota < ActiveRecord::Base
belongs_to :pool
+ validates_presence_of :pool_id
+
+
+ validates_numericality_of :total_vcpus,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new{ |quota| quota.total_vcpus.nil? }
+
+ validates_numericality_of :total_vmemory,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new{ |quota| quota.total_vmemory.nil? }
+
+ validates_numericality_of :total_vnics,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new{ |quota| quota.total_vnics.nil? }
+
+ validates_numericality_of :total_storage,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new{ |quota| quota.total_storage.nil? }
+
+ validates_numericality_of :total_vms,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new{ |quota| quota.total_vms.nil? }
def total_vmemory_in_mb
kb_to_mb(total_vmemory)
diff --git a/src/app/models/smart_pool_tag.rb b/src/app/models/smart_pool_tag.rb
index e135ddf..00bb5a7 100644
--- a/src/app/models/smart_pool_tag.rb
+++ b/src/app/models/smart_pool_tag.rb
@@ -31,6 +31,11 @@ class SmartPoolTag < ActiveRecord::Base
validates_uniqueness_of :smart_pool_id, :scope => [:tagged_id,
:tagged_type]
+ validates_presence_of :tagged_id
+
+ validates_inclusion_of :tagged_type,
+ :in => %w( Pool StoragePool Host Vm )
+
def tagged_type=(sType)
super(sType.to_s.classify.constantize.base_class.to_s)
end
diff --git a/src/app/models/storage_pool.rb b/src/app/models/storage_pool.rb
index bab7031..f9abb55 100644
--- a/src/app/models/storage_pool.rb
+++ b/src/app/models/storage_pool.rb
@@ -39,6 +39,13 @@ class StoragePool < ActiveRecord::Base
validates_presence_of :hardware_pool_id
+ validates_inclusion_of :type,
+ :in => %w( IscsiStoragePool LvmStoragePool NfsStoragePool )
+
+
+ validates_numericality_of :capacity,
+ :greater_than_or_equal_to => 0
+
acts_as_xapian :texts => [ :ip_addr, :target, :export_path, :type ],
:terms => [ [ :search_users, 'U',
"search_users" ] ],
:eager_load => :smart_pools
@@ -54,6 +61,9 @@ class StoragePool < ActiveRecord::Base
STATE_PENDING_DELETION = "pending_deletion"
STATE_AVAILABLE = "available"
+ validates_inclusion_of :state,
+ :in => [ STATE_PENDING_SETUP, STATE_PENDING_DELETION, STATE_AVAILABLE]
+
def self.factory(type, params = {})
params[:state] = STATE_PENDING_SETUP unless params[:state]
case type
@@ -121,4 +131,11 @@ class StoragePool < ActiveRecord::Base
end
return_hash
end
+
+ def movable?
+ storage_volumes.each{ |x|
+ return false unless x.movable?
+ }
+ return true
+ end
end
diff --git a/src/app/models/storage_volume.rb b/src/app/models/storage_volume.rb
index 39b72d5..0f490f8 100644
--- a/src/app/models/storage_volume.rb
+++ b/src/app/models/storage_volume.rb
@@ -32,10 +32,23 @@ class StorageVolume < ActiveRecord::Base
end
end
+ validates_presence_of :storage_pool_id
+
+
+ validates_numericality_of :size,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new { |storage_volume| storage_volume.nil? }
+
+ validates_inclusion_of :type,
+ :in => %w( IscsiStorageVolume LvmStorageVolume NfsStorageVolume )
+
STATE_PENDING_SETUP = "pending_setup"
STATE_PENDING_DELETION = "pending_deletion"
STATE_AVAILABLE = "available"
+ validates_inclusion_of :state,
+ :in => [ STATE_PENDING_SETUP, STATE_PENDING_DELETION, STATE_AVAILABLE]
+
def self.factory(type, params = {})
params[:state] = STATE_PENDING_SETUP unless params[:state]
case type
@@ -131,4 +144,12 @@ class StorageVolume < ActiveRecord::Base
return_hash
end
+ def movable?
+ if vms.size > 0 or
+ (! lvm_storage_pool.nil? and ! lvm_storage_pool.movable?)
+ return false
+ end
+ return true
+ end
+
end
diff --git a/src/app/models/task.rb b/src/app/models/task.rb
index f231c18..cb715e7 100644
--- a/src/app/models/task.rb
+++ b/src/app/models/task.rb
@@ -45,6 +45,16 @@ class Task < ActiveRecord::Base
COMPLETED_STATES = [STATE_FINISHED, STATE_FAILED, STATE_CANCELED]
WORKING_STATES = [STATE_QUEUED, STATE_RUNNING, STATE_PAUSED]
+ validates_inclusion_of :type,
+ :in => %w( HostTask StorageTask StorageVolumeTask VmTask )
+
+ validates_inclusion_of :state,
+ :in => COMPLETED_STATES + WORKING_STATES
+
+ # FIXME validate action depending on type / subclass
+ # validate task_target_id, task_type_id, arg, message
+ # depending on subclass, action, state
+
def cancel
self[:state] = STATE_CANCELED
save!
@@ -71,4 +81,9 @@ class Task < ActiveRecord::Base
""
end
+ def validate
+ errors.add("time_ended", "Tasks ends before its
started") unless time_ended.nil? or time_started.nil? or time_ended >
time_started
+ errors.add("time_started", "Tasks starts before its
created") unless time_started.nil? or time_created.nil? or time_started
> time_created
+ end
+
end
diff --git a/src/app/models/usage.rb b/src/app/models/usage.rb
index 353e8f4..59b0e48 100644
--- a/src/app/models/usage.rb
+++ b/src/app/models/usage.rb
@@ -18,4 +18,10 @@
class Usage < ActiveRecord::Base
has_and_belongs_to_many :networks, :join_table =>
'networks_usages'
+
+ validates_presence_of :label
+ validates_presence_of :usage
+
+ validates_uniqueness_of :usage
+
end
diff --git a/src/app/models/vm.rb b/src/app/models/vm.rb
index 227f343..4d66c63 100644
--- a/src/app/models/vm.rb
+++ b/src/app/models/vm.rb
@@ -34,7 +34,32 @@ class Vm < ActiveRecord::Base
validates_presence_of :uuid, :description, :num_vcpus_allocated,
:boot_device, :memory_allocated_in_mb,
- :memory_allocated, :vnic_mac_addr
+ :memory_allocated, :vnic_mac_addr,
+ :vm_resource_pool_id
+
+ validates_format_of :uuid,
+ :with =>
%r([a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12})
+
+ validates_numericality_of :needs_restart,
+ :greater_than_or_equal_to => 0,
+ :less_than_or_equal_to => 1,
+ :unless => Proc.new{ |vm| vm.needs_restart.nil? }
+
+ validates_numericality_of :memory_used,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new{ |vm| vm.memory_used.nil? }
+
+ validates_numericality_of :vnc_port,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new{ |vm| vm.vnc_port.nil? }
+
+ validates_numericality_of :num_vcpus_allocated,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new{ |vm| vm.num_vcpus_allocated.nil? }
+
+ validates_numericality_of :memory_allocated,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new{ |vm| vm.memory_allocated.nil? }
acts_as_xapian :texts => [ :uuid, :description, :vnic_mac_addr, :state ],
:terms => [ [ :search_users, 'U',
"search_users" ] ],
@@ -117,9 +142,14 @@ class Vm < ActiveRecord::Base
STATE_SAVED => STATE_SAVED,
STATE_RESTORING => STATE_RUNNING,
STATE_MIGRATING => STATE_RUNNING,
- STATE_CREATE_FAILED => STATE_CREATE_FAILED}
+ STATE_CREATE_FAILED => STATE_CREATE_FAILED,
+ STATE_INVALID => STATE_INVALID}
TASK_STATE_TRANSITIONS = []
+ validates_inclusion_of :state,
+ :in => EFFECTIVE_STATE.keys
+
+
def get_hardware_pool
pool = vm_resource_pool
pool = pool.get_hardware_pool if pool
diff --git a/src/test/fixtures/hosts.yml b/src/test/fixtures/hosts.yml
index 31e7580..4b97053 100644
--- a/src/test/fixtures/hosts.yml
+++ b/src/test/fixtures/hosts.yml
@@ -5,7 +5,7 @@ prod_corp_com:
memory: 18384
is_disabled: 0
state: available
- hypervisor_type: xen
+ hypervisor_type: KVM
hardware_pool: corp_com
workstation_corp_com:
uuid: 1f2a8694-961d-11dc-9387-001558c41534
@@ -13,7 +13,8 @@ workstation_corp_com:
arch: i386
memory: 2048
is_disabled: 0
- hypervisor_type: qemu
+ state: available
+ hypervisor_type: KVM
hardware_pool: corp_com
macworkstation_qa_corp_com:
uuid: 58a85f44-75fd-4934-805f-88e45b40d4b4
@@ -21,7 +22,8 @@ macworkstation_qa_corp_com:
arch: mips
memory: 2048
is_disabled: 0
- hypervisor_type: kvm
+ state: available
+ hypervisor_type: KVM
hardware_pool: corp_com_qa
fedoraworkstation_qa_corp_com:
uuid: 520bbb34-6515-490e-9d07-0c8b14f76805
@@ -29,7 +31,8 @@ fedoraworkstation_qa_corp_com:
arch: i386
memory: 2048
is_disabled: 1
- hypervisor_type: kvm
+ state: available
+ hypervisor_type: KVM
hardware_pool: corp_com_qa
pipeline_qa_corp_com:
uuid: 2e422f66-324e-48d4-973f-0b91b33070f9
@@ -37,7 +40,8 @@ pipeline_qa_corp_com:
arch: xeon
memory: 1048576
is_disabled: 0
- hypervisor_type: kvm
+ state: available
+ hypervisor_type: KVM
hardware_pool: corp_com_qa
app_qa_corp_com:
uuid: bb0ce7c7-f234-49ae-84b5-6f4fd8bddcaa
@@ -45,7 +49,8 @@ app_qa_corp_com:
arch: xeon
memory: 16384
is_disabled: 0
- hypervisor_type: kvm
+ state: available
+ hypervisor_type: KVM
hardware_pool: corp_com_qa
issue_qa_corp_com:
uuid: ec0d86de-657b-48f6-b7cc-e733a3f9a834
@@ -53,7 +58,8 @@ issue_qa_corp_com:
arch: x86
memory: 4096
is_disabled: 0
- hypervisor_type: kvm
+ state: available
+ hypervisor_type: KVM
hardware_pool: corp_com_qa
db_dev_corp_com:
uuid: 81c15560-dbf4-45f5-9b75-106cdbd63aeb
@@ -61,7 +67,8 @@ db_dev_corp_com:
arch: x86
memory: 4096
is_disabled: 0
- hypervisor_type: kvm
+ state: available
+ hypervisor_type: KVM
hardware_pool: corp_com_dev
mystation_dev_corp_com:
uuid: 6ae3d22e-97e0-4d86-9712-5395b20a0f45
@@ -69,7 +76,8 @@ mystation_dev_corp_com:
arch: i686
memory: 2048
is_disabled: 0
- hypervisor_type: xen
+ state: available
+ hypervisor_type: KVM
hardware_pool: corp_com_dev
node3_priv_ovirt_org:
uuid: 75f8d5f3-faf0-4308-9f20-e4b03d013a27
@@ -77,16 +85,17 @@ node3_priv_ovirt_org:
arch: x86_64
memory: 4194304
is_disabled: 0
- hypervisor_type: kvm
- hardware_pool: default
state: available
+ hypervisor_type: KVM
+ hardware_pool: default
mailservers_managed_node:
uuid: 182a8596-961d-11dc-9387-001558c41534
hostname: mail.mynetwork.com
arch: i386
memory: 16384
is_disabled: 0
- hypervisor_type: kvm
+ state: available
+ hypervisor_type: KVM
hardware_pool: corp_com_prod
fileserver_managed_node:
uuid: 928ad8172-9723-11dc-9387-001558c41534
@@ -94,7 +103,8 @@ fileserver_managed_node:
arch: x86_64
memory: 32768
is_disabled: 0
- hypervisor_type: kvm
+ state: available
+ hypervisor_type: KVM
hardware_pool: corp_com_prod
ldapserver_managed_node:
uuid: 919ae372-9156-11dc-9387-001558c41534
@@ -102,7 +112,8 @@ ldapserver_managed_node:
arch: i386
memory: 16384
is_disabled: 0
- hypervisor_type: kvm
+ state: available
+ hypervisor_type: KVM
hardware_pool: corp_com_prod
buildserver_managed_node:
uuid: 6293acd9-2784-11dc-9387-001558c41534
@@ -110,7 +121,8 @@ buildserver_managed_node:
arch: x86_64
memory: 65536
is_disabled: 0
- hypervisor_type: kvm
+ state: available
+ hypervisor_type: KVM
hardware_pool: corp_com_prod
mediaserver_managed_node:
uuid: 6293acd9-2784-11dc-9387-001558c41534
@@ -118,5 +130,6 @@ mediaserver_managed_node:
arch: x86_64
memory: 65536
is_disabled: 0
- hypervisor_type: kvm
+ state: available
+ hypervisor_type: KVM
hardware_pool: corp_com_prod
diff --git a/src/test/fixtures/storage_pools.yml
b/src/test/fixtures/storage_pools.yml
index ac72f99..3cda6db 100644
--- a/src/test/fixtures/storage_pools.yml
+++ b/src/test/fixtures/storage_pools.yml
@@ -5,14 +5,18 @@ corp_com_ovirtpriv_storage:
port: 3260
target: ovirtpriv:storage
state: available
+ capacity: 1024
corp_com_nfs_ovirtnfs:
hardware_pool: corp_com
type: NfsStoragePool
ip_addr: 192.168.50.2
export_path: /ovirtnfs
+ capacity: 1024
+ state: available
corp_com_dev_nfs_ovirtnfs:
hardware_pool: corp_com_dev
type: NfsStoragePool
ip_addr: 192.168.50.3
export_path: /devnfs
state: available
+ capacity: 1024
diff --git a/src/test/functional/resources_controller_test.rb
b/src/test/functional/resources_controller_test.rb
index 8e3989b..0312c0c 100644
--- a/src/test/functional/resources_controller_test.rb
+++ b/src/test/functional/resources_controller_test.rb
@@ -45,7 +45,7 @@ class ResourcesControllerTest < ActionController::TestCase
def test_create
assert_difference('VmResourcePool.count') do
- post :create, :vm_resource_pool => { :name =>
'foo_resource_pool' }, :parent_id => pools(:default).id
+ post :create, :pool => { :name => 'foo_resource_pool'},
:parent_id => pools(:default).id
end
assert_response :success
diff --git a/src/test/functional/storage_controller_test.rb
b/src/test/functional/storage_controller_test.rb
index a448fdf..f08fe1b 100644
--- a/src/test/functional/storage_controller_test.rb
+++ b/src/test/functional/storage_controller_test.rb
@@ -69,11 +69,11 @@ class StorageControllerTest < Test::Unit::TestCase
assert_not_nil assigns(:storage_pools)
end
- def test_create
+ def test_create_storage_controller
hw_pool = HardwarePool.get_default_pool
num_storage_volumes = StoragePool.count
- post :create, :storage_type => 'NFS', :storage_pool => {
:hardware_pool => hw_pool, :ip_addr => '111.121.131.141',
:export_path => '/tmp/path' }
+ post :create, :storage_type => 'NFS', :storage_pool => {
:hardware_pool => hw_pool, :ip_addr => '111.121.131.141',
:export_path => '/tmp/path', :state => 'available',
:capacity => 1024 }
assert_response :success
--
1.6.0.4