Re: [PATCH v5 19/23] migration/multifd: Prepare multifd sync for mapped-ram migration

[Peter Xu <peterx@redhat.com>]

On Thu, Feb 29, 2024 at 10:19:12AM -0300, Fabiano Rosas wrote:
> > IMHO EOS cannot be accounted as "invalid" here because it always exists.
> > Rather than this trick (then explicitly ignore it below... which is even
> > hackier, IMHO), we can avoid setting EOS in invalid_flags, but explicitly
> > ignore EOS in below code to bypass it for mapped-ram:
> >
> > @@ -4301,7 +4302,12 @@ static int ram_load_precopy(QEMUFile *f)
> >          case RAM_SAVE_FLAG_EOS:
> >              /* normal exit */
> >              if (migrate_multifd() &&
> > -                migrate_multifd_flush_after_each_section()) {
> > +                migrate_multifd_flush_after_each_section() &&
> > +                /*
> > +                 * Mapped-ram migration flushes once and for all after
> > +                 * parsing ramblocks.  Always ignore EOS for it.
> > +                 */
> > +                !migrate_mapped_ram()) {
> >                  multifd_recv_sync_main();
> >              }
> >              break;
> 
> I thought we were already spraying too many migrate_mapped_ram() checks
> all over the code. But wat you said makes sense, I'll change it.

Yep that's not good, but I can't think of anything better yet and
simple. E.g. we could have some flag so ram_save_iterate()/etc. generates
nothing to the stream but only update the pages with the offsets, then we
don't need this at all and EOS can be legally accounted as invalid.  But
that can involve more changes and not helpful on this series to converge.

And it's also the long condition which makes me even more worry.. For the
long run I think we should cleanup most of these "multifd &&
after_flush_each_section && !mapped_ram" at some point..

-- 
Peter Xu