Milan Zamazal has posted comments on this change. Change subject: migrations: enhance legacy downtime alg ......................................................................
Patch Set 3: (6 comments) https://gerrit.ovirt.org/#/c/56561/3//COMMIT_MSG Commit Message: PS3, Line 12: during first ... during the first ... PS3, Line 15: change the way and params the wait for next step is calculated so I can't parse this sentence. https://gerrit.ovirt.org/#/c/56561/3/lib/vdsm/config.py.in File lib/vdsm/config.py.in: Line 112: Line 113: ('migration_downtime_delay', '100', Line 114: 'This value is used on the source host to define the delay before ' Line 115: 'setting/increasing the downtime of a migration. ' Line 116: 'The value is per GiB of RAM. A minimum of twice this value is ' When we are on it, we could probably add to the description the time unit of this value (seconds?). Line 117: 'used for VMs with less than 2 GiB of RAM'), Line 118: Line 119: ('migration_downtime_steps', '5', Line 120: 'Incremental steps used to reach migration_downtime.'), Line 113: ('migration_downtime_delay', '100', Line 114: 'This value is used on the source host to define the delay before ' Line 115: 'setting/increasing the downtime of a migration. ' Line 116: 'The value is per GiB of RAM. A minimum of twice this value is ' Line 117: 'used for VMs with less than 2 GiB of RAM'), Really?? Line 118: Line 119: ('migration_downtime_steps', '5', Line 120: 'Incremental steps used to reach migration_downtime.'), Line 121: https://gerrit.ovirt.org/#/c/56561/3/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 501: Line 502: delay_per_gib = config.getint('vars', 'migration_downtime_delay') Line 503: memSize = int(vm.conf['memSize']) Line 504: self._wait = min(delay_per_gib * Line 505: (memSize + 1023) / 1024 / self._steps, 60) I'm confused about migration_downtime_delay units and the whole formula here. I'm having trouble to match the numbers with the commit message. We reach the 60 seconds limit here much sooner than on 2 GB with delay_per_gib == 100, don't we? And don't we want to divide 60 by self._steps as well? Line 506: if self._steps > 1: Line 507: self._exponentialDowntimes = \ Line 508: list(exponential_downtime(downtime, self._steps)) Line 509: else: Line 503: memSize = int(vm.conf['memSize']) Line 504: self._wait = min(delay_per_gib * Line 505: (memSize + 1023) / 1024 / self._steps, 60) Line 506: if self._steps > 1: Line 507: self._exponentialDowntimes = \ Please don't use camelCase for attribute names, use underscores. Line 508: list(exponential_downtime(downtime, self._steps)) Line 509: else: Line 510: self._exponentialDowntimes = [downtime] Line 511: -- To view, visit https://gerrit.ovirt.org/56561 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5d18a77c91a1344110afef1577e310faab3ffe5b Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Jelinek <tjeli...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek <mskri...@redhat.com> Gerrit-Reviewer: Milan Zamazal <mzama...@redhat.com> Gerrit-Reviewer: Tomas Jelinek <tjeli...@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/mailman/listinfo/vdsm-patches
