On Fri, 2010-11-26 at 12:02 +0100, Frank Becker wrote:
> From: Frank Becker <[email protected]>
Hi Frank, was reviewing your patchset, and I must say the feature is
implemented cleanly and in a minimally intrusive way, congrats! I have
noticed some changes I do not agree with, see below:
> ---
> client/bin/job.py | 24 +++++++++++++++---------
> 1 files changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/client/bin/job.py b/client/bin/job.py
> index 3d552ce..141feab 100644
> --- a/client/bin/job.py
> +++ b/client/bin/job.py
> @@ -162,9 +162,9 @@ class base_client_job(base_job.base_job):
> self._cleanup_results_dir()
>
> logging_manager.configure_logging(
> - client_logging_config.ClientLoggingConfig(),
> - results_dir=self.resultdir,
> - verbose=options.verbose)
> + client_logging_config.ClientLoggingConfig(),
> + results_dir=self.resultdir,
> + verbose=options.verbose)
> logging.info('Writing results to %s', self.resultdir)
>
> # init_group_level needs the state
> @@ -190,8 +190,8 @@ class base_client_job(base_job.base_job):
> # send the entry to stdout, if it's enabled
> logging.info(rendered_entry)
> self._logger = base_job.status_logger(
> - self, status_indenter(self), record_hook=client_job_record_hook)
> -
> + self, status_indenter(self), record_hook=client_job_record_hook,
> + tap_writer=self._tap)
>
> def _post_record_init(self, control, options, drop_caches,
> extra_copy_cmdline):
> @@ -663,8 +663,7 @@ class base_client_job(base_job.base_job):
> # check to see if any partitions have changed
> partition_list = partition_lib.get_partition_list(self,
> exclude_swap=False)
> - mount_info = partition_lib.get_mount_info(partition_list)
> -
> + mount_info = set((p.device, p.get_mountpoint()) for p in
> partition_list)
^ What was the purpose of this change? With get_mount_info we have a
more reliable method of identifying the devices on newer distros, and it
has a fallback in case blkid is not available...
> old_mount_info = self._state.get('client', 'mount_info')
> if mount_info != old_mount_info:
> new_entries = mount_info - old_mount_info
> @@ -783,7 +782,7 @@ class base_client_job(base_job.base_job):
> # save the partition list and mount points, as well as the cpu count
> partition_list = partition_lib.get_partition_list(self,
> exclude_swap=False)
> - mount_info = partition_lib.get_mount_info(partition_list)
> + mount_info = set((p.device, p.get_mountpoint()) for p in
> partition_list)
^ Same here. Maybe you are having trouble with get_mount_info? Please
elucidate.
> self._state.set('client', 'mount_info', mount_info)
> self._state.set('client', 'cpu_count', utils.count_cpus())
>
> @@ -870,7 +869,12 @@ class base_client_job(base_job.base_job):
>
>
> def complete(self, status):
> - """Clean up and exit"""
> + """Write pending TAP reports, clean up, and exit"""
> + # write out TAP reports
> + if self._tap.do_tap_report:
> + self._tap.write()
> + self._tap._write_tap_archive()
> +
> # We are about to exit 'complete' so clean up the control file.
> dest = os.path.join(self.resultdir,
> os.path.basename(self._state_file))
> shutil.move(self._state_file, dest)
> @@ -1168,8 +1172,10 @@ def runjob(control, drop_caches, options):
> if options.cont and not os.path.exists(state):
> raise error.JobComplete("all done")
>
> +
> myjob = job(control=control, drop_caches=drop_caches,
> options=options)
>
> +
> # Load in the users control file, may do any one of:
> # 1) execute in toto
> # 2) define steps, and select the first via next_step()
_______________________________________________
Autotest mailing list
[email protected]
http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
