[dvsim] Minor fixes to coverage extraction
The issue was reported by @rasmus-madsen - if the cleanup tasks in
Deploy::post_finish() fails, it attempts append the error message to a
Launcher class member called `fail_msg` which has not been created,
causing the dvsim invocation to bomb improperly.
The primary goal of this change is to allow the failure of the cleanup
tasks to also be factored into whether the job failed or not. This is
achieved by making the job `status` a member of the `Launcher` class
which is finally returned to the Scheduler. Cleanup tasks are run
regardless of the job's outcome, but now that invocation is updated to
catch any exceptions thrown, so that the `status` can be updated if the
cleanup tasks fail for whatever reason, within the
`Launcher::_post_finish()` invocation.
In addition, a minor change in the Xcelium coverage extraction code
fixes the issue of some coverage metrics not showing up correctly.
Signed-off-by: Srikrishna Iyer <sriyer@google.com>
diff --git a/util/dvsim/Deploy.py b/util/dvsim/Deploy.py
index dac9788..2a120a7 100644
--- a/util/dvsim/Deploy.py
+++ b/util/dvsim/Deploy.py
@@ -612,21 +612,16 @@
def post_finish(self, status):
"""Extract the coverage results summary for the dashboard.
- If that fails for some reason, report the job as a failure.
+ If the extraction fails, an appropriate exception is raised, which must
+ be caught by the caller to mark the job as a failure.
"""
if self.dry_run or status != 'P':
return
- results, self.cov_total, ex_msg = get_cov_summary_table(
+ results, self.cov_total = get_cov_summary_table(
self.cov_report_txt, self.sim_cfg.tool)
- if ex_msg:
- self.launcher.fail_msg += ex_msg
- log.error(ex_msg)
- return
-
- # Succeeded in obtaining the coverage data.
colalign = (("center", ) * len(results[0]))
self.cov_results = tabulate(results,
headers="firstrow",
diff --git a/util/dvsim/Launcher.py b/util/dvsim/Launcher.py
index cddf436..cc9d308 100644
--- a/util/dvsim/Launcher.py
+++ b/util/dvsim/Launcher.py
@@ -134,6 +134,12 @@
# Store the deploy object handle.
self.deploy = deploy
+ # Status of the job. This is primarily determined by the
+ # _check_status() method, but eventually updated by the _post_finish()
+ # method, in case any of the cleanup tasks fails. This value is finally
+ # returned to the Scheduler by the poll() method.
+ self.status = None
+
# Return status of the process running the job.
self.exit_code = None
@@ -160,7 +166,6 @@
"""
dest = Path(self.deploy.sim_cfg.links[status], self.deploy.qual_name)
-
mk_symlink(self.deploy.odir, dest)
# Delete the symlink from dispatched directory if it exists.
@@ -302,11 +307,24 @@
"""
assert status in ['P', 'F', 'K']
- if status in ['P', 'F']:
- self._link_odir(status)
- self.deploy.post_finish(status)
+ self._link_odir(status)
log.debug("Item %s has completed execution: %s", self, status)
- if status != "P":
+
+ try:
+ # Run the target-specific cleanup tasks regardless of the job's
+ # outcome.
+ self.deploy.post_finish(status)
+ except Exception as e:
+ # If the job had already failed, then don't do anything. If it's
+ # cleanup task failed, then mark the job as failed.
+ if status == "P":
+ status = "F"
+ err_msg = ErrorMessage(line_number=None,
+ message=f"{e}",
+ context=[])
+
+ self.status = status
+ if self.status != "P":
assert err_msg and isinstance(err_msg, ErrorMessage)
self.fail_msg = err_msg
log.log(VERBOSE, err_msg.message)
diff --git a/util/dvsim/LocalLauncher.py b/util/dvsim/LocalLauncher.py
index 0a09682..03f997a 100644
--- a/util/dvsim/LocalLauncher.py
+++ b/util/dvsim/LocalLauncher.py
@@ -80,7 +80,7 @@
self.exit_code = self.process.returncode
status, err_msg = self._check_status()
self._post_finish(status, err_msg)
- return status
+ return self.status
def kill(self):
'''Kill the running process.
@@ -104,9 +104,9 @@
context=[]))
def _post_finish(self, status, err_msg):
- super()._post_finish(status, err_msg)
self._close_process()
self.process = None
+ super()._post_finish(status, err_msg)
def _close_process(self):
'''Close the file descriptors associated with the process.'''
diff --git a/util/dvsim/LsfLauncher.py b/util/dvsim/LsfLauncher.py
index e7119f0..1069ecb 100644
--- a/util/dvsim/LsfLauncher.py
+++ b/util/dvsim/LsfLauncher.py
@@ -119,10 +119,6 @@
def __init__(self, deploy):
super().__init__(deploy)
- # Set the status. Only update after the job is done - i.e. status will
- # transition from None to P/F/K.
- self.status = None
-
# Maintain the job script output as an instance variable for polling
# and cleanup.
self.bsub_out = None
@@ -390,7 +386,6 @@
def _post_finish(self, status, err_msg):
if self.bsub_out_fd:
self.bsub_out_fd.close()
- self.status = status
if self.exit_code is None:
self.exit_code = 0 if status == 'P' else 1
super()._post_finish(status, err_msg)
diff --git a/util/dvsim/sim_utils.py b/util/dvsim/sim_utils.py
index 9408778..435e00f 100644
--- a/util/dvsim/sim_utils.py
+++ b/util/dvsim/sim_utils.py
@@ -16,21 +16,15 @@
# and the coverage was extracted successfully. It returns a tuple of:
# List of metrics and values
# Final coverage total
-# Error message, if failed
+#
+# Raises the appropriate exception if the coverage summary extraction fails.
def get_cov_summary_table(cov_report_txt, tool):
- try:
- with open(cov_report_txt, 'r') as f:
- if tool == 'xcelium':
- return xcelium_cov_summary_table(f)
- if tool == 'vcs':
- return vcs_cov_summary_table(f)
-
- err_msg = "Unsupported tool for cov extraction: {}".format(tool)
- return None, None, err_msg
-
- except Exception as e:
- err_msg = "Exception occurred: {}".format(str(e))
- return None, None, err_msg
+ with open(cov_report_txt, 'r') as f:
+ if tool == 'xcelium':
+ return xcelium_cov_summary_table(f)
+ if tool == 'vcs':
+ return vcs_cov_summary_table(f)
+ raise NotImplementedError(f"{tool} is unsupported for cov extraction.")
# Same desc as above, but specific to Xcelium and takes an opened input stream.
@@ -56,7 +50,7 @@
values = line.strip().split()
for i, value in enumerate(values):
value = value.strip()
- m = re.search(r"\((\d+)/(\d+)\)", value)
+ m = re.search(r"\((\d+)/(\d+).*\)", value)
if m:
items[metrics[i]]['covered'] += int(m.group(1))
items[metrics[i]]['total'] += int(m.group(2))
@@ -75,11 +69,10 @@
values.append(value)
if metric == 'Score':
cov_total = value
- return [metrics, values], cov_total, None
+ return [metrics, values], cov_total
# If we reached here, then we were unable to extract the coverage.
- err_msg = "ParseError: coverage data not found!"
- return None, None, err_msg
+ raise SyntaxError(f"Coverage data not found in {buf.name}!")
# Same desc as above, but specific to VCS and takes an opened input stream.
@@ -100,8 +93,7 @@
values.append(val)
# first row is coverage total
cov_total = values[0]
- return [metrics, values], cov_total, None
+ return [metrics, values], cov_total
# If we reached here, then we were unable to extract the coverage.
- err_msg = "ParseError: coverage data not found!"
- return None, None, err_msg
+ raise SyntaxError(f"Coverage data not found in {buf.name}!")
