This patch fixes VM shutdown. Previously when shutting down a vm we
just set it to 'stopped'. However, this is not always the case as
it's
possible for the guest OS to ignore the shutdown event. Also it is
possible for a VM to be stopped by the user in which case it would be
nice to set it stopped in the DB and clear the flags.
Signed-off-by: Ian Main <imain at redhat.com>
---
src/db-omatic/db_omatic.rb | 20 +++++++++++++++++++-
src/task-omatic/taskomatic.rb | 25 +++++++++++++++++++------
2 files changed, 38 insertions(+), 7 deletions(-)
diff --git a/src/db-omatic/db_omatic.rb b/src/db-omatic/db_omatic.rb
index 8409c91..7ae08da 100755
--- a/src/db-omatic/db_omatic.rb
+++ b/src/db-omatic/db_omatic.rb
@@ -181,8 +181,26 @@ class DbOmatic < Qpid::Qmf::Console
end
@logger.info "Updating VM #{domain['name']} to state
#{state}"
+
+ if state == Vm::STATE_STOPPED
+ @logger.info "VM has moved to stopped, clearing VM
attributes."
+ qmf_vm = @session.object(:class => "domain",
'uuid' => vm.uuid)
+ if qmf_vm
+ @logger.info "Deleting VM #{vm.description}."
+ result = qmf_vm.undefine
+ if result.status == 0
+ @logger.info "Delete of VM #{vm.description}
successful, syncing DB."
+ vm.host_id = nil
+ vm.memory_used = nil
+ vm.num_vcpus_used = nil
+ vm.state = Vm::STATE_STOPPED
+ vm.needs_restart = nil
+ vm.vnc_port = nil
+ end
+ end
+ end
vm.state = state
- vm.save
+ vm.save!
domain[:synced] = true
end
diff --git a/src/task-omatic/taskomatic.rb b/src/task-omatic/taskomatic.rb
index 7e3540c..df59e8d 100755
--- a/src/task-omatic/taskomatic.rb
+++ b/src/task-omatic/taskomatic.rb
@@ -266,7 +266,12 @@ class TaskOmatic
raise "Unable to get node that vm is on??" unless node
if vm.state == "shutdown" or vm.state == "shutoff"
- set_vm_shut_down(db_vm)
+ result = vm.undefine
+ if result.status == 0
+ @logger.info "Deleted VM #{db_vm.description}."
+ set_vm_shut_down(db_vm)
+ teardown_storage_pools(node)
+ end
return
elsif vm.state == "suspended"
raise "Cannot shutdown suspended domain"
@@ -276,9 +281,11 @@ class TaskOmatic
if action == :shutdown
result = vm.shutdown
+ @logger.info "shutdown - result.status is #{result.status}"
raise "Error shutting down VM: #{result.text}" unless
result.status == 0
elsif action == :destroy
result = vm.destroy
+ @logger.info "destroy - result.status is #{result.status}"
raise "Error destroying VM: #{result.text}" unless
result.status == 0
end
@@ -288,12 +295,18 @@ class TaskOmatic
# FIXME: we really should have a marker in the database somehow so that
# we can tell if this domain was migrated; that way, we can tell the
# difference between a real undefine failure and one because of migration
+ #
+ # We only set the vm to shutdown if the undefine succeeds. Otherwise,
+ # dbomatic will pick up any state changes. doing a 'shutdown'
rather
+ # than destroy for instance can possibly take quite some time.
result = vm.undefine
- @logger.info "Error undefining VM: #{result.text}" unless
result.status == 0
-
- teardown_storage_pools(node)
-
- set_vm_shut_down(db_vm)
+ if result.status == 0
+ @logger.info "Deleted VM #{db_vm.description}."
+ set_vm_shut_down(db_vm)
+ teardown_storage_pools(node)
+ else
+ @logger.info "Error undefining VM: #{result.text}" unless
result.status == 0
+ end
end
def task_start_vm(task)
--
1.6.0.6
On Wed, Mar 25, 2009 at 12:01:44PM -0700, Ian Main wrote:> This patch fixes VM shutdown. Previously when shutting down a vm we > just set it to 'stopped'. However, this is not always the case as it's > possible for the guest OS to ignore the shutdown event. Also it is > possible for a VM to be stopped by the user in which case it would be > nice to set it stopped in the DB and clear the flags. > > Signed-off-by: Ian Main <imain at redhat.com> > --- > src/db-omatic/db_omatic.rb | 20 +++++++++++++++++++- > src/task-omatic/taskomatic.rb | 25 +++++++++++++++++++------ > 2 files changed, 38 insertions(+), 7 deletions(-) > > diff --git a/src/db-omatic/db_omatic.rb b/src/db-omatic/db_omatic.rb > index 8409c91..7ae08da 100755 > --- a/src/db-omatic/db_omatic.rb > +++ b/src/db-omatic/db_omatic.rb > @@ -181,8 +181,26 @@ class DbOmatic < Qpid::Qmf::Console > end > > @logger.info "Updating VM #{domain['name']} to state #{state}" > + > + if state == Vm::STATE_STOPPED > + @logger.info "VM has moved to stopped, clearing VM attributes." > + qmf_vm = @session.object(:class => "domain", 'uuid' => vm.uuid) > + if qmf_vm > + @logger.info "Deleting VM #{vm.description}." > + result = qmf_vm.undefine > + if result.status == 0 > + @logger.info "Delete of VM #{vm.description} successful, syncing DB." > + vm.host_id = nil > + vm.memory_used = nil > + vm.num_vcpus_used = nil > + vm.state = Vm::STATE_STOPPED > + vm.needs_restart = nil > + vm.vnc_port = nil > + end > + end > + end > vm.state = state > - vm.save > + vm.save! > > domain[:synced] = true > end > diff --git a/src/task-omatic/taskomatic.rb b/src/task-omatic/taskomatic.rb > index 7e3540c..df59e8d 100755 > --- a/src/task-omatic/taskomatic.rb > +++ b/src/task-omatic/taskomatic.rb > @@ -266,7 +266,12 @@ class TaskOmatic > raise "Unable to get node that vm is on??" unless node > > if vm.state == "shutdown" or vm.state == "shutoff" > - set_vm_shut_down(db_vm) > + result = vm.undefine > + if result.status == 0 > + @logger.info "Deleted VM #{db_vm.description}." > + set_vm_shut_down(db_vm) > + teardown_storage_pools(node) > + end > return > elsif vm.state == "suspended" > raise "Cannot shutdown suspended domain" > @@ -276,9 +281,11 @@ class TaskOmatic > > if action == :shutdown > result = vm.shutdown > + @logger.info "shutdown - result.status is #{result.status}" > raise "Error shutting down VM: #{result.text}" unless result.status == 0 > elsif action == :destroy > result = vm.destroy > + @logger.info "destroy - result.status is #{result.status}" > raise "Error destroying VM: #{result.text}" unless result.status == 0 > end > > @@ -288,12 +295,18 @@ class TaskOmatic > # FIXME: we really should have a marker in the database somehow so that > # we can tell if this domain was migrated; that way, we can tell the > # difference between a real undefine failure and one because of migration > + # > + # We only set the vm to shutdown if the undefine succeeds. Otherwise, > + # dbomatic will pick up any state changes. doing a 'shutdown' rather > + # than destroy for instance can possibly take quite some time. > result = vm.undefine > - @logger.info "Error undefining VM: #{result.text}" unless result.status == 0 > - > - teardown_storage_pools(node) > - > - set_vm_shut_down(db_vm) > + if result.status == 0 > + @logger.info "Deleted VM #{db_vm.description}." > + set_vm_shut_down(db_vm) > + teardown_storage_pools(node) > + else > + @logger.info "Error undefining VM: #{result.text}" unless result.status == 0 > + end > end > > def task_start_vm(task) > --Tested this and it appears to work, with the caveat that ACPI shutdown does not appear to be functional on Fedora 10 (works for win2k3 however). Therefore ACK. --Hugh