As described before, this patch should be : --- utils/utils.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/utils/utils.py b/utils/utils.py index 147c1ef..ec09c33 100644 --- a/utils/utils.py +++ b/utils/utils.py @@ -409,9 +409,8 @@ def exec_cmd(command, sudo=False, cwd=None, infile=None, outfile=None, shell=Fal p = subprocess.Popen(command, shell=shell, close_fds=True, cwd=cwd, stdin=infile, stdout=outfile, stderr=subprocess.PIPE) (out, err) = p.communicate(data) - if out == None: - # Prevent splitlines() from barfing later on - out = "" + if out == "": + out = err return (p.returncode, out.splitlines()) def remote_exec_pexpect(hostname, username, password, cmd): -- 1.8.3.1 ----- Original Message -----

----- Original Message -----

...

The 'out' returned from exec_cmds is not 'None' type if stderr occurs, it is because utils pass 'subprocess.PIPE' to stdout to open this subprocess.

"Similarly, to get anything other than None in the result tuple, you need to give stdout=PIPE and/or stderr=PIPE too." (see http://docs.python.org/2/library/subprocess.html#module-subprocess)

Finally, make out equals to err when stderr happened. --- utils/utils.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/utils/utils.py b/utils/utils.py index 147c1ef..2248ce1 100644 --- a/utils/utils.py +++ b/utils/utils.py @@ -404,14 +404,12 @@ def exec_cmd(command, sudo=False, cwd=None, infile=None, outfile=None, shell=Fal command = ["sudo"] + command if infile == None: infile = subprocess.PIPE - if outfile == None: - outfile = subprocess.PIPE

hmm, there is a little problem: if outfile is not subprocess.PIPE, exec_cmd would not capture the output of command. So I think the best choice is check whether 'out' is a null string, rather than a None type.

...

p = subprocess.Popen(command, shell=shell, close_fds=True, cwd=cwd, stdin=infile, stdout=outfile, stderr=subprocess.PIPE) (out, err) = p.communicate(data) if out == None: - # Prevent splitlines() from barfing later on - out = "" + # Because stderr is PIPE, err will not be None, and can be splitlines. + out = err return (p.returncode, out.splitlines())

def remote_exec_pexpect(hostname, username, password, cmd): -- 1.8.3.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On 2013年11月26日 09:32, Jincheng Miao wrote:

ping gren

----- Original Message -----

...

As described before, this patch should be :

--- utils/utils.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/utils/utils.py b/utils/utils.py index 147c1ef..ec09c33 100644 --- a/utils/utils.py +++ b/utils/utils.py @@ -409,9 +409,8 @@ def exec_cmd(command, sudo=False, cwd=None, infile=None, outfile=None, shell=Fal p = subprocess.Popen(command, shell=shell, close_fds=True, cwd=cwd, stdin=infile, stdout=outfile, stderr=subprocess.PIPE) (out, err) = p.communicate(data) - if out == None: - # Prevent splitlines() from barfing later on - out = "" + if out == "": + out = err return (p.returncode, out.splitlines())

def remote_exec_pexpect(hostname, username, password, cmd): -- 1.8.3.1

Sorry I missed your patch. The p.returncode can indicate the result of executing command unless you want the standard error. The subprocess.PIPE can ensure the variable out is always string type, but if the stdout is not the subprocess.PIPE, the variable out possibly be the type of None. so I think it is necessary to use the following code if out == None: out = "" If you want to get standard error in the case of executing command failure. we need to consider other way. Guannan

On 2013年11月26日 17:17, Jincheng Miao wrote:

Thanks for review, yes, I missed this situation: stdout is not the subprocess.PIPE.

Since the stderr is always subprocess.PIPE, my another way is err after Popen.communicate().

The patch looks like: --- utils/utils.py | 2 ++--- 1 file changed, 2 insertions(+)

diff --git a/utils/utils.py b/utils/utils.py index 147c1ef..d107cbd 100644 --- a/utils/utils.py +++ b/utils/utils.py @@ -412,6 +412,8 @@ def exec_cmd(command, sudo=False, cwd=None, infile=None, outfile=None, shell=Fal p = subprocess.Popen(command, shell=shell, close_fds=True, cwd=cwd, stdin=infile, stdout=outfile, stderr=subprocess.PIPE) (out, err) = p.communicate(data) if out == None: # Prevent splitlines() from barfing later on out = "" + if err != "": + out += err return (p.returncode, out.splitlines())

def remote_exec_pexpect(hostname, username, password, cmd):

Why to append standard error to standard output. It is not right in semantics. In order to get the standard error if executing command failed, the following change is enough: if out == None: # Prevent splitlines() from barfing later on out = "" + if p.returncode: + out = err return (p.returncode, out.splitlines()) Guannan