Federico Simoncelli has posted comments on this change. Change subject: Live Merge: Simplify pivot flow ......................................................................
Patch Set 5: (3 comments) http://gerrit.ovirt.org/#/c/31849/5/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 5365: else: Line 5366: # Libvirt has stopped reporting this job so we know it will Line 5367: # never report it again. Line 5368: doPivot = False Line 5369: storedJob['gone'] = True A whitespace here would have helped to understand that we're entering in another new if block (not related to the previous if/else) Line 5370: if not liveInfo or doPivot: Line 5371: if not cleanThread: Line 5372: # There is no cleanup thread so the job must have just Line 5373: # ended. Spawn an async cleanup. Line 5636: # active layer contains the same data as its parent so the ILLEGAL Line 5637: # flag indicates that the VM should be restarted using the parent. Line 5638: newVols = [vol['volumeID'] for vol in self.drive.volumeChain Line 5639: if vol['volumeID'] != self.drive.volumeID] Line 5640: self.vm.cif.irs.imageSyncVolumeChain(self.drive.domainID, I remember that we agreed on using only one irs command (imageSyncVolumeChain) because it would have been called in one place only from vm.py. Instead it seems that now we're calling it from different places knowing in advance if it should pivot or not. Line 5641: self.drive.imageID, Line 5642: self.drive['volumeID'], newVols) Line 5643: Line 5644: self.vm.log.info("Requesting pivot to complete active layer commit " Line 5653: if self.doPivot: Line 5654: self.tryPivot() Line 5655: self.vm.log.info("Synchronizing volume chain after live merge " Line 5656: "(job %s)", self.jobId) Line 5657: self.vm._syncVolumeChain(self.drive) Is this going to call imageSyncVolumeChain once again? Is there anything of the two flows that we can unify? Line 5658: self.success = True Line 5659: self.vm.log.info("Synchronization completed (job %s)", self.jobId) Line 5660: Line 5661: def isSuccessful(self): -- To view, visit http://gerrit.ovirt.org/31849 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5e6a9ea7096b5d81f418754d411f135450bf572e Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
