Change in vdsm[master]: vm: simplify the releaseVm flow

fromani at redhat.com fromani at redhat.com
Tue Apr 29 07:48:04 UTC 2014


Francesco Romani has uploaded a new change for review.

Change subject: vm: simplify the releaseVm flow
......................................................................

vm: simplify the releaseVm flow

After inspection of the libvirt code, turns out
the time.sleep(30) we currently have in the releaseVm
flow is redundant and wasteful.

Libvirt (~1.2.3) already has hardcoded (!) timeout to handle
the QEMU process termination, and this patch leverages that.

Before this patch, the sequence of events triggered
by the dom.destroy() call in the worst case scenario
(QEMU not responding) is:
- send SIGTERM (clean shutdown request)
- wait 10s (libvirt, hardcoded)
- (fails)
- log error (libvirt, vdsm)
- wait 30s (vdsm)
- send SIGTERM (clean shutdown request)
- wait 10s (libvirt)
- (fails)
- log error (libvirt)
- send SIGKILL (forced shutdown request)
- wait 5s( libvirt, hardcoded)
total time: 10+30+10+5 = 45s (all hardcoded!)

With this patch, the flow becomes:
- send SIGTERM (clean shutdown request)
- wait 10s (libvirt, hardcoded)
- (fails)
- log error (libvirt, vdsm)
- send SIGTERM (clean shutdown request)
- wait 10s (libvirt)
- (fails)
- log error (libvirt)
- send SIGKILL (forced shutdown request)
- wait 5s( libvirt, hardcoded)
total time: 10+10+5 = 25s (no values hardcoded in VDSM)

Change-Id: Ia25794217e63dd4c755d9c141a2e941e4baa6fd3
Bug-Url: https://bugzilla.redhat.com/1091389
Signed-off-by: Francesco Romani <fromani at redhat.com>
---
M vdsm/virt/vm.py
1 file changed, 20 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/75/27175/1

diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 79ff40b..655689a 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -4257,16 +4257,7 @@
                 if self.guestAgent:
                     self.guestAgent.stop()
                 if self._dom:
-                    try:
-                        self._dom.destroyFlags(
-                            libvirt.VIR_DOMAIN_DESTROY_GRACEFUL)
-                    except libvirt.libvirtError as e:
-                        if (e.get_error_code() ==
-                                libvirt.VIR_ERR_OPERATION_FAILED):
-                            self.log.warn("Failed to destroy VM '%s' "
-                                          "gracefully", self.conf['vmId'])
-                            time.sleep(30)
-                            self._dom.destroy()
+                    self._destroyVm()
             except libvirt.libvirtError as e:
                 if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN:
                     self.log.warning("libvirt domain not found", exc_info=True)
@@ -4288,6 +4279,25 @@
 
         return {'status': doneCode}
 
+    def _destroyVm(self):
+        """
+        dom.destroy() goes like this (as libvirt ~1.2.3):
+        - sends SIGTERM (clean shutdown)
+        - waits for 10s (hardcoded)
+        - if !GRACEFUL:
+          - sends SIGKILL (forced shutdown)
+          - waits for 5s more (hardcoded)
+        so we actually do two clean shutdown requests and
+        if the second fails, we resort to forced shutdown.
+        """
+        try:
+            self._dom.destroyFlags(libvirt.VIR_DOMAIN_DESTROY_GRACEFUL)
+        except libvirt.libvirtError as e:
+            if e.get_error_code() == libvirt.VIR_ERR_OPERATION_FAILED:
+                self.log.warn("Failed to destroy VM '%s' "
+                              "gracefully", self.conf['vmId'])
+            self._dom.destroy()
+
     def deleteVm(self):
         """
         Clean VM from the system


-- 
To view, visit http://gerrit.ovirt.org/27175
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia25794217e63dd4c755d9c141a2e941e4baa6fd3
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>


More information about the vdsm-patches mailing list