Nir Soffer has posted comments on this change. Change subject: Implement SDM.copy_data ......................................................................
Patch Set 4: (2 comments) https://gerrit.ovirt.org/#/c/60420/4/vdsm/storage/sdm/api/copy_data.py File vdsm/storage/sdm/api/copy_data.py: Line 59: if self._operation: Line 60: self._operation.abort() Line 61: Line 62: def _run(self): Line 63: with self.source.acquire(), self.destination.acquire(exclusive=True): > Actually the domain lock needs to be taken before the task is scheduled (en Does not sound like the right way. protect_image is not relevant to endpoints, only to vdsm volume. Why task manager enforces the domain lock? Can we change this now, or bypass this check for the new verbs? Lets move locking tot the next patch, so engine guys can test this code with the engine side, while we find the best way to get locking right. Line 64: with (self.source.prepare(), Line 65: self.destination.prepare(writable=True)): Line 66: # Do not start copying if we have already been aborted Line 67: if self._status == jobs.STATUS.ABORTED: Line 135: # concurrent copy_data operations from taking place on the same host. Line 136: # To allow this, we'll need to lock the destination image in shared Line 137: # mode and the destination volume in exclusive mode. This requires Line 138: # changes to the resourceManager. Line 139: vars.task.getSharedLock(sc.STORAGE, self.sd_id) > Yep. Since it has to be taken before the task is run I'll be moving it out But this means creation of the endpoints in hsm.py, making crate_endpoint public - I like the minimal code in hsm.py. Having *all* the logic of a verb in *one* file is great. Line 140: res_ns = sd.getNamespace(self.sd_id, IMAGE_NAMESPACE) Line 141: lock_type = rm.LockType.exclusive if exclusive else rm.LockType.shared Line 142: with rmanager.acquireResource(res_ns, self.img_id, lock_type): Line 143: yield -- To view, visit https://gerrit.ovirt.org/60420 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I30ff635c0c73f67b296033b4a506fc3b9ededfbe Gerrit-PatchSet: 4 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: 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
