Adam Litke has posted comments on this change. Change subject: Live Merge: Suspend disk stats collection during pivot ......................................................................
Patch Set 8: (2 comments) http://gerrit.ovirt.org/#/c/31367/8/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 5642: if mode in (LiveMergeCleanupThread.MODE_CLEANUP, Line 5643: LiveMergeCleanupThread.MODE_PIVOT): Line 5644: self.log.info("Live merge job completed (job %s)", jobID) Line 5645: self._syncVolumeChain(drive) Line 5646: self.startDisksStatsCollection() > I asked about this here: http://gerrit.ovirt.org/#/c/31267/5/vdsm/virt/vm.p ok. I've done some refactoring for the next series that should look nicer. Line 5647: else: Line 5648: raise RuntimeError("Invalid mode: '%s'" % mode) Line 5649: Line 5650: def _initLegacyConf(self): Line 5642: if mode in (LiveMergeCleanupThread.MODE_CLEANUP, Line 5643: LiveMergeCleanupThread.MODE_PIVOT): Line 5644: self.log.info("Live merge job completed (job %s)", jobID) Line 5645: self._syncVolumeChain(drive) Line 5646: self.startDisksStatsCollection() > Do we have try finally block ensuring that we reach this line after line 56 If pivot succeeds but cleanup fails you don't want to resume stats collection because the vdsm drive info remains out of sync with reality. You need to wait until a successful cleanup before re-enabling. This may happen the next time a cleanup thread is created (so it's not possible for it to be deterministic. Line 5647: else: Line 5648: raise RuntimeError("Invalid mode: '%s'" % mode) Line 5649: Line 5650: def _initLegacyConf(self): -- To view, visit http://gerrit.ovirt.org/31367 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I08689aec4d61871a568a6f92661b560fbf4d0b57 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Greg Padgett <[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
