From 4241a30236232974e478282d6e48bca235bf1521 Mon Sep 17 00:00:00 2001 From: Valentin Schneider Date: Tue, 5 Feb 2019 18:18:42 +0000 Subject: [PATCH 1/6] target_script: Split foreground & background logic While at it, cleanup the code and add some more doc. --- lisa/target_script.py | 102 +++++++++++++++++++++++++++--------------- 1 file changed, 67 insertions(+), 35 deletions(-) diff --git a/lisa/target_script.py b/lisa/target_script.py index fcf369940..c013ca6e8 100644 --- a/lisa/target_script.py +++ b/lisa/target_script.py @@ -15,10 +15,8 @@ # limitations under the License. # -import os import os.path - -SCRIPT_NAME = 'remote_script.sh' +from time import sleep class TargetScript: """ @@ -42,20 +40,25 @@ class TargetScript: _target_attrs = ['screen_resolution', 'android_id', 'abi', 'os_version', 'model'] - def __init__(self, env, script_name=SCRIPT_NAME, local_dir='./'): - self._env = env - self._target = env.target - self._script_name = script_name - self.local_dir = local_dir + def __init__(self, te, script_name='remote_script.sh', local_dir='./'): + self.target = te.target + + self.script_name = script_name + self.local_path = os.path.join(local_dir, script_name) + self.remote_path = "" + self.commands = [] - # This is made to look like the devlib Target execute() + self._proc = None + def execute(self, cmd): """ Accumulate command for later execution. :param cmd: Command that would be run on the target :type cmd: str + + This is made to look like the devlib Target execute() """ self.append(cmd) @@ -78,7 +81,7 @@ class TargetScript: # dunder name lookup would have succeeded by now, like __setstate__ if not (name.startswith('__') and name.endswith('__')) \ and name in self._target_attrs: - return getattr(self._target, name) + return getattr(self.target, name) return super().__getattribute__(name) @@ -89,43 +92,72 @@ class TargetScript: The script is created and stored on the host, and is then sent to the target. """ - actions = ['set -e'] + self.commands + ['set +e'] - actions = ['#!{} sh'.format(self._target.busybox)] + actions + actions = ['#!{} sh'.format(self.target.busybox)] + actions actions = str.join('\n', actions) # Create script locally - self._local_path = os.path.join(self.local_dir, self._script_name) - with open(self._local_path, 'w') as script: + with open(self.local_path, 'w') as script: script.write(actions) # Push it on target - self._remote_path = self._target.install(self._local_path) + self.remote_path = self.target.install(self.local_path) - def run(self, as_root=False, background=False, timeout=None): + def _prerun_check(self): + if not self.target.file_exists(self.remote_path): + raise FileNotFoundError('Remote script was not found on target device') + + def run(self, as_root=False, timeout=None): """ Run the previously pushed script + + :param as_root: Execute that script as root + :type as_root: bool + + :param timeout: Timeout (in seconds) for the execution of the script + :type timeout: int + """ + self._prerun_check() + self.target.execute(self.remote_path, as_root=as_root, timeout=timeout) + + def background(self, as_root=False): """ - if self._target.file_exists(self._remote_path): - self._run_as_root = as_root - self._bg_shell = None - if background: - self._bg_shell = self._target.background(self._remote_path, - as_root=self._run_as_root) - else: - self._target.execute(self._remote_path, - as_root=self._run_as_root, - timeout=timeout) - else: - raise IOError('Remote script was not found on target device') - - def kill(self): + Non-blocking variant of :meth:`run` + + :param as_root: Execute that script as root + :type as_root: bool + """ + self._prerun_check() + self._proc = self.target.background(self.remote_path, as_root=as_root) + + def wait(self, poll_sleep_s=1): """ - Kill a running script + Wait for a script started via :meth:`background` to complete + + :param poll_sleep_s: Sleep duration between poll() calls + :type poll_sleep_s: int + + :raises: :class:`devlib.exception.TargetNotRespondingError` """ - cmd_pid = '$(pgrep {})'.format(self._script_name) - self._target.kill(cmd_pid, as_root=self._run_as_root) - if self._bg_shell: - self._bg_shell.kill() + if not self._proc: + raise RuntimeError('No background process currently executing') + + while self._proc.poll() is None: + self.target.check_responsive(explode=True) + sleep(poll_sleep_s) + + def kill(self, as_root=False): + """ + Kill a script started via :meth:`background` + + :param as_root: Kill the script as root + :type as_root: bool + """ + if not self._proc: + raise RuntimeError('No background process currently executing') + + cmd_pid = '$(pgrep {})'.format(self.script_name) + self.target.kill(cmd_pid, as_root=as_root) + self._proc.kill() # vim :set tabstop=4 shiftwidth=4 textwidth=80 expandtab -- GitLab From 3d5d730f2dd803d3491ce74ccfde249b0ca9d937 Mon Sep 17 00:00:00 2001 From: Valentin Schneider Date: Wed, 6 Feb 2019 11:40:07 +0000 Subject: [PATCH 2/6] target_script: Only ask for a Target instead of a TestEnv A Target is all we really need. --- lisa/target_script.py | 10 +++++----- lisa/tests/hotplug/torture.py | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lisa/target_script.py b/lisa/target_script.py index c013ca6e8..9560ddaa0 100644 --- a/lisa/target_script.py +++ b/lisa/target_script.py @@ -27,9 +27,9 @@ class TargetScript: be swapped with an instance of this TargetScript class, and the commands will be accumulated for later use instead of being executed straight away. - :param env: Reference TestEnv instance. Will be used for some commands - that must really be executed instead of accumulated. - :type env: TestEnv + :param target: Reference :class:`devlib.target.Target` instance. Will be + used for some commands that must really be executed instead of accumulated. + :type env: devlib.target.Target :param script_name: Name of the script that will be pushed on the target :type script_name: str @@ -40,8 +40,8 @@ class TargetScript: _target_attrs = ['screen_resolution', 'android_id', 'abi', 'os_version', 'model'] - def __init__(self, te, script_name='remote_script.sh', local_dir='./'): - self.target = te.target + def __init__(self, target, script_name='remote_script.sh', local_dir='./'): + self.target = target self.script_name = script_name self.local_path = os.path.join(local_dir, script_name) diff --git a/lisa/tests/hotplug/torture.py b/lisa/tests/hotplug/torture.py index 2a79e6d72..ca709cf2c 100644 --- a/lisa/tests/hotplug/torture.py +++ b/lisa/tests/hotplug/torture.py @@ -117,7 +117,7 @@ class HotplugBase(TestBundle, abc.ABC): parameter. """ shift = ' ' - script = TargetScript(te, 'random_cpuhp.sh', res_dir) + script = TargetScript(te.target, 'random_cpuhp.sh', res_dir) # Record configuration # script.append('# File generated automatically') -- GitLab From 1b984b401570dd2917bda2ac676caf9145af3571 Mon Sep 17 00:00:00 2001 From: Valentin Schneider Date: Tue, 19 Feb 2019 14:47:57 +0000 Subject: [PATCH 3/6] lisa/target_script: Use pgrep -x to not kill the world --- lisa/target_script.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lisa/target_script.py b/lisa/target_script.py index 9560ddaa0..e13a0173e 100644 --- a/lisa/target_script.py +++ b/lisa/target_script.py @@ -156,7 +156,7 @@ class TargetScript: if not self._proc: raise RuntimeError('No background process currently executing') - cmd_pid = '$(pgrep {})'.format(self.script_name) + cmd_pid = '$(pgrep -x {})'.format(self.script_name) self.target.kill(cmd_pid, as_root=as_root) self._proc.kill() -- GitLab From d5f987ef5cb702244135b1e29b53303091fcbb22 Mon Sep 17 00:00:00 2001 From: Valentin Schneider Date: Tue, 19 Feb 2019 14:48:46 +0000 Subject: [PATCH 4/6] lisa/target_script: Return Popen handle in background() --- lisa/target_script.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/lisa/target_script.py b/lisa/target_script.py index e13a0173e..be29db043 100644 --- a/lisa/target_script.py +++ b/lisa/target_script.py @@ -126,10 +126,23 @@ class TargetScript: :param as_root: Execute that script as root :type as_root: bool + + :returns: the :class:`subprocess.Popen` instance for the command + + .. attention:: + + You'll have to properly close the file descriptors used by + :class:`subprocess.Popen`, for this we recommend using it as a context + manager:: + + with script.background(): + pass """ self._prerun_check() self._proc = self.target.background(self.remote_path, as_root=as_root) + return self._proc + def wait(self, poll_sleep_s=1): """ Wait for a script started via :meth:`background` to complete -- GitLab From b3ba309ae10df3d8e6b879fb30da6a1634a7d36a Mon Sep 17 00:00:00 2001 From: Valentin Schneider Date: Tue, 19 Feb 2019 14:49:45 +0000 Subject: [PATCH 5/6] doc: Build the doc for lisa.target_script While at it, add some more doc to the module. --- doc/internals.rst | 3 +++ lisa/target_script.py | 12 ++++++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/doc/internals.rst b/doc/internals.rst index 0cb55cf50..1a3b169bb 100644 --- a/doc/internals.rst +++ b/doc/internals.rst @@ -64,3 +64,6 @@ Miscellaneous utilities .. automodule:: lisa.utils :members: + +.. automodule:: lisa.target_script + :members: diff --git a/lisa/target_script.py b/lisa/target_script.py index be29db043..b8015963a 100644 --- a/lisa/target_script.py +++ b/lisa/target_script.py @@ -16,6 +16,8 @@ # import os.path +import contextlib + from time import sleep class TargetScript: @@ -23,10 +25,6 @@ class TargetScript: This class provides utility to create and run a script directly on a devlib target. - The execute() method is made to look like Devlib's, so a Target instance can - be swapped with an instance of this TargetScript class, and the commands - will be accumulated for later use instead of being executed straight away. - :param target: Reference :class:`devlib.target.Target` instance. Will be used for some commands that must really be executed instead of accumulated. :type env: devlib.target.Target @@ -36,6 +34,10 @@ class TargetScript: :param local_dir: Local directory to use to prepare the script :type local_dir: str + + :meth:`execute` is made to look like Devlib's, so a Target instance can + be swapped with an instance of this class, and the commands will be + accumulated for later use instead of being executed straight away. """ _target_attrs = ['screen_resolution', 'android_id', 'abi', 'os_version', 'model'] @@ -116,6 +118,8 @@ class TargetScript: :param timeout: Timeout (in seconds) for the execution of the script :type timeout: int + + .. attention:: :meth:`push` must have been called beforehand """ self._prerun_check() self.target.execute(self.remote_path, as_root=as_root, timeout=timeout) -- GitLab From 4157c7a9ed96627742849921060e6283a9f389ab Mon Sep 17 00:00:00 2001 From: Valentin Schneider Date: Tue, 5 Feb 2019 18:30:41 +0000 Subject: [PATCH 6/6] lisa/tests: hotplug: Get rid of the loop in the script --- lisa/tests/hotplug/torture.py | 51 +++++++++++++---------------------- 1 file changed, 19 insertions(+), 32 deletions(-) diff --git a/lisa/tests/hotplug/torture.py b/lisa/tests/hotplug/torture.py index ca709cf2c..c342ea011 100644 --- a/lisa/tests/hotplug/torture.py +++ b/lisa/tests/hotplug/torture.py @@ -20,9 +20,10 @@ import sys import random import os.path import collections +from time import sleep from devlib.module.hotplug import HotplugModule -from devlib.exception import TimeoutError +from devlib.exception import TargetNotRespondingError from lisa.tests.base import TestMetric, ResultBundle, TestBundle from lisa.target_script import TargetScript @@ -108,15 +109,12 @@ class HotplugBase(TestBundle, abc.ABC): @classmethod def _cpuhp_script(cls, te, res_dir, sequence, sleep_min_ms, - sleep_max_ms, timeout_s, random_gen): + sleep_max_ms, random_gen): """ Generate a script consisting of a random sequence of hotplugs operations Two consecutive hotplugs can be separated by a random sleep in the script. - The hotplug stress must be stopped after some time using the timeout_s - parameter. """ - shift = ' ' script = TargetScript(te.target, 'random_cpuhp.sh', res_dir) # Record configuration @@ -126,28 +124,21 @@ class HotplugBase(TestBundle, abc.ABC): # script.append('# Hotpluggable CPUs:') # script.append('# {}'.format(cls.hotpluggable_cpus)) - script.append('while true') - script.append('do') for cpu, plug_way in sequence: # Write in sysfs entry cmd = 'echo {} > {}'.format(plug_way, HotplugModule._cpu_path(te.target, cpu)) - script.append(shift + cmd) + script.append(cmd) + # Sleep if necessary if sleep_max_ms > 0: sleep_dur_sec = random_gen.randint(sleep_min_ms, sleep_max_ms)/1000.0 - script.append(shift + 'sleep {}'.format(sleep_dur_sec)) - script.append('done &') - - # Make sure to stop the hotplug stress after timeout_s seconds - script.append('LOOP_PID=$!') - script.append('sleep {}'.format(timeout_s)) - script.append('[ $(ps -q $LOOP_PID | wc -l) -gt 1 ] && kill -9 $LOOP_PID') + script.append('sleep {}'.format(sleep_dur_sec)) return script @classmethod def _from_testenv(cls, te, res_dir, seed, nr_operations, sleep_min_ms, - sleep_max_ms, duration_s, max_cpus_off): + sleep_max_ms, max_cpus_off): # Instantiate a generator so we can change the seed without any global # effect @@ -164,21 +155,20 @@ class HotplugBase(TestBundle, abc.ABC): max_cpus_off, sequence) script = cls._cpuhp_script( - te, res_dir, sequence, sleep_min_ms, sleep_max_ms, duration_s, - random_gen) + te, res_dir, sequence, sleep_min_ms, sleep_max_ms, random_gen) script.push() - target_alive = True - timeout = duration_s + 60 + # We don't want a timeout but we do want to detect if/when the target + # stops responding. So start a background shell and poll on it + with script.background(as_root=True): + try: + script.wait() - try: - script.run(as_root=True, timeout=timeout) - te.target.hotplug.online_all() - except TimeoutError: - #msg = 'Target not responding after {} seconds ...' - #cls._log.info(msg.format(timeout)) - target_alive = False + target_alive = True + te.target.hotplug.online_all() + except TargetNotRespondingError: + target_alive = False live_cpus = te.target.list_online_cpus() if target_alive else [] @@ -187,7 +177,7 @@ class HotplugBase(TestBundle, abc.ABC): @classmethod def from_testenv(cls, te:TestEnv, res_dir:ArtifactPath=None, seed=None, nr_operations=100, sleep_min_ms=10, sleep_max_ms=100, - duration_s=10, max_cpus_off=sys.maxsize) -> 'HotplugBase': + max_cpus_off=sys.maxsize) -> 'HotplugBase': """ :param seed: Seed of the RNG used to create the hotplug sequences :type seed: int @@ -202,9 +192,6 @@ class HotplugBase(TestBundle, abc.ABC): (0 would lead to no sleep) :type sleep_max_ms: int - :param duration_s: Total duration of the hotplug torture - :type duration_s: int - :param max_cpus_off: Maximum number of CPUs hotplugged out at any given moment :type max_cpus_off: int @@ -213,7 +200,7 @@ class HotplugBase(TestBundle, abc.ABC): return super().from_testenv( te, res_dir, seed=seed, nr_operations=nr_operations, sleep_min_ms=sleep_min_ms, sleep_max_ms=sleep_max_ms, - duration_s=duration_s, max_cpus_off=max_cpus_off) + max_cpus_off=max_cpus_off) def test_target_alive(self) -> ResultBundle: """ -- GitLab