On Fri, Apr 21, 2023 at 11:54:55PM +0200, Juan Quintela wrote: > Daniel P. Berrangé <berra...@redhat.com> wrote: > > The 'unsigned int interations' config for migration is somewhat > > overkill. Most tests don't set it, and a value of '0' is treated > > as equivalent to '1'. The only test that does set it, xbzrle, > > used a value of '2'. > > > > This setting, however, only relates to the migration iterations > > that take place prior to allowing convergence. IOW, on top of > > this iteration count, there is always at least 1 further migration > > iteration done to deal with pages that are dirtied during the > > previous iteration(s). > > > > IOW, even with iterations==1, the xbzrle test will be running for > > a minimum of 2 iterations. With this in mind we can simplify the > > code and just get rid of the special case. > > Perhaps the old code was already wrong, but we need at least three > iterations for the xbzrle test: > - 1st iteration: xbzrle is not used, nothing is on cache.
Are you sure about this ? I see ram_save_page() calling save_xbzrle_page() and unless I'm mis-understanding the code, it doesn't appear to skip anything on the 1st iteration. IIUC save_xbzrle_page will add pages into the cache on the first iteration, so the second iteration will get cache hits > - 2nd iteration: pages are put into cache, no xbzrle is used because > there is no previous page. > - 3rd iteration: We really use xbzrle now against the copy of the > previous iterations. > > And yes, this should be commented somewhere. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
