[Piglit] [PATCH] programs/run.py add file sync command line option

[Dylan Baker]

On Monday, January 02, 2012 05:09:08 PM Matthew Atwood wrote:
> From: Matt Atwood <matthew.s.atwood at intel.com>
> 
> Currently while running igt tests a kernel panic causes the results json
> file to lose all data. This patch adds a command line option (-s,
> --sync) that syncs the file descriptor to disk after every test allowing
> data to persist through hard crashes.
> ---
>  framework/core.py         |  3 ++-
>  framework/programs/run.py | 14 ++++++++++----
>  framework/results.py      | 12 +++++++++++-
>  3 files changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/framework/core.py b/framework/core.py
> index d3922a9..8c4ff57 100644
> --- a/framework/core.py
> +++ b/framework/core.py
> @@ -100,7 +100,7 @@ class Options(object):
>      """
>      def __init__(self, concurrent=True, execute=True, include_filter=None,
>                   exclude_filter=None, valgrind=False, dmesg=False,
> -                 verbose=False):
> +                 verbose=False, sync=False):
>          self.concurrent = concurrent
>          self.execute = execute
>          self.filter = [re.compile(x) for x in include_filter or []]
> @@ -109,6 +109,7 @@ class Options(object):
>          self.valgrind = valgrind
>          self.dmesg = dmesg
>          self.verbose = verbose
> +        self.sync = sync
>          # env is used to set some base environment variables that are not going
>          # to change across runs, without sending them to os.environ which is
>          # fickle as easy to break
> diff --git a/framework/programs/run.py b/framework/programs/run.py
> index eb67d7f..468bff4 100644
> --- a/framework/programs/run.py
> +++ b/framework/programs/run.py
> @@ -97,6 +97,9 @@ def run(input_):
>                          type=path.realpath,
>                          metavar="<Results Path>",
>                          help="Path to results folder")
> +    parser.add_argument("-s", "--sync",
> +                        action="store_true",
> +                        help="Sync results to disk after every test")
>      args = parser.parse_args(input_)
>  
>      # Disable Windows error message boxes for this and all child processes.
> @@ -132,7 +135,8 @@ def run(input_):
>                          execute=args.execute,
>                          valgrind=args.valgrind,
>                          dmesg=args.dmesg,
> -                        verbose=args.verbose)
> +                        verbose=args.verbose,
> +                        sync=args.sync)
>  
>      # Set the platform to pass to waffle
>      if args.platform:
> @@ -154,7 +158,7 @@ def run(input_):
>      # Begin json.
>      result_filepath = path.join(args.results_path, 'results.json')
>      result_file = open(result_filepath, 'w')
> -    json_writer = framework.results.JSONWriter(result_file)
> +    json_writer = framework.results.JSONWriter(result_file, opts.sync)
>  
>      # Create a dictionary to pass to initialize json, it needs the contents of
>      # the env dictionary and profile and platform information
> @@ -211,7 +215,8 @@ def resume(input_):
>                          execute=results.options['execute'],
>                          valgrind=results.options['valgrind'],
>                          dmesg=results.options['dmesg'],
> -                        verbose=results.options['verbose'])
> +                        verbose=results.options['verbose'],
> +                        sync=results.options['sync'])
>  
>      core.get_config(args.config_file)
>  
> @@ -219,7 +224,8 @@ def resume(input_):
>          opts.env['PIGLIT_PLATFORM'] = results.options['platform']
>  
>      results_path = path.join(args.results_path, 'results.json')
> -    json_writer = framework.results.JSONWriter(open(results_path, 'w+'))
> +    json_writer = framework.results.JSONWriter(open(results_path, 'w+'),
> +                                               opts.sync)
>      json_writer.initialize_json(results.options, results.name,
>                                  core.collect_system_info())
>  
> diff --git a/framework/results.py b/framework/results.py
> index 4b5fb30..d39f63b 100644
> --- a/framework/results.py
> +++ b/framework/results.py
> @@ -102,8 +102,9 @@ class JSONWriter(object):
>  
>      INDENT = 4
>  
> -    def __init__(self, f):
> +    def __init__(self, f, fsync):
>          self.file = f
> +        self.fsync = fsync
>          self.__indent_level = 0
>          self.__inhibit_next_indent = False
>          self.__encoder = json.JSONEncoder(indent=self.INDENT,
> @@ -175,6 +176,11 @@ class JSONWriter(object):
>          self.file.close()
>  
>      @synchronized_self
> +    def __file_sync(self):
> +        if self.fsync:
> +            os.fsync(self.file)
> +
> +    @synchronized_self
>      def __write_indent(self):
>          if self.__inhibit_next_indent:
>              self.__inhibit_next_indent = False
> @@ -201,6 +207,7 @@ class JSONWriter(object):
>          self.__indent_level += 1
>          self.__is_collection_empty.append(True)
>          self._open_containers.append('dict')
> +        self.__file_sync()
>  
>      @synchronized_self
>      def close_dict(self):
> @@ -212,6 +219,7 @@ class JSONWriter(object):
>          self.file.write('}')
>          assert self._open_containers[-1] == 'dict'
>          self._open_containers.pop()
> +        self.__file_sync()
>  
>      @synchronized_self
>      def write_dict_item(self, key, value):
> @@ -221,6 +229,8 @@ class JSONWriter(object):
>          # Write value.
>          self.__write(value)
>  
> +        self.__file_sync()
> +
>      @synchronized_self
>      def write_dict_key(self, key):
>          # Write comma if this is not the initial item in the dict.

I don't think that you've used your new method everywhere you want to
use it, I think that you need to call it at the end of each method
except __init__.

I think a decorator would be an elegant way to solve this, but it would
be a rather complex decorator (or, actually probably a pair of
decorators), and it would probably mean working on the synchronized_self
decorator as well, since I strongly suspect there would be a bad
interaction there.

> -- 
> 1.8.3.2
> 
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20140813/9b778068/attachment.sig>