[Cluster-devel] [PATCH 3/3] fence_amt: option --method and new option --amttool-path

[Fabio M. Di Nitto <fdinitto@redhat.com>]

On 11/29/2013 05:32 PM, Ondrej Mular wrote:
> Add support for option --method and new option --amttool-path
> 
> ---
>  fence/agents/amt/fence_amt.py | 72 ++++++++++++++++++++++++++++++++++---------
>  1 file changed, 57 insertions(+), 15 deletions(-)
> 
> diff --git a/fence/agents/amt/fence_amt.py b/fence/agents/amt/fence_amt.py
> index 8fe2dbc..7077828 100755
> --- a/fence/agents/amt/fence_amt.py
> +++ b/fence/agents/amt/fence_amt.py
> @@ -1,6 +1,6 @@
>  #!/usr/bin/python
>  
> -import sys, subprocess, re
> +import sys, subprocess, re, os, stat
>  from pipes import quote
>  sys.path.append("@FENCEAGENTSLIBDIR@")
>  from fencing import *
> @@ -21,12 +21,11 @@ def get_power_status(_, options):
>      try:
>          process = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True)
>      except OSError:
> -        fail(EC_TOOL_FAIL)
> +        fail_usage("Amttool not found or not accessible")
>  
>      process.wait()
>  
>      output = process.communicate()
> -
>      process.stdout.close()
>  
>      match = re.search('Powerstate:[\\s]*(..)', str(output))
> @@ -51,19 +50,44 @@ def set_power_status(_, options):
>          process = subprocess.Popen(cmd, stdout=null, stderr=null, shell=True)
>      except OSError:
>          null.close()
> -        fail(EC_TOOL_FAIL)
> +        fail_usage("Amttool not found or not accessible")
>  
>      process.wait()
>      null.close()
>  
>      return
>  
> +def reboot_cycle(_, options):
> +    cmd = create_command(options, "cycle")
> +
> +    if options["log"] >= LOG_MODE_VERBOSE:
> +        options["debug_fh"].write("executing: " + cmd + "\n")
> +
> +    null = open('/dev/null', 'w')
> +    try:
> +        process = subprocess.Popen(cmd, stdout=null, stderr=null, shell=True)
> +    except OSError:
> +        null.close()
> +        fail_usage("Amttool not found or not accessible")
> +
> +    status = process.wait()
> +    null.close()
> +
> +    return not bool(status)
> +
> +def is_executable(path):
> +    if os.path.exists(path):
> +        stats = os.stat(path)
> +        if stat.S_ISREG(stats.st_mode) and os.access(path, os.X_OK):
> +            return True
> +    return False
> +
>  def create_command(options, action):
>  
>      # --password / -p
>      cmd = "AMT_PASSWORD=" + quote(options["--password"])
>  
> -    cmd += " " + options["amttool_path"]
> +    cmd += " " + options["--amttool-path"]
>  
>      # --ip / -a
>      cmd += " " + options["--ip"]
> @@ -77,7 +101,10 @@ def create_command(options, action):
>      elif action == "off":
>          cmd = "echo \"y\"|" + cmd
>          cmd += " powerdown"
> -    if action in ["on", "off"] and options.has_key("--boot-options"):
> +    elif action == "cycle":
> +        cmd = "echo \"y\"|" + cmd
> +        cmd += " powercycle"
> +    if action in ["on", "off", "cycle"] and options.has_key("--boot-options"):
>          cmd += options["--boot-options"]
>  
>      # --use-sudo / -d
> @@ -86,25 +113,40 @@ def create_command(options, action):
>  
>      return cmd
>  
> -def main():
> -
> -    atexit.register(atexit_handler)
> -
> -    device_opt = [ "ipaddr", "no_login", "passwd", "boot_option", "no_port", "sudo"]
> -
> +def define_new_opts():
>      all_opt["boot_option"] = {
>          "getopt" : "b:",
>          "longopt" : "boot-option",
> -        "help":"-b, --boot-option=[option] Change the default boot behavior of the machine. (pxe|hd|hdsafe|cd|diag)",
> +        "help":"-b, --boot-option=[option]     Change the default boot behavior of the machine. (pxe|hd|hdsafe|cd|diag)",
>          "required" : "0",
>          "shortdesc" : "Change the default boot behavior of the machine.",
>              "choices" : ["pxe", "hd", "hdsafe", "cd", "diag"],
>            "order" : 1
>          }
> +    all_opt["amttool_path"] = {
> +        "getopt" : "i:",
> +        "longopt" : "amttool-path",
> +        "help" : "--amttool-path=[path]          Path to amttool binary",
> +        "required" : "0",
> +        "shortdesc" : "Path to amttool binary",
> +        "default" : "/usr/bin/amttool",

similar here. Hardcoding paths is bad.

Fabio