Adam Litke has posted comments on this change.

Change subject: sdm: Use guarded.context in copy_data
......................................................................


Patch Set 13:

(4 comments)

https://gerrit.ovirt.org/#/c/61693/13/tests/storage_sdm_copy_data_test.py
File tests/storage_sdm_copy_data_test.py:

Line 136
Line 137
Line 138
Line 139
Line 140
> We need to write these tests now.
Let's save this for a follow-up patch.  This patch is about adding locking, not 
adding these tests.


Line 116: 
Line 117:             with checked_locks() as checker:
Line 118:                 job.run()
Line 119:                 wait_for_job(job)
Line 120:                 checker.validate(self.get_lock_list(src_vol, dst_vol))
> I don't think that anyone can understand from this code what is the expecte
Good idea.  This also avoids the need for the LockChecker infrastructure.
Line 121: 
Line 122:             self.assertEqual(jobs.STATUS.DONE, job.status)
Line 123:             self.assertEqual(100.0, job.progress)
Line 124:             self.assertNotIn('error', job.info())


https://gerrit.ovirt.org/#/c/61693/13/vdsm/storage/sdm/api/copy_data.py
File vdsm/storage/sdm/api/copy_data.py:

Line 95
Line 96
Line 97
Line 98
Line 99
> We must implement this todo to finish this milestone, you can add it in the
Will do.


Line 96:         self._vol = None
Line 97: 
Line 98:     # @property
Line 99:     # def host_id(self):
Line 100:     #     return self._host_id
> Please remove if not needed.
Done
Line 101: 
Line 102:     @property
Line 103:     def locks(self):
Line 104:         img_ns = sd.getNamespace(sc.IMAGE_NAMESPACE, self.sd_id)


-- 
To view, visit https://gerrit.ovirt.org/61693
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5c2a9527f5e3787069e0847e1bf775a60321b306
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <ali...@redhat.com>
Gerrit-Reviewer: Adam Litke <ali...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Reply via email to