Scott Seago
2008-Oct-14 21:28 UTC
[Ovirt-devel] [PATCH] fix validation issues for VM destroy
Signed-off-by: Scott Seago <sseago at redhat.com>
---
src/app/controllers/vm_controller.rb | 25 +++++++++++++++++--------
src/app/models/vm.rb | 23 +++++++++++++++++++++++
2 files changed, 40 insertions(+), 8 deletions(-)
diff --git a/src/app/controllers/vm_controller.rb
b/src/app/controllers/vm_controller.rb
index bc88760..a9ac9a5 100644
--- a/src/app/controllers/vm_controller.rb
+++ b/src/app/controllers/vm_controller.rb
@@ -137,26 +137,35 @@ class VmController < ApplicationController
def delete
vm_ids_str = params[:vm_ids]
vm_ids = vm_ids_str.split(",").collect {|x| x.to_i}
-
+ failure_list = []
+ success = false
begin
Vm.transaction do
vms = Vm.find(:all, :conditions => "id in (#{vm_ids.join(',
')})")
vms.each do |vm|
- vm.destroy
+ if vm.is_destroyable?
+ vm.destroy
+ else
+ failure_list << vm.description
+ end
end
end
- render :json => { :object => "vm", :success => true,
- :alert => "Virtual Machines were successfully deleted." }
+ if failure_list.empty?
+ success = true
+ alert = "Virtual Machines were successfully deleted."
+ else
+ alert = "The following Virtual Machines were not deleted (a VM
must be stopped to delete it): "
+ alert+= failure_list.join(', ')
+ end
rescue
- render :json => { :object => "vm", :success => false,
- :alert => "Error deleting virtual machines." }
+ alert = "Error deleting virtual machines."
end
+ render :json => { :object => "vm", :success => success,
:alert => alert }
end
def destroy
vm_resource_pool = @vm.vm_resource_pool_id
- if ((@vm.state == Vm::STATE_STOPPED and @vm.get_pending_state ==
Vm::STATE_STOPPED) or
- (@vm.state == Vm::STATE_PENDING and @vm.get_pending_state ==
Vm::STATE_PENDING))
+ if (@vm.is_destroyable?)
@vm.destroy
render :json => { :object => "vm", :success => true,
:alert => "Virtual Machine was successfully deleted." }
diff --git a/src/app/models/vm.rb b/src/app/models/vm.rb
index d7beacf..6853157 100644
--- a/src/app/models/vm.rb
+++ b/src/app/models/vm.rb
@@ -84,6 +84,11 @@ class Vm < ActiveRecord::Base
STATE_CREATE_FAILED = "create_failed"
STATE_INVALID = "invalid"
+ DESTROYABLE_STATES = [STATE_PENDING,
+ STATE_STOPPED,
+ STATE_CREATE_FAILED,
+ STATE_INVALID]
+
RUNNING_STATES = [STATE_RUNNING,
STATE_SUSPENDED,
STATE_STOPPING,
@@ -272,6 +277,24 @@ class Vm < ActiveRecord::Base
end
end
+ # whether this VM may be validly deleted. running VMs should not be
+ # allowed to be deleted. Currently we restrict deletion to VMs that
+ # are currently stopped, pending (new without any create_vm tasks having
+ # been run), or create_failed. Also, get_pending_state must equal the
+ # current state -- so that we won't delete a VM with a current pending
task
+ def is_destroyable?
+ current_state = state
+ pending_state = get_pending_state
+ DESTROYABLE_STATES.include?(current_state) and (current_state ==
pending_state)
+ end
+
+ def destroy
+ if !is_destroyable?
+ raise "VM must be stopped to delete it"
+ end
+ super
+ end
+
protected
def validate
resources = vm_resource_pool.max_resources_for_vm(self)
--
1.5.5.1
Perry Myers
2008-Oct-15 02:12 UTC
[Ovirt-devel] [PATCH] fix validation issues for VM destroy
Scott Seago wrote:> Signed-off-by: Scott Seago <sseago at redhat.com> > --- > src/app/controllers/vm_controller.rb | 25 +++++++++++++++++-------- > src/app/models/vm.rb | 23 +++++++++++++++++++++++ > 2 files changed, 40 insertions(+), 8 deletions(-)This seems to work ok, so ACK and I'm going to push this. Perry> diff --git a/src/app/controllers/vm_controller.rb b/src/app/controllers/vm_controller.rb > index bc88760..a9ac9a5 100644 > --- a/src/app/controllers/vm_controller.rb > +++ b/src/app/controllers/vm_controller.rb > @@ -137,26 +137,35 @@ class VmController < ApplicationController > def delete > vm_ids_str = params[:vm_ids] > vm_ids = vm_ids_str.split(",").collect {|x| x.to_i} > - > + failure_list = [] > + success = false > begin > Vm.transaction do > vms = Vm.find(:all, :conditions => "id in (#{vm_ids.join(', ')})") > vms.each do |vm| > - vm.destroy > + if vm.is_destroyable? > + vm.destroy > + else > + failure_list << vm.description > + end > end > end > - render :json => { :object => "vm", :success => true, > - :alert => "Virtual Machines were successfully deleted." } > + if failure_list.empty? > + success = true > + alert = "Virtual Machines were successfully deleted." > + else > + alert = "The following Virtual Machines were not deleted (a VM must be stopped to delete it): " > + alert+= failure_list.join(', ') > + end > rescue > - render :json => { :object => "vm", :success => false, > - :alert => "Error deleting virtual machines." } > + alert = "Error deleting virtual machines." > end > + render :json => { :object => "vm", :success => success, :alert => alert } > end > > def destroy > vm_resource_pool = @vm.vm_resource_pool_id > - if ((@vm.state == Vm::STATE_STOPPED and @vm.get_pending_state == Vm::STATE_STOPPED) or > - (@vm.state == Vm::STATE_PENDING and @vm.get_pending_state == Vm::STATE_PENDING)) > + if (@vm.is_destroyable?) > @vm.destroy > render :json => { :object => "vm", :success => true, > :alert => "Virtual Machine was successfully deleted." } > diff --git a/src/app/models/vm.rb b/src/app/models/vm.rb > index d7beacf..6853157 100644 > --- a/src/app/models/vm.rb > +++ b/src/app/models/vm.rb > @@ -84,6 +84,11 @@ class Vm < ActiveRecord::Base > STATE_CREATE_FAILED = "create_failed" > STATE_INVALID = "invalid" > > + DESTROYABLE_STATES = [STATE_PENDING, > + STATE_STOPPED, > + STATE_CREATE_FAILED, > + STATE_INVALID] > + > RUNNING_STATES = [STATE_RUNNING, > STATE_SUSPENDED, > STATE_STOPPING, > @@ -272,6 +277,24 @@ class Vm < ActiveRecord::Base > end > end > > + # whether this VM may be validly deleted. running VMs should not be > + # allowed to be deleted. Currently we restrict deletion to VMs that > + # are currently stopped, pending (new without any create_vm tasks having > + # been run), or create_failed. Also, get_pending_state must equal the > + # current state -- so that we won't delete a VM with a current pending task > + def is_destroyable? > + current_state = state > + pending_state = get_pending_state > + DESTROYABLE_STATES.include?(current_state) and (current_state == pending_state) > + end > + > + def destroy > + if !is_destroyable? > + raise "VM must be stopped to delete it" > + end > + super > + end > + > protected > def validate > resources = vm_resource_pool.max_resources_for_vm(self)-- |=- Red Hat, Engineering, Emerging Technologies, Boston -=| |=- Email: pmyers at redhat.com -=| |=- Office: +1 412 474 3552 Mobile: +1 703 362 9622 -=| |=- GnuPG: E65E4F3D 88F9 F1C9 C2F3 1303 01FE 817C C5D2 8B91 E65E 4F3D -=|