From 6b578665d4573b248e01ae5e0a77f200833c3b5c Mon Sep 17 00:00:00 2001 From: John Garbutt Date: Tue, 25 Feb 2014 11:46:39 +0000 Subject: [PATCH] xenapi: Cleanup tar process on glance error Currently, when the network connection to glance is interrupted, there is different behaviour on upload and download. This change ensures the behaviour between the two code paths is more consistent. Uploads generally need to be given more time before they timeout, so to keep a single timeout between upload and download, the timeout is increased to 90 seconds. At the same time, it ensures the tar process gets killed when any issues occur with the communication between the hypervisor and glance. Change-Id: Id5396e5d3c1052dc2979476a886412da65e08670 Closes-Bug: #1284596 --- .../xenapi/etc/xapi.d/plugins/glance | 19 +++++-- .../xenapi/etc/xapi.d/plugins/utils.py | 49 +++++++++++++------ 2 files changed, 48 insertions(+), 20 deletions(-) diff --git a/plugins/xenserver/xenapi/etc/xapi.d/plugins/glance b/plugins/xenserver/xenapi/etc/xapi.d/plugins/glance index a281c5c7d7..8f3c16ff18 100755 --- a/plugins/xenserver/xenapi/etc/xapi.d/plugins/glance +++ b/plugins/xenserver/xenapi/etc/xapi.d/plugins/glance @@ -34,7 +34,7 @@ pluginlib_nova.configure_logging('glance') logging = pluginlib_nova.logging PluginError = pluginlib_nova.PluginError -DEFAULT_SOCKET_TIMEOUT_SECONDS = 60 +SOCKET_TIMEOUT_SECONDS = 90 class RetryableError(Exception): @@ -42,10 +42,12 @@ class RetryableError(Exception): def _download_tarball_and_verify(request, staging_path): - # NOTE(johngarbutt) to ensure the script does not hang - # if we lose connection to glance we add a default socket - # The default is to never timeout. - socket.setdefaulttimeout(DEFAULT_SOCKET_TIMEOUT_SECONDS) + # NOTE(johngarbutt) By default, there is no timeout. + # To ensure the script does not hang if we lose connection + # to glance, we add this socket timeout. + # This is here so there is no chance the timeout out has + # been adjusted by other library calls. + socket.setdefaulttimeout(SOCKET_TIMEOUT_SECONDS) try: response = urllib2.urlopen(request) @@ -124,6 +126,13 @@ def _upload_tarball(staging_path, image_id, glance_host, glance_port, Create a tarball of the image and then stream that into Glance using chunked-transfer-encoded HTTP. """ + # NOTE(johngarbutt) By default, there is no timeout. + # To ensure the script does not hang if we lose connection + # to glance, we add this socket timeout. + # This is here so there is no chance the timeout out has + # been adjusted by other library calls. + socket.setdefaulttimeout(SOCKET_TIMEOUT_SECONDS) + if glance_use_ssl: scheme = 'https' else: diff --git a/plugins/xenserver/xenapi/etc/xapi.d/plugins/utils.py b/plugins/xenserver/xenapi/etc/xapi.d/plugins/utils.py index 55b6dece99..349201f8c2 100644 --- a/plugins/xenserver/xenapi/etc/xapi.d/plugins/utils.py +++ b/plugins/xenserver/xenapi/etc/xapi.d/plugins/utils.py @@ -19,6 +19,7 @@ import errno import logging import os import shutil +import signal import subprocess import tempfile @@ -117,6 +118,16 @@ def run_command(cmd, cmd_input=None, ok_exit_codes=None): ok_exit_codes=ok_exit_codes) +def try_kill_process(proc): + """Sends the given process the SIGKILL signal.""" + pid = proc.pid + LOG.info("Killing process %s" % pid) + try: + os.kill(pid, signal.SIGKILL) + except Exception: + LOG.exception("Failed to kill %s" % pid) + + def make_staging_area(sr_path): """The staging area is a place where we can temporarily store and manipulate VHDs. The use of the staging area is different for upload and @@ -378,16 +389,20 @@ def create_tarball(fileobj, path, callback=None, compression_level=None): env["GZIP"] = "-%d" % compression_level tar_proc = make_subprocess(tar_cmd, stdout=True, stderr=True, env=env) - while True: - chunk = tar_proc.stdout.read(CHUNK_SIZE) - if chunk == '': - break + try: + while True: + chunk = tar_proc.stdout.read(CHUNK_SIZE) + if chunk == '': + break - if callback: - callback(chunk) + if callback: + callback(chunk) - if fileobj: - fileobj.write(chunk) + if fileobj: + fileobj.write(chunk) + except Exception: + try_kill_process(tar_proc) + raise finish_subprocess(tar_proc, tar_cmd) @@ -402,15 +417,19 @@ def extract_tarball(fileobj, path, callback=None): tar_cmd = ["tar", "-zx", "--directory=%s" % path] tar_proc = make_subprocess(tar_cmd, stderr=True, stdin=True) - while True: - chunk = fileobj.read(CHUNK_SIZE) - if chunk == '': - break + try: + while True: + chunk = fileobj.read(CHUNK_SIZE) + if chunk == '': + break - if callback: - callback(chunk) + if callback: + callback(chunk) - tar_proc.stdin.write(chunk) + tar_proc.stdin.write(chunk) + except Exception: + try_kill_process(tar_proc) + raise finish_subprocess(tar_proc, tar_cmd)