Re: [PATCH v3 6/7] Add multifd_tcp_plain test using list of channels instead of uri

[Het Gala]

On 06/03/24 8:37 pm, Fabiano Rosas wrote:

> Het Gala <het.gala@nutanix.com> writes:
>
> > Add a positive test to check multifd live migration but this time
> > using list of channels (restricted to 1) as the starting point
> > instead of simple uri string.
> >
> > Signed-off-by: Het Gala <het.gala@nutanix.com>
> > Suggested-by: Fabiano Rosas <farosas@suse.de>
> > ---
> >  tests/qtest/migration-test.c | 29 ++++++++++++++++++++++++++---
> >  1 file changed, 26 insertions(+), 3 deletions(-)
> >
> > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> > index f94fe713b2..05e5f3ebe5 100644
> > --- a/tests/qtest/migration-test.c
> > +++ b/tests/qtest/migration-test.c
> > @@ -659,6 +659,12 @@ typedef struct {
> >       */
> >      const char *connect_uri;
> >  
> > +    /*
> > +     * Optional: the JSON formatted list of URIs for the src
> > +     * QEMU to connect to
> > +     */
> >       
> You could add some words here mentioning that a port of '0' will be
> automatically converted to the correct port that the destination is
> using.
>     
    Ack, will add these while defining
      connect_channels.

> > +    const char *connect_channels;
> > +
> >      /* Optional: callback to run at start to set migration parameters */
> >      TestMigrateStartHook start_hook;
> >      /* Optional: callback to run at finish to cleanup */
> > @@ -2623,7 +2629,7 @@ test_migrate_precopy_tcp_multifd_zstd_start(QTestState *from,
> >  }
> >  #endif /* CONFIG_ZSTD */
> >  
> > -static void test_multifd_tcp_none(void)
> > +static void test_multifd_tcp_uri_none(void)
> >  {
> >      MigrateCommon args = {
> >          .listen_uri = "defer",
> > @@ -2638,6 +2644,21 @@ static void test_multifd_tcp_none(void)
> >      test_precopy_common(&args);
> >  }
> >  
> > +static void test_multifd_tcp_channels_none(void)
> > +{
> > +    MigrateCommon args = {
> > +        .listen_uri = "defer",
> > +        .start_hook = test_migrate_precopy_tcp_multifd_start,
> > +        .live = true,
> > +        .connect_channels = "[ { 'channel-type': 'main',"
> > +                            "    'addr': { 'transport': 'socket',"
> > +                            "              'type': 'inet',"
> > +                            "              'host': '127.0.0.1',"
> > +                            "              'port': '0' } } ]",
> > +    };
> > +    test_precopy_common(&args);
> > +}
> > +
> >  static void test_multifd_tcp_zlib(void)
> >  {
> >      MigrateCommon args = {
> > @@ -3531,8 +3552,10 @@ int main(int argc, char **argv)
> >                                 test_migrate_dirty_limit);
> >          }
> >      }
> > -    migration_test_add("/migration/multifd/tcp/plain/none",
> > -                       test_multifd_tcp_none);
> > +    migration_test_add("/migration/multifd/tcp/uri/plain/none",
> > +                       test_multifd_tcp_uri_none);
> > +    migration_test_add("/migration/multifd/tcp/channels/plain/none",
> > +                       test_multifd_tcp_channels_none);
> >       
> We should eventually make a pass to standardize/simplify these
> strings. We could have a little "grammar" defined on how to construct
> them.
>
> /<test-type>/<migration-mode>/<transport>/<invocation>/<compression>/<encryption>
>
> test-type      :: migrate | validate
> migration-mode :: multifd | precopy | postcopy
> transport      :: tcp | fd | unix | file
> invocation     :: uri | channels
> compression    :: zlib | zstd | none
> encryption     :: tls | plain
>
> Anyway, work for the future.
>

Yes, completely agree with you. It makes it much easier for people to identify and define every test.

I can take this up as a separate patchset after this one gets merged maybe

Regards,

Het Gala