From: "Brian C. Lane" bcl@redhat.com
Changes to the execWith* methods removed the ability to handle a string as stdin/out/err. Change the callers to first open where they want output to be sent to and pass that instead. Only stdout is needed because _run_program combines stdout and stderr, pass stderr through but ignore it.
Also clean up a couple places that were passing strings that don't need to. --- anaconda | 3 +-- pyanaconda/iutil.py | 61 ++++++++++++++++++++++++++++++------------------- pyanaconda/kickstart.py | 35 +++++++++++++--------------- pyanaconda/network.py | 3 +-- 4 files changed, 55 insertions(+), 47 deletions(-)
diff --git a/anaconda b/anaconda index 324d31e..06773ca 100755 --- a/anaconda +++ b/anaconda @@ -85,8 +85,7 @@ def startMetacityWM(): try: args = ['--display', ':1', '--sm-disable'] - iutil.execWithRedirect('metacity', args, - stdout='/dev/null', stderr='/dev/null') + iutil.execWithRedirect('metacity', args) except BaseException as e: # catch all possible exceptions log.error("Problems running the window manager: %s" % str(e)) diff --git a/pyanaconda/iutil.py b/pyanaconda/iutil.py index dbc7c3b..2d2b08c 100644 --- a/pyanaconda/iutil.py +++ b/pyanaconda/iutil.py @@ -49,7 +49,15 @@ def augmentEnv(): }) return env
-def _run_program(argv, root='/', stdin=None, env_prune=None): +def _run_program(argv, root='/', stdin=None, stdout=None, env_prune=None): + """ Run an external program, log the output and return it to the caller + @param argv The command to run and argument + @param root The directory to chroot to before running command. + @param stdin The file descriptor to read stdin from. + @param stdout Optional file descriptor to write stdout and stderr to. + @param env_prune environment variable to remove before execution + @return The return code of the command and the output + """ if env_prune is None: env_prune = []
@@ -75,6 +83,8 @@ def _run_program(argv, root='/', stdin=None, env_prune=None): if out: for line in out.splitlines(): program_log.info(line) + if stdout: + stdout.write(line)
except OSError as e: program_log.error("Error running %s: %s" % (argv[0], e.strerror)) @@ -84,41 +94,44 @@ def _run_program(argv, root='/', stdin=None, env_prune=None):
return (proc.returncode, out)
-## Run an external program and redirect the output to a file. -# @param command The command to run. -# @param argv A list of arguments. -# @param stdin The file descriptor to read stdin from. -# @param stdout The file descriptor to redirect stdout to. -# @param stderr The file descriptor to redirect stderr to. -# @param root The directory to chroot to before running command. -# @return The return code of command. -def execWithRedirect(command, argv, stdin = None, stdout = None, - stderr = None, root = '/', env_prune=[]): +def execWithRedirect(command, argv, stdin=None, stdout=None, + stderr=None, root='/', env_prune=[]): + """ Run an external program and redirect the output to a file. + @param command The command to run + @param argv The argument list + @param stdin The file descriptor to read stdin from. + @param stdout Optional file descriptor to redirect stdout and stderr to. + @param stderr not used + @param root The directory to chroot to before running command. + @param env_prune environment variable to remove before execution + @return The return code of the command + """ if flags.testing: log.info("not running command because we're testing: %s %s" % (command, " ".join(argv))) return 0
argv = [command] + argv - return _run_program(argv, stdin=stdin, root=root, env_prune=env_prune)[0] - -## Run an external program and capture standard out. -# @param command The command to run. -# @param argv A list of arguments. -# @param stdin The file descriptor to read stdin from. -# @param stderr The file descriptor to redirect stderr to. -# @param root The directory to chroot to before running command. -# @param fatal Boolean to determine if non-zero exit is fatal. -# @return The output of command from stdout. -def execWithCapture(command, argv, stdin = None, stderr = None, root='/', - fatal = False): + return _run_program(argv, stdin=stdin, stdout=stdout, root=root, env_prune=env_prune)[0] + +def execWithCapture(command, argv, stdin=None, stderr=None, root='/', + fatal=False): + """ Run an external program and capture standard out and err. + @param command The command to run + @param argv The argument list + @param stdin The file descriptor to read stdin from. + @param stderr not used + @param root The directory to chroot to before running command. + @param fatal not used + @return The output of the command + """ if flags.testing: log.info("not running command because we're testing: %s %s" % (command, " ".join(argv))) return ""
argv = [command] + argv - return _run_program(argv, stdin=stdin, root=root)[1] + return _run_program(argv, stdin=stdin, stdout=stdout, root=root)[1]
## Run a shell. def execConsole(): diff --git a/pyanaconda/kickstart.py b/pyanaconda/kickstart.py index a93c552..35f90e2 100644 --- a/pyanaconda/kickstart.py +++ b/pyanaconda/kickstart.py @@ -81,7 +81,17 @@ packagesSeen = False topology = None
class AnacondaKSScript(KSScript): + """ Execute a kickstart script + + This will write the script to a file named /tmp/ks-script- before + execution. + Output is logged by the program logger, the path specified by --log + or to /tmp/ks-script-*.log + """ def run(self, chroot): + """ Run the kickstart script + @param chroot directory path to chroot into before execution + """ if self.inChroot: scriptRoot = chroot else: @@ -93,9 +103,9 @@ class AnacondaKSScript(KSScript): os.close(fd) os.chmod(path, 0700)
- # Always log stdout/stderr from scripts. Using --logfile just lets you + # Always log stdout/stderr from scripts. Using --log just lets you # pick where it goes. The script will also be logged to program.log - # because of execWithRedirect, and to anaconda.log if the script fails. + # because of execWithRedirect. if self.logfile: if self.inChroot: messages = "%s/%s" % (scriptRoot, self.logfile) @@ -110,26 +120,13 @@ class AnacondaKSScript(KSScript): # chroot later. messages = "/tmp/%s.log" % os.path.basename(path)
- rc = iutil.execWithRedirect(self.interp, ["/tmp/%s" % os.path.basename(path)], - stdin = messages, stdout = messages, stderr = messages, - root = scriptRoot) + with open(messages, "w") as fp: + rc = iutil.execWithRedirect(self.interp, ["/tmp/%s" % os.path.basename(path)], + stdin=fp, stdout=fp, + root = scriptRoot)
- # Always log an error. Only fail if we have a handle on the - # windowing system and the kickstart file included --erroronfail. if rc != 0: log.error("Error code %s running the kickstart script at line %s" % (rc, self.lineno)) - - if os.path.isfile(messages): - try: - f = open(messages, "r") - except IOError as e: - err = None - else: - err = f.readlines() - f.close() - for l in err: - log.error("\t%s" % l) - if self.errorOnFail: errorHandler.cb(ScriptError(), self.lineno, err) sys.exit(0) diff --git a/pyanaconda/network.py b/pyanaconda/network.py index 294f937..0d352d2 100644 --- a/pyanaconda/network.py +++ b/pyanaconda/network.py @@ -836,8 +836,7 @@ def set_hostname(hn): return
log.info("setting installation environment hostname to %s" % hn) - iutil.execWithRedirect("hostname", ["-v", hn ], - stdout="/dev/tty5", stderr="/dev/tty5") + iutil.execWithRedirect("hostname", ["-v", hn ])
def get_NM_settings_value(iface, key1, key2): value = ""
From: "Brian C. Lane" bcl@redhat.com
--- dracut/parse-kickstart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/dracut/parse-kickstart b/dracut/parse-kickstart index afca075..fcf847a 100755 --- a/dracut/parse-kickstart +++ b/dracut/parse-kickstart @@ -269,8 +269,8 @@ def ksnet_to_ifcfg(net, filename=None): os.makedirs("/tmp/ifcfg") ifcfg['DEVICE'] = dev ifcfg['HWADDR'] = readsysfile("/sys/class/net/%s/address" % dev) - uuid = str(uuid.uuid4()) - ifcfg['UUID'] = uuid + if_uuid = str(uuid.uuid4()) + ifcfg['UUID'] = if_uuid # we set real ONBOOT value in anaconda, here # we use it to activate devcies by NM on start ifcfg['ONBOOT'] = "yes" if net.activate else "no" @@ -313,7 +313,7 @@ def ksnet_to_ifcfg(net, filename=None): if net.bootProto == 'dhcp': srcpath = "/tmp/dhclient.%s.lease" % dev dstdir = "/tmp/ifcfg-leases" - dstpath = "%s/dhclient-%s-%s.lease" % (dstdir, uuid, dev) + dstpath = "%s/dhclient-%s-%s.lease" % (dstdir, if_uuid, dev) if os.path.exists(srcpath): if not os.path.isdir(dstdir): os.makedirs(dstdir)
On Wed, 2013-02-13 at 14:21 -0800, Brian C. Lane wrote:
From: "Brian C. Lane" bcl@redhat.com
Changes to the execWith* methods removed the ability to handle a string as stdin/out/err. Change the callers to first open where they want output to be sent to and pass that instead. Only stdout is needed because _run_program combines stdout and stderr, pass stderr through but ignore it.
Also clean up a couple places that were passing strings that don't need to.
anaconda | 3 +-- pyanaconda/iutil.py | 61 ++++++++++++++++++++++++++++++------------------- pyanaconda/kickstart.py | 35 +++++++++++++--------------- pyanaconda/network.py | 3 +-- 4 files changed, 55 insertions(+), 47 deletions(-)
diff --git a/anaconda b/anaconda index 324d31e..06773ca 100755 --- a/anaconda +++ b/anaconda @@ -85,8 +85,7 @@ def startMetacityWM(): try: args = ['--display', ':1', '--sm-disable']
iutil.execWithRedirect('metacity', args,stdout='/dev/null', stderr='/dev/null')
iutil.execWithRedirect('metacity', args) except BaseException as e: # catch all possible exceptions log.error("Problems running the window manager: %s" % str(e))diff --git a/pyanaconda/iutil.py b/pyanaconda/iutil.py index dbc7c3b..2d2b08c 100644 --- a/pyanaconda/iutil.py +++ b/pyanaconda/iutil.py @@ -49,7 +49,15 @@ def augmentEnv(): }) return env
-def _run_program(argv, root='/', stdin=None, env_prune=None): +def _run_program(argv, root='/', stdin=None, stdout=None, env_prune=None):
- """ Run an external program, log the output and return it to the caller
@param argv The command to run and argument@param root The directory to chroot to before running command.@param stdin The file descriptor to read stdin from.@param stdout Optional file descriptor to write stdout and stderr to.
These should be both 'file object' instead of 'file descriptor'. The same goes for the other doc strings as well.
Otherwise these patches both look good to me. My only concern is about using proc.communicate() instead of the 'tee' class. It is a story much older than my history in the team but I'm quite sure there was a reason for writing and using that class. Do we support e.g. interactive scripts? Or don't we want output of the program logged even before the program exits so that it can be easily seen, that it e.g. expects input? Are we going to use _run_program for something with really long output? Because in all those cases proc.communicate() is not a good idea.
On Thu, Feb 14, 2013 at 10:25:09AM +0100, Vratislav Podzimek wrote:
On Wed, 2013-02-13 at 14:21 -0800, Brian C. Lane wrote:
From: "Brian C. Lane" bcl@redhat.com
Changes to the execWith* methods removed the ability to handle a string as stdin/out/err. Change the callers to first open where they want output to be sent to and pass that instead. Only stdout is needed because _run_program combines stdout and stderr, pass stderr through but ignore it.
Also clean up a couple places that were passing strings that don't need to.
anaconda | 3 +-- pyanaconda/iutil.py | 61 ++++++++++++++++++++++++++++++------------------- pyanaconda/kickstart.py | 35 +++++++++++++--------------- pyanaconda/network.py | 3 +-- 4 files changed, 55 insertions(+), 47 deletions(-)
diff --git a/anaconda b/anaconda index 324d31e..06773ca 100755 --- a/anaconda +++ b/anaconda @@ -85,8 +85,7 @@ def startMetacityWM(): try: args = ['--display', ':1', '--sm-disable']
iutil.execWithRedirect('metacity', args,stdout='/dev/null', stderr='/dev/null')
iutil.execWithRedirect('metacity', args) except BaseException as e: # catch all possible exceptions log.error("Problems running the window manager: %s" % str(e))diff --git a/pyanaconda/iutil.py b/pyanaconda/iutil.py index dbc7c3b..2d2b08c 100644 --- a/pyanaconda/iutil.py +++ b/pyanaconda/iutil.py @@ -49,7 +49,15 @@ def augmentEnv(): }) return env
-def _run_program(argv, root='/', stdin=None, env_prune=None): +def _run_program(argv, root='/', stdin=None, stdout=None, env_prune=None):
- """ Run an external program, log the output and return it to the caller
@param argv The command to run and argument@param root The directory to chroot to before running command.@param stdin The file descriptor to read stdin from.@param stdout Optional file descriptor to write stdout and stderr to.These should be both 'file object' instead of 'file descriptor'. The same goes for the other doc strings as well.
Thanks.
Otherwise these patches both look good to me. My only concern is about using proc.communicate() instead of the 'tee' class. It is a story much older than my history in the team but I'm quite sure there was a reason for writing and using that class. Do we support e.g. interactive scripts? Or don't we want output of the program logged even before the program exits so that it can be easily seen, that it e.g. expects input? Are we going to use _run_program for something with really long output? Because in all those cases proc.communicate() is not a good idea.
Well, this patch doesn't change that :) But I think it was related to wanting to log things to /dev/tty5 as well as program.log and dlehman fixed that by adding it to the program_logger, which is a pretty elegant solution.
anaconda-patches@lists.fedorahosted.org
-
Brian C. Lane -
Vratislav Podzimek