Re: [PATCH v2] tuna: Rework exception handling of invalid policy

The implementation of the error handling block in tuna.get_policy_and_rtprio() caused the exceptions raised by tuna_sched.Policy to be consumed by the finally block. When an invalid policy is passed via the '--priority' flag, this consumption of the exception causes tuna to fail silently. Additionally, in tuna-cmd.py, if a thread list is passed along with the '--priority' flag, tuna.get_policy_and_rtprio() is called twice, first directly, and then again by tuna.threads_set_priority(). The expectation is that tuna.threads_set_priorty will handle exceptions raised by tuna.get_policy_and_rtprio(). This results in a failure to handle exceptions that are raised by the initial direct call to tuna.get_policy_and_rtprio(). If no exceptions are raised, a duplicate call to this function is still unnecessary. Remove the finally block in tuna.get_policy_and_rtprio() and move the error handling so that exceptions are handled directly within the function, rather than in various locations outside the function. Let tuna.threads_set_priority() return the tuple (policy, rtprio) so that it may be called in tuna-cmd.py without a seperate prior call to tuna.get_policy_and_rtprio(). Small fix to differentiate between unsupported policy and invalid policy or priority. Signed-off-by: Leah Leshchinsky <lleshchi(a)redhat.com&gt;

diff --git a/tuna-cmd.py b/tuna-cmd.py index cf117ec046ff..88ff626766a0 100755 --- a/tuna-cmd.py +++ b/tuna-cmd.py @@ -601,13 +601,13 @@ def main(): tuna.include_cpus(cpu_list, get_nr_cpus()) elif o in ("-p", "--priority"):

# Save policy and rtprio for future Actions (e.g. --run).

- (policy, rtprio) = tuna.get_policy_and_rtprio(a) if not thread_list: # For backward compatibility p_waiting_action = True

+ (policy, rtprio) = tuna.get_policy_and_rtprio(a) else: try: - tuna.threads_set_priority(thread_list, a, affect_children) + (policy, rtprio) = tuna.threads_set_priority(thread_list, a, affect_children) except OSError as err: print("tuna: %s" % err) sys.exit(2) diff --git a/tuna/tuna.py b/tuna/tuna.py index 8fb42121e2e4..7b0cc1f3fe30 100755 --- a/tuna/tuna.py +++ b/tuna/tuna.py @@ -507,21 +507,20 @@ def get_policy_and_rtprio(parm): rtprio = 0 policy = None - try: - cp = tuna_sched.Policy(parms[0]) - except: - if parms[0].isdigit(): - rtprio = int(parms[0]) - else: - raise ValueError

+ if parms[0].isdigit(): + rtprio = int(parms[0]) else: - policy = cp.get_policy() - if len(parms) > 1: - rtprio = int(parms[1]) - elif cp.is_rt(): - rtprio = 1 - finally: - return(policy, rtprio)

+ try: + cp = tuna_sched.Policy(parms[0]) + policy = cp.get_policy() + if len(parms) > 1: + rtprio = int(parms[1]) + elif cp.is_rt(): + rtprio = 1

+ except ValueError as err: + print(err) + sys.exit(2) + return (policy, rtprio) def thread_filtered(tid, cpus_filtered, show_kthreads, show_uthreads): if cpus_filtered: @@ -558,11 +557,8 @@ def thread_set_priority(tid, policy, rtprio): os.sched_setscheduler(tid, policy, param) def threads_set_priority(tids, parm, affect_children=False): - try: - (policy, rtprio) = get_policy_and_rtprio(parm) - except ValueError: - print("tuna: " + _("\"%s\" is unsupported priority value!") % parms[0]) - return + + (policy, rtprio) = get_policy_and_rtprio(parm) for tid in tids: try: @@ -580,6 +576,7 @@ def threads_set_priority(tids, parm, affect_children=False): if err.args[0] == errno.ESRCH: continue raise err + return (policy, rtprio) class sched_tunings: def __init__(self, name, pid, policy, rtprio, affinity, percpu): diff --git a/tuna/tuna_sched.py b/tuna/tuna_sched.py index de9846bb5fae..8c13b914d13a 100644 --- a/tuna/tuna_sched.py +++ b/tuna/tuna_sched.py @@ -51,12 +51,12 @@ class Policy: can use fifo, FIFO, sched_fifo, SCHED_FIFO, etc """ self.policy = None - policy = policy.upper() - if policy[:6] != "SCHED_": - policy = "SCHED_" + policy - if policy not in list(Policy.SCHED_POLICIES): - raise OSError - self.policy = policy

+ policy_str = policy.upper() + if policy_str[:6] != "SCHED_": + policy_str = "SCHED_" + policy_str + if policy_str not in list(Policy.SCHED_POLICIES):

+ raise ValueError("tuna: " + _("\"%s\" is an unsupported priority value!") % policy) + self.policy = policy_str @classmethod def num_init(cls, policy): -- 2.27.0