Nir Soffer has posted comments on this change. Change subject: storage: Fix abort race in SDM.copy_data ......................................................................
Patch Set 5: Code-Review-1 (2 comments) https://gerrit.ovirt.org/#/c/65150/5/tests/storage_sdm_copy_data_test.py File tests/storage_sdm_copy_data_test.py: Line 309: job_id = make_uuid() Line 310: job = storage.sdm.api.copy_data.Job(job_id, 0, source, dest) Line 311: # Simulate status changing to aborting right after _run called Line 312: job._run = partial(_fake_run, job, job._run) Line 313: job.run() I think we can simplify - since we are already using job._run, we can simulate the framework: job = Job() job._status = RUNNING job.abort() The job should be in ABORTING state, so: with self.assertRaises(ActionStopped): job._run() Line 314: self.assertEqual(jobs.STATUS.ABORTED, job.status) Line 315: self.assertEqual(sc.LEGAL_VOL, dst_vol.getLegality()) Line 316: self.assertEqual(gen_id, dst_vol.getMetaParam(sc.GENERATION)) Line 317: https://gerrit.ovirt.org/#/c/65150/5/vdsm/storage/sdm/api/copy_data.py File vdsm/storage/sdm/api/copy_data.py: Line 73: Line 74: with self._status_lock: Line 75: # Do not start copying if we have already been aborted Line 76: if self._status == jobs.STATUS.ABORTING: Line 77: raise exception.ActionStopped() This will be needed by every job supporting abort. We probably need to add a helper in the framework, like _check_aborting. We also need to rename the exception to Aborted. Line 78: self._operation = qemuimg.convert( Line 79: self._source.path, Line 80: self._dest.path, Line 81: srcFormat=src_format, -- To view, visit https://gerrit.ovirt.org/65150 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0c7d9aadb981e8b851a70c7ad0fdc0d75b46dc51 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list -- [email protected] To unsubscribe send an email to [email protected]
