Re: [PATCH 4/5] migration: refactor ram_save_target_page functions

[Peter Xu <peterx@redhat.com>]

On Mon, Nov 04, 2024 at 05:26:45PM +0530, Prasad Pandit wrote:
> On Fri, 1 Nov 2024 at 20:09, Peter Xu <peterx@redhat.com> wrote:
> > > +    if (migrate_multifd()) {
> > > +        RAMBlock *block = pss->block;
> > > +        /*
> > > +         * While using multifd live migration, we still need to handle zero
> > > +         * page checking on the migration main thread.
> > > +         */
> > > +        if (migrate_zero_page_detection() == ZERO_PAGE_DETECTION_LEGACY) {
> > > +            if (save_zero_page(rs, pss, offset)) {
> > > +                return 1;
> > > +            }
> >          }
> > There's one more save_zero_page() below.  Please consider properly merging them.
> 
>             if (save_zero_page(rs, pss, offset)) {
>                 return 1;
>             }
> 
> * First is called in migrate_multifd() mode, the second (above) is
> called in non-multifd mode. Will check how/if we can conflate them.

Yes, IMHO it's better when merged.

One more note here, that even with ZERO_PAGE_DETECTION_MULTIFD, qemu will
fallback to use LEGACY in reality when !multifd before.  We need to keep
that behavior.

> 
> > >      migration_ops = g_malloc0(sizeof(MigrationOps));
> > > +    migration_ops->ram_save_target_page = ram_save_target_page_common;
> >
> > If we want to merge the hooks, we should drop the hook in one shot, then
> > call the new function directly.
> >
> 
> * Ie. drop the 'migration_ops' object altogether? And call
> ram_save_target_page() as it used to be before multifd mode?

Yes.

Thanks,

-- 
Peter Xu