[Kimchi-devel] [PATCH V1] add a synchronous function with timeout to execute command

[Royce Lv]

On 2014年01月10日 21:34, Sheldon wrote:
> How about just check timeout by ”proc.returncode == -9“, not sure we 
> can kill the subprocess manually.
>
> The follow is an Example:
>
> /import subprocess//
> //from threading import Timer//
> //
> //def run_command(cmd, timeout=None)://
> //    proc = None//
> //    timeout_flag = False//
> //
> //    try://
> //        proc = subprocess.Popen(cmd, stdout=subprocess.PIPE,//
> //                                stderr=subprocess.PIPE)//
> //        if timeout is not None://
> //            timer = Timer(timeout, lambda proc: proc.kill(), [proc])//
> //            timer.start()//
> //
> //        out, error = proc.communicate()//
> //        print "Run command '%s'" % " ".join(cmd)//
> //
> //        if out or error://
> //            print "out:\n %s\nerror:\n %s" % (out, error)//
> //        if proc.returncode == -9://
> //            timeout_flag = True//
> //            print "process is killed by signal.SIGKILL for timeout 
> %s seconds" % timeout//
> //        if timeout is not None://
> //            timer.cancel()//
> //        return out, error, proc.returncode, timeout_flag//
> //    except Exception as e://
> //        msg = "Failed to run command: %s." % " ".join(cmd)//
> //        print "%s\n  %s" % (msg, e)//
> //        return None, None, None, timeout_flag//
> //
> /
> On 01/10/2014 04:16 PM, Royce Lv wrote:
>> On 2014年01月10日 09:00, shaohef at linux.vnet.ibm.com wrote:
>>> From: ShaoHe Feng <shaohef at linux.vnet.ibm.com>
>>>
>>> We need a common function to execute shell command.
>>> We also need timeout when execute shell command.
>>>
>>> A threading.Timer is used to send signal.SIGKILL to kill the command
>>> when timeout.
>>>
>>> Signed-off-by: Royce Lv <lvroyce at linux.vnet.ibm.com>
>>> Signed-off-by: ShaoHe Feng <shaohef at linux.vnet.ibm.com>
>>> ---
>>>   src/kimchi/utils.py | 39 +++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 39 insertions(+)
>>>
>>> diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py
>>> index af245c6..94097ad 100644
>>> --- a/src/kimchi/utils.py
>>> +++ b/src/kimchi/utils.py
>>> @@ -23,6 +23,7 @@
>>>
>>>   import cherrypy
>>>   import os
>>> +import subprocess
>>>   import urllib2
>>>
>>>
>>> @@ -30,6 +31,7 @@ from cherrypy.lib.reprconf import Parser
>>>
>>>
>>>   from kimchi import config
>>> +from threading import Timer
>>>
>>>
>>>   kimchi_log = cherrypy.log.error_log
>>> @@ -96,3 +98,40 @@ def check_url_path(path):
>>>           return False
>>>
>>>       return True
>>> +
>>> +
>>> +def run_command(cmd, timeout=None):
>>> +    def kill_proc(proc, timeout_flag):
>>> +        timeout_flag[0] = True
>>> +        proc.kill()
>>> +
>>> +    proc = None
>>> +    timer = None
>>> +    timeout_flag = [False]
>>> +
>>> +    try:
>>> +        proc = subprocess.Popen(cmd, stdout=subprocess.PIPE,
>>> +                                stderr=subprocess.PIPE)
>>> +        if timeout is not None:
>>> +            timer = Timer(timeout, kill_proc, [proc, timeout_flag])
>>> +            timer.start()
>> I like the idea of using timer, I think the only thing we need to 
>> take care is check timeout happens and report a specific error.
>> Or when we kill the proc and it is blocked, the proc.communicate() 
>> error will not be 'timeout happens'
>>> +
>>> +        out, error = proc.communicate()
>>> +        kimchi_log.debug("Run command '%s'", " ".join(cmd))
>>> +
>>> +        if out or error:
>>> +            kimchi_log.debug("out:\n %s\nerror:\n %s", out, error)
>>> +
>>> +        return out, error, proc.returncode, timeout_flag[0]
>>> +    except Exception as e:
>>> +        msg = "Failed to run command: %s." % " ".join(cmd)
>>> +        msg = msg if proc is None else msg + "\n  error code: %s."
>>> +        kimchi_log.error("%s\n  %s", msg, e)
>>> +
>>> +        if proc:
>>> +            return out, error, proc.returncode, timeout_flag[0]
>>> +        else:
>>> +            return None, None, None, timeout_flag[0]
I like the idea raise Timeout Exception better, because when timeout 
occurs, the ordinary execution of cmd becomes invalid and we do not need 
to check the execute result any more.
And outside of this helper function, I think using:
try:
     run_cmd(cmd)
except TimeoutException:
    ...
Will help to separate the ordinary logic. What do you think?
>>> +    finally:
>>> +        if timer and not timeout_flag[0]:
>>> +            timer.cancel()
>>
>
>
> -- 
> Thanks and best regards!
>
> Sheldon Feng(冯少合)<shaohef at linux.vnet.ibm.com>
> IBM Linux Technology Center

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20140113/d3eba22e/attachment.html>