Martin Polednik has posted comments on this change. Change subject: vm: migration: exponential downtime increment ......................................................................
Patch Set 8: Code-Review-1 (5 comments) -1 for visibility, other than the minor things it seems to be OK (+ need answer for the steps >= 2 question) http://gerrit.ovirt.org/#/c/25820/8/tests/vmMigrationTests.py File tests/vmMigrationTests.py: Line 46: class TestVmMigrationDowntimeSequence(TestCaseBase): Line 47: Line 48: @permutations(_PARAMS) Line 49: def test_downtime_is_sequence(self, dtime, steps): Line 50: self.assertTrue(len(self._default(dtime, steps)) >= 2) can be rewritten as self.assertGreaterEqual(len(self._default(dtime, steps)), 2) another thing, is steps really >= 2 or do we allow single step? Line 51: Line 52: @permutations(_PARAMS) Line 53: def test_downtime_increasing(self, dtime, steps): Line 54: for a, b in pairwise(self._default(dtime, steps)): Line 51: Line 52: @permutations(_PARAMS) Line 53: def test_downtime_increasing(self, dtime, steps): Line 54: for a, b in pairwise(self._default(dtime, steps)): Line 55: self.assertTrue(a <= b) can be rewritten as self.assertGreater(b, a) Line 56: Line 57: @permutations(_PARAMS) Line 58: def test_exponential_dowtime_never_zero(self, dtime, steps): Line 59: for dt in self._default(dtime, steps): Line 56: Line 57: @permutations(_PARAMS) Line 58: def test_exponential_dowtime_never_zero(self, dtime, steps): Line 59: for dt in self._default(dtime, steps): Line 60: self.assertTrue(dt > 0) can be rewritten as self.assertGreater(dt, 0) Line 61: Line 62: @permutations(_PARAMS) Line 63: def test_exponential_downtime_is_lower(self, dtime, steps): Line 64: # it's OK if exponential starts a little higher than linear... Line 69: Line 70: # ...but what matters is that after that, it stays lower. Line 71: for i, (a, b) in enumerate(zip(exp[1:], lin[1:])): Line 72: msg = 'step=%i/%i exp=%f lin=%f' % (i+1, steps, a, b) Line 73: self.assertTrue(a <= b, msg) again, self assertGreater(b, a, msg) Line 74: Line 75: @permutations(_PARAMS) Line 76: def test_exponential_same_end_value(self, dtime, steps): Line 77: exp = self._default(dtime, steps) Line 107: def pairwise(iterable): Line 108: "s -> (s0,s1), (s1,s2), (s2, s3), ..." Line 109: a, b = tee(iterable) Line 110: next(b, None) Line 111: return izip(a, b) thought about this a bit, wouldn't importing islice and running izip(a, islice(b, 1, None)) be a bit nicer? Line 112: Line 113: Line 114: def _linear_downtime(downtime, steps): Line 115: "this is the old formula as reference" -- To view, visit http://gerrit.ovirt.org/25820 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6401772f52ea28144452e67198bddff18f6703eb Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Martin Betak <mbe...@redhat.com> Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com> Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com> Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches