diff --git a/plugins/xenserver/xenapi/etc/xapi.d/plugins/bittorrent b/plugins/xenserver/xenapi/etc/xapi.d/plugins/bittorrent index 70c62ec237..111741e607 100755 --- a/plugins/xenserver/xenapi/etc/xapi.d/plugins/bittorrent +++ b/plugins/xenserver/xenapi/etc/xapi.d/plugins/bittorrent @@ -169,12 +169,10 @@ def _seed(torrent_path, seed_cache_path, torrent_seed_duration, torrent_listen_port_start, torrent_listen_port_end): plugin_path = os.path.dirname(inspect.getabsfile(inspect.currentframe())) seeder_path = os.path.join(plugin_path, SEEDER_PROCESS) - seed_cmd = "%s %s %s %d %d %d" % ( - seeder_path, torrent_path, seed_cache_path, torrent_seed_duration, - torrent_listen_port_start, torrent_listen_port_end) - - seed_proc = utils.make_subprocess(seed_cmd) - utils.finish_subprocess(seed_proc, seed_cmd) + seed_cmd = [seeder_path, torrent_path, seed_cache_path, + torrent_seed_duration, torrent_listen_port_start, + torrent_listen_port_end] + utils.run_command(seed_cmd) def _seed_if_needed(seed_cache_path, tarball_path, torrent_path, diff --git a/plugins/xenserver/xenapi/etc/xapi.d/plugins/migration b/plugins/xenserver/xenapi/etc/xapi.d/plugins/migration index 9233aa101f..7906887376 100755 --- a/plugins/xenserver/xenapi/etc/xapi.d/plugins/migration +++ b/plugins/xenserver/xenapi/etc/xapi.d/plugins/migration @@ -33,15 +33,15 @@ def move_vhds_into_sr(session, instance_uuid, sr_path, uuid_stack): def _rsync_vhds(instance_uuid, host, staging_path, user="root"): - ssh_cmd = '\"ssh -o StrictHostKeyChecking=no\"' + ssh_cmd = 'ssh -o StrictHostKeyChecking=no' if not staging_path.endswith('/'): staging_path += '/' dest_path = '%s@%s:/images/instance%s/' % (user, host, instance_uuid) - rsync_cmd = "/usr/bin/rsync -av --progress -e %(ssh_cmd)s "\ - "%(staging_path)s %(dest_path)s" % locals() + rsync_cmd = ["/usr/bin/rsync", "-av", "--progress", "-e", ssh_cmd, + staging_path, dest_path] # NOTE(hillad): rsync's progress is carriage returned, requiring # universal_newlines for real-time output. diff --git a/plugins/xenserver/xenapi/etc/xapi.d/plugins/utils.py b/plugins/xenserver/xenapi/etc/xapi.d/plugins/utils.py index a15ace92af..8404fd961b 100644 --- a/plugins/xenserver/xenapi/etc/xapi.d/plugins/utils.py +++ b/plugins/xenserver/xenapi/etc/xapi.d/plugins/utils.py @@ -18,7 +18,6 @@ import cPickle as pickle import errno import logging import os -import shlex import shutil import subprocess import tempfile @@ -56,37 +55,54 @@ def _rename(src, dst): def make_subprocess(cmdline, stdout=False, stderr=False, stdin=False, - universal_newlines=False): + universal_newlines=False, close_fds=True): """Make a subprocess according to the given command-line string """ - # NOTE(dprince): shlex python 2.4 doesn't like unicode so we - # explicitly convert to ascii - cmdline = cmdline.encode('ascii') - LOG.info("Running cmd '%s'" % cmdline) + LOG.info("Running cmd '%s'" % " ".join(cmdline)) kwargs = {} kwargs['stdout'] = stdout and subprocess.PIPE or None kwargs['stderr'] = stderr and subprocess.PIPE or None kwargs['stdin'] = stdin and subprocess.PIPE or None kwargs['universal_newlines'] = universal_newlines - args = shlex.split(cmdline) - LOG.info("Running args '%s'" % args) - proc = subprocess.Popen(args, **kwargs) + kwargs['close_fds'] = close_fds + proc = subprocess.Popen(cmdline, **kwargs) return proc -def finish_subprocess(proc, cmdline, ok_exit_codes=None): +class SubprocessException(Exception): + def __init__(self, cmdline, ret, out, err): + Exception.__init__(self, "'%s' returned non-zero exit code: " + "retcode=%i, out='%s', stderr='%s'" + % (cmdline, ret, out, err)) + self.cmdline = cmdline + self.ret = ret + self.out = out + self.err = err + + +def finish_subprocess(proc, cmdline, cmd_input=None, ok_exit_codes=None): """Ensure that the process returned a zero exit code indicating success """ if ok_exit_codes is None: ok_exit_codes = [0] + out, err = proc.communicate(cmd_input) - out, err = proc.communicate() ret = proc.returncode if ret not in ok_exit_codes: - raise Exception("'%(cmdline)s' returned non-zero exit code: " - "retcode=%(ret)i, out='%(out)s', stderr='%(err)s'" - % locals()) - return out, err + raise SubprocessException(' '.join(cmdline), ret, out, err) + return out + +def run_command(cmd, cmd_input=None, ok_exit_codes=None): + """Abstracts out the basics of issuing system commands. If the command + returns anything in stderr, an exception is raised with that information. + Otherwise, the output from stdout is returned. + + cmd_input is passed to the process on standard input. + """ + proc = make_subprocess(cmd, stdout=True, stderr=True, stdin=True, + close_fds=True) + return finish_subprocess(proc, cmd, cmd_input=cmd_input, + ok_exit_codes=ok_exit_codes) def make_staging_area(sr_path): @@ -165,9 +181,8 @@ def _assert_vhd_not_hidden(path): If this flag is incorrectly set, then when we move the VHD into the SR, it will be deleted out from under us. """ - query_cmd = "vhd-util query -n %(path)s -f" % locals() - query_proc = make_subprocess(query_cmd, stdout=True, stderr=True) - out, err = finish_subprocess(query_proc, query_cmd) + query_cmd = ["vhd-util", "query", "-n", path, "-f"] + out = run_command(query_cmd) for line in out.splitlines(): if line.lower().startswith('hidden'): @@ -190,10 +205,8 @@ def _validate_vhd(vdi_path): Dom0's are out-of-sync. This would corrupt the SR if it were imported, so generate an exception to bail. """ - check_cmd = "vhd-util check -n %(vdi_path)s -p" % locals() - check_proc = make_subprocess(check_cmd, stdout=True, stderr=True) - out, err = finish_subprocess( - check_proc, check_cmd, ok_exit_codes=[0, 22]) + check_cmd = ["vhd-util", "check", "-n", vdi_path, "-p"] + out = run_command(check_cmd, ok_exit_codes=[0, 22]) first_line = out.splitlines()[0].strip() if 'invalid' in first_line: @@ -230,10 +243,8 @@ def _validate_vdi_chain(vdi_path): failures. """ def get_parent_path(path): - query_cmd = "vhd-util query -n %(path)s -p" % locals() - query_proc = make_subprocess(query_cmd, stdout=True, stderr=True) - out, err = finish_subprocess( - query_proc, query_cmd, ok_exit_codes=[0, 22]) + query_cmd = ["vhd-util", "query", "-n", path, "-p"] + out = run_command(query_cmd, ok_exit_codes=[0, 22]) first_line = out.splitlines()[0].strip() if first_line.endswith(".vhd"): @@ -316,10 +327,9 @@ def import_vhds(sr_path, staging_path, uuid_stack): for vhd_path in reversed(files_to_move): if parent_path: # Link to parent - modify_cmd = ("vhd-util modify -n %(vhd_path)s" - " -p %(parent_path)s" % locals()) - modify_proc = make_subprocess(modify_cmd, stderr=True) - finish_subprocess(modify_proc, modify_cmd) + modify_cmd = ["vhd-util", "modify", "-n", vhd_path, + "-p", parent_path] + run_command(modify_cmd) parent_path = vhd_path @@ -353,7 +363,7 @@ def create_tarball(fileobj, path, callback=None): :param path: path to create tarball from :param callback: optional callback to call on each chunk written """ - tar_cmd = "tar -zc --directory=%(path)s ." % locals() + tar_cmd = ["tar", "-zc", "--directory=%s" % path, "."] tar_proc = make_subprocess(tar_cmd, stdout=True, stderr=True) while True: @@ -377,7 +387,7 @@ def extract_tarball(fileobj, path, callback=None): :param path: path to extract tarball into :param callback: optional callback to call on each chunk read """ - tar_cmd = "tar -zx --directory=%(path)s" % locals() + tar_cmd = ["tar", "-zx", "--directory=%s" % path] tar_proc = make_subprocess(tar_cmd, stderr=True, stdin=True) while True: diff --git a/plugins/xenserver/xenapi/etc/xapi.d/plugins/xenhost b/plugins/xenserver/xenapi/etc/xapi.d/plugins/xenhost index 9f61b79a90..ea446d6559 100755 --- a/plugins/xenserver/xenapi/etc/xapi.d/plugins/xenhost +++ b/plugins/xenserver/xenapi/etc/xapi.d/plugins/xenhost @@ -26,13 +26,11 @@ try: except ImportError: import simplejson as json import logging -import os -import random import re -import subprocess -import tempfile import time +import utils + import pluginlib_nova as pluginlib import XenAPI import XenAPIPlugin @@ -56,48 +54,13 @@ class TimeoutError(StandardError): pass -def _run_command(cmd): - """Abstracts out the basics of issuing system commands. If the command - returns anything in stderr, a PluginError is raised with that information. - Otherwise, the output from stdout is returned. +def _run_command(cmd, cmd_input=None): + """Wrap utils.run_command to raise PluginError on failure """ - pipe = subprocess.PIPE - proc = subprocess.Popen([cmd], shell=True, stdin=pipe, stdout=pipe, - stderr=pipe, close_fds=True) - proc.wait() - err = proc.stderr.read() - if err: - raise pluginlib.PluginError(err) - return proc.stdout.read() - - -# NOTE (salvatore-orlando): -# Instead of updating run_command a new method has been implemented, -# in order to avoid risking breaking existing functions calling _run_command -def _run_command_with_input(cmd, process_input): - """Abstracts out the basics of issuing system commands. If the command - returns anything in stderr, a PluginError is raised with that information. - Otherwise, the output from stdout is returned. - - process_input specificies a variable to use as the process' standard input. - """ - pipe = subprocess.PIPE - # cmd can be either a single string with command and arguments, - # or a sequence of string - if not hasattr(cmd, '__iter__'): - cmd = [cmd] # make it iterable - - #Note(salvatore-orlando): the shell argument has been set to False - proc = subprocess.Popen(cmd, shell=False, stdin=pipe, stdout=pipe, - stderr=pipe, close_fds=True) - if process_input is not None: - (output, err) = proc.communicate(process_input) - else: - (output, err) = proc.communicate() - if err: - raise pluginlib.PluginError(err) - # This is tantamount to proc.stdout.read() - return output + try: + return utils.run_command(cmd, cmd_input=cmd_input) + except utils.SubprocessException, e: + raise pluginlib.PluginError(e.err) def _resume_compute(session, compute_ref, compute_uuid): @@ -111,7 +74,7 @@ def _resume_compute(session, compute_ref, compute_uuid): # join has been successful, wait for xapi to become alive again for c in xrange(0, DEFAULT_TRIES): try: - _run_command("xe vm-start uuid=%s" % compute_uuid) + _run_command(["xe", "vm-start", "uuid=%s" % compute_uuid]) return except pluginlib.PluginError, e: logging.exception('Waited %d seconds for the slave to ' @@ -134,19 +97,17 @@ def set_host_enabled(self, arg_dict): host_uuid = arg_dict['host_uuid'] if enabled == "true": - result = _run_command("xe host-enable uuid=%s" % host_uuid) + result = _run_command(["xe", "host-enable", "uuid=%s" % host_uuid]) elif enabled == "false": - result = _run_command("xe host-disable uuid=%s" % host_uuid) + result = _run_command(["xe", "host-disable", "uuid=%s" % host_uuid]) else: raise pluginlib.PluginError(_("Illegal enabled status: %s") % enabled) # Should be empty string if result: raise pluginlib.PluginError(result) # Return the current enabled status - cmd = "xe host-param-list uuid=%s | grep enabled" % host_uuid - resp = _run_command(cmd) - # Response should be in the format: "enabled ( RO): true" - host_enabled = resp.strip().split()[-1] + cmd = ["xe", "host-param-get", "uuid=%s" % host_uuid, "param-name=enabled"] + host_enabled = _run_command(cmd) if host_enabled == "true": status = "enabled" else: @@ -231,7 +192,7 @@ def iptables_config(session, args): 'iptables-restore', 'ip6tables-save', 'ip6tables-restore'): - result = _run_command_with_input(cmd, process_input) + result = _run_command(cmd, process_input) ret_str = json.dumps(dict(out=result, err='')) logging.debug("iptables_config:exit") @@ -244,18 +205,18 @@ def iptables_config(session, args): def _power_action(action, arg_dict): # Host must be disabled first host_uuid = arg_dict['host_uuid'] - result = _run_command("xe host-disable uuid=%s" % host_uuid) + result = _run_command(["xe", "host-disable", "uuid=%s" % host_uuid]) if result: raise pluginlib.PluginError(result) # All running VMs must be shutdown - result = _run_command("xe vm-shutdown --multiple " - "resident-on=%s" % host_uuid) + result = _run_command(["xe", "vm-shutdown", "--multiple", + "resident-on=%s" % host_uuid]) if result: raise pluginlib.PluginError(result) - cmds = {"reboot": "xe host-reboot uuid=%s", - "startup": "xe host-power-on uuid=%s", - "shutdown": "xe host-shutdown uuid=%s"} - result = _run_command(cmds[action] % host_uuid) + cmds = {"reboot": "host-reboot", + "startup": "host-power-on", + "shutdown": "host-shutdown",} + result = _run_command(["xe", cmds[action], "uuid=%s" % host_uuid]) # Should be empty string if result: raise pluginlib.PluginError(result) @@ -316,7 +277,7 @@ def host_data(self, arg_dict): information. """ host_uuid = arg_dict['host_uuid'] - resp = _run_command("xe host-param-list uuid=%s" % host_uuid) + resp = _run_command(["xe", "host-param-list", "uuid=%s" % host_uuid]) parsed_data = parse_response(resp) # We have the raw dict of values. Extract those that we need, # and convert the data types as needed. @@ -345,7 +306,7 @@ def parse_response(resp): @jsonify def host_uptime(self, arg_dict): """Returns the result of the uptime command on the xenhost.""" - return {"uptime": _run_command('uptime')} + return {"uptime": _run_command(['uptime'])} def cleanup(dct): diff --git a/plugins/xenserver/xenapi/etc/xapi.d/plugins/xenstore.py b/plugins/xenserver/xenapi/etc/xapi.d/plugins/xenstore.py index a12704248d..d7eadfe233 100755 --- a/plugins/xenserver/xenapi/etc/xapi.d/plugins/xenstore.py +++ b/plugins/xenserver/xenapi/etc/xapi.d/plugins/xenstore.py @@ -27,9 +27,7 @@ try: except ImportError: import simplejson as json -import logging -import os -import subprocess +import utils import XenAPIPlugin @@ -69,14 +67,14 @@ def _record_exists(arg_dict): is determined from the given path and dom_id in the arg_dict.""" cmd = ["xenstore-exists", "/local/domain/%(dom_id)s/%(path)s" % arg_dict] try: - ret, result = _run_command(cmd) + _run_command(cmd) + return True except XenstoreError, e: if e.stderr == '': # if stderr was empty, this just means the path did not exist return False # otherwise there was a real problem raise - return True @jsonify @@ -89,7 +87,7 @@ def read_record(self, arg_dict): """ cmd = ["xenstore-read", "/local/domain/%(dom_id)s/%(path)s" % arg_dict] try: - ret, result = _run_command(cmd) + result = _run_command(cmd) return result.strip() except XenstoreError, e: if not arg_dict.get("ignore_missing_path", False): @@ -99,7 +97,7 @@ def read_record(self, arg_dict): # Just try again in case the agent write won the race against # the record_exists check. If this fails again, it will likely raise # an equally meaningful XenstoreError as the one we just caught - ret, result = _run_command(cmd) + result = _run_command(cmd) return result.strip() @@ -128,14 +126,14 @@ def list_records(self, arg_dict): dirpath = "/local/domain/%(dom_id)s/%(path)s" % arg_dict cmd = ["xenstore-ls", dirpath.rstrip("/")] try: - ret, recs = _run_command(cmd) + recs = _run_command(cmd) except XenstoreError, e: if not _record_exists(arg_dict): return {} # Just try again in case the path was created in between # the "ls" and the existence check. If this fails again, it will # likely raise an equally meaningful XenstoreError - ret, recs = _run_command(cmd) + recs = _run_command(cmd) base_path = arg_dict["path"] paths = _paths_from_ls(recs) ret = {} @@ -160,13 +158,12 @@ def delete_record(self, arg_dict): """ cmd = ["xenstore-rm", "/local/domain/%(dom_id)s/%(path)s" % arg_dict] try: - ret, result = _run_command(cmd) + return _run_command(cmd) except XenstoreError, e: if 'could not remove path' in e.stderr: # Entry already gone. We're good to go. return '' raise - return result def _paths_from_ls(recs): @@ -174,7 +171,6 @@ def _paths_from_ls(recs): useful. This method cleans that up into a dict with each path as the key, and the associated string as the value. """ - ret = {} last_nm = "" level = 0 path = [] @@ -203,19 +199,12 @@ def _paths_from_ls(recs): def _run_command(cmd): - """Abstracts out the basics of issuing system commands. If the command - returns anything in stderr, a PluginError is raised with that information. - Otherwise, a tuple of (return code, stdout data) is returned. + """Wrap utils.run_command to raise XenstoreError on failure """ - logging.info(' '.join(cmd)) - pipe = subprocess.PIPE - proc = subprocess.Popen(cmd, stdin=pipe, stdout=pipe, stderr=pipe, - close_fds=True) - out, err = proc.communicate() - if proc.returncode is not os.EX_OK: - raise XenstoreError(cmd, proc.returncode, err, out) - return proc.returncode, out - + try: + return utils.run_command(cmd) + except utils.SubprocessException, e: + raise XenstoreError(e.cmdline, e.ret, e.err, e.out) if __name__ == "__main__": XenAPIPlugin.dispatch(