Adam Litke has uploaded a new change for review.
Change subject: fix abort to raise ......................................................................
fix abort to raise
Change-Id: I5efe2319d023bd813777c76a0bbaa0b9576281f6 Signed-off-by: Adam Litke alitke@redhat.com --- M lib/vdsm/jobs.py M tests/jobsTests.py 2 files changed, 21 insertions(+), 8 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/47/65147/1
diff --git a/lib/vdsm/jobs.py b/lib/vdsm/jobs.py index 2c6c27c..42f43bc 100644 --- a/lib/vdsm/jobs.py +++ b/lib/vdsm/jobs.py @@ -155,13 +155,24 @@ self._status = STATUS.RUNNING try: self._run() + except exception.ActionStopped: + # Once a job has been aborted we expect _run to raise ActionStopped + with self._status_lock: + if self.status != STATUS.ABORTING: + logging.warning("Unexpected ActionStopped exception in " + "job %r from state %r", + self.id, self.status) + self._status = STATUS.ABORTED + if self.autodelete: + self._autodelete() except Exception as e: with self._status_lock: if self.status == STATUS.ABORTING: + # Other exceptions while in aborting state mean that abort + # was not successful. We should not autodelete because + # there may still be an ongoing operation. logging.exception("Failed to abort job (id=%s desc=%s)", self.id, self.description) - # We do not autodelete failed aborting jobs because it is - # likely that something is still running on the system. else: self._status = STATUS.FAILED logging.exception("Job (id=%s desc=%s) failed", @@ -172,13 +183,12 @@ e = exception.GeneralException(str(e)) self._error = e else: + # We always mark the job done. Even if we were in aborting state + # _run might finish normally before the abort action executes. with self._status_lock: - if self.status == STATUS.ABORTING: - self._status = STATUS.ABORTED - else: - self._status = STATUS.DONE - if self.autodelete: - self._autodelete() + self._status = STATUS.DONE + if self.autodelete: + self._autodelete()
def _abort(self): """ @@ -188,6 +198,8 @@ status to change to aborted. - Must raise if the job could not be aborted - Must not raise if the job was aborted + - A successful abort must cause the job's _run method to raise an + ActionStopped exception. """ raise AbortNotSupported()
diff --git a/tests/jobsTests.py b/tests/jobsTests.py index 6c6d49a..4c83810 100644 --- a/tests/jobsTests.py +++ b/tests/jobsTests.py @@ -86,6 +86,7 @@ def _run(self): self.event_running.set() self.event_aborted.wait(1) + raise exception.ActionStopped()
def _abort(self): self.event_aborted.set()
gerrit-hooks has posted comments on this change.
Change subject: fix abort to raise ......................................................................
Patch Set 1:
* Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0'])
Nir Soffer has posted comments on this change.
Change subject: fix abort to raise ......................................................................
Patch Set 1:
(2 comments)
https://gerrit.ovirt.org/#/c/65147/1/lib/vdsm/jobs.py File lib/vdsm/jobs.py:
Line 186 Line 187 Line 188 Line 189 Line 190 This seems to conflict the new comment.
Line 198: status to change to aborted. Line 199: - Must raise if the job could not be aborted Line 200: - Must not raise if the job was aborted Line 201: - A successful abort must cause the job's _run method to raise an Line 202: ActionStopped exception. Are you sure this is always possible? Line 203: """ Line 204: raise AbortNotSupported() Line 205: Line 206: def _run(self):
Nir Soffer has posted comments on this change.
Change subject: fix abort to raise ......................................................................
Patch Set 1: Code-Review-1
See comments.
Adam Litke has abandoned this change.
Change subject: fix abort to raise ......................................................................
Abandoned
gerrit-hooks has posted comments on this change.
Change subject: fix abort to raise ......................................................................
Patch Set 1:
* Update tracker: IGNORE, no Bug-Url found
vdsm-patches@lists.fedorahosted.org