Milan Zamazal has posted comments on this change.

Change subject: migrations: change convergence schedule from time to iterations
......................................................................


Patch Set 3: Code-Review-1

(6 comments)

The code seems to be OK to me. But please fix errors and confusions in the 
commit message -- it's already difficult to understand the concept itself and 
it's even more difficult to understand it with the errors.

https://gerrit.ovirt.org/#/c/56558/3//COMMIT_MSG
Commit Message:

PS3, Line 12: than
... then ...


Line 18: In the first iteration there can be no stalling, qemu only copies 
memory and
Line 19: does not look at how much of it changed.
Line 20: 
Line 21: After this period of time it checks if it can move the rest of the VMs 
memory
Line 22: when pauses it for the dowtime. If not, it starts a new iteration. 
This new
when *it* pauses it for the dow*n*time. ... ?
Line 23: iteration starts copying all the dirtyed memory which can be a lot. 
And the
Line 24: whole second iteration the lowmark of the memory will be lower than 
the current
Line 25: remaining thus being threated as stalling. But this does not actually 
mean we
Line 26: want to enlarge the downtime during the iteration since we don't know 
if qemu


PS3, Line 23: dirtyed
... dirtied ...


PS3, Line 23: And the
            : whole second iteration the lowmark of the memory will be lower 
than the current
            : remaining
I can't parse this sentence.


PS3, Line 25: threated
... treated ... ?


PS3, Line 42: migration either
            :     progressing inside one iteration or the next iteration but it 
was so fast
            :     that it went under the remembered value.
Difficult to understand. How about: ... migration is either progressing within 
the same iteration, or it is already in the next iteration but the progress was 
fast enough to get below the remembered value.


-- 
To view, visit https://gerrit.ovirt.org/56558
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6f87c954031842c35c99888c228a34ec7f19d800
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

Reply via email to