[Ovirt-devel] [PATCH server] Fix VM shutdown

[Ian Main]

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