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 <[email protected]> --- 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 <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
