Re: [External] Re: [PATCH v2 5/7] migration/multifd: Add new migration test cases for legacy zero page checking.

[Hao Xiang <hao.xiang@bytedance.com>]

On Wed, Feb 21, 2024 at 12:59 PM Fabiano Rosas <farosas@suse.de> wrote:
>
> Hao Xiang <hao.xiang@bytedance.com> writes:
>
> > Now that zero page checking is done on the multifd sender threads by
> > default, we still provide an option for backward compatibility. This
> > change adds a qtest migration test case to set the zero-page-detection
> > option to "legacy" and run multifd migration with zero page checking on the
> > migration main thread.
> >
> > Signed-off-by: Hao Xiang <hao.xiang@bytedance.com>
> > ---
> >  tests/qtest/migration-test.c | 52 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 52 insertions(+)
> >
> > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> > index 8a5bb1752e..c27083110a 100644
> > --- a/tests/qtest/migration-test.c
> > +++ b/tests/qtest/migration-test.c
> > @@ -2621,6 +2621,24 @@ test_migrate_precopy_tcp_multifd_start(QTestState *from,
> >      return test_migrate_precopy_tcp_multifd_start_common(from, to, "none");
> >  }
> >
> > +static void *
> > +test_migrate_precopy_tcp_multifd_start_zero_page_legacy(QTestState *from,
> > +                                                        QTestState *to)
> > +{
> > +    test_migrate_precopy_tcp_multifd_start_common(from, to, "none");
> > +    migrate_set_parameter_str(from, "zero-page-detection", "legacy");
> > +    return NULL;
> > +}
> > +
> > +static void *
> > +test_migration_precopy_tcp_multifd_start_no_zero_page(QTestState *from,
> > +                                                      QTestState *to)
> > +{
> > +    test_migrate_precopy_tcp_multifd_start_common(from, to, "none");
> > +    migrate_set_parameter_str(from, "zero-page-detection", "none");
> > +    return NULL;
> > +}
> > +
> >  static void *
> >  test_migrate_precopy_tcp_multifd_zlib_start(QTestState *from,
> >                                              QTestState *to)
> > @@ -2652,6 +2670,36 @@ static void test_multifd_tcp_none(void)
> >      test_precopy_common(&args);
> >  }
> >
> > +static void test_multifd_tcp_zero_page_legacy(void)
> > +{
> > +    MigrateCommon args = {
> > +        .listen_uri = "defer",
> > +        .start_hook = test_migrate_precopy_tcp_multifd_start_zero_page_legacy,
> > +        /*
> > +         * Multifd is more complicated than most of the features, it
> > +         * directly takes guest page buffers when sending, make sure
> > +         * everything will work alright even if guest page is changing.
> > +         */
> > +        .live = true,
> > +    };
> > +    test_precopy_common(&args);
> > +}
> > +
> > +static void test_multifd_tcp_no_zero_page(void)
> > +{
> > +    MigrateCommon args = {
> > +        .listen_uri = "defer",
> > +        .start_hook = test_migration_precopy_tcp_multifd_start_no_zero_page,
> > +        /*
> > +         * Multifd is more complicated than most of the features, it
> > +         * directly takes guest page buffers when sending, make sure
> > +         * everything will work alright even if guest page is changing.
> > +         */
> > +        .live = true,
> > +    };
> > +    test_precopy_common(&args);
> > +}
> > +
> >  static void test_multifd_tcp_zlib(void)
> >  {
> >      MigrateCommon args = {
> > @@ -3550,6 +3598,10 @@ int main(int argc, char **argv)
> >      }
> >      migration_test_add("/migration/multifd/tcp/plain/none",
> >                         test_multifd_tcp_none);
> > +    migration_test_add("/migration/multifd/tcp/plain/zero_page_legacy",
> > +                       test_multifd_tcp_zero_page_legacy);
> > +    migration_test_add("/migration/multifd/tcp/plain/no_zero_page",
> > +                       test_multifd_tcp_no_zero_page);
>
> Here it's better to separate the main feature from the states. That way
> we can run only the zero-page tests with:
>
>  migration-test -r /x86_64/migration/multifd/tcp/plain/zero-page
>
> Like so: (also dashes instead of underscores)
> /zero-page/legacy
> /zero-page/none
>

Sounds good.

> >      migration_test_add("/migration/multifd/tcp/plain/cancel",
> >                         test_multifd_tcp_cancel);
> >      migration_test_add("/migration/multifd/tcp/plain/zlib",