qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 1/3] qtest: migration: Enhance qtest migration functions t


From: Het Gala
Subject: Re: [PATCH v2 1/3] qtest: migration: Enhance qtest migration functions to support 'channels' argument
Date: Sat, 24 Feb 2024 18:18:02 +0530
User-agent: Mozilla Thunderbird


On 24/02/24 1:42 am, Fabiano Rosas wrote:
Het Gala <het.gala@nutanix.com> writes:

Introduce support for adding a 'channels' argument to migrate_qmp_fail,
migrate_incoming_qmp and migrate_qmp functions within the migration qtest
framework, enabling enhanced control over migration scenarios.
Can't we just pass a channels string like you did in the original series
with migrate_postcopy_prepare?

We'd change migrate_* functions like this:

  void migrate_qmp(QTestState *who, const char *uri, const char *channels,
                   const char *fmt, ...)
  {
  ...
      g_assert(!qdict_haskey(args, "uri"));
      if (uri) {
          qdict_put_str(args, "uri", uri);
      }
  
      g_assert(!qdict_haskey(args, "channels"));
      if (channels) {
          qdict_put_str(args, "channels", channels);
      }
  }

Write the test like this:

  static void test_multifd_tcp_none_channels(void)
  {
      MigrateCommon args = {
          .listen_uri = "defer",
          .start_hook = test_migrate_precopy_tcp_multifd_start,
          .live = true,
          .connect_channels = "'channels': [ { 'channel-type': 'main',"
                              "      'addr': { 'transport': 'socket',"
                              "                'type': 'inet',"
                              "                'host': '127.0.0.1',"
                              "                'port': '0' } } ]",
          .connect_uri = NULL;
                               
      };
      test_precopy_common(&args);
  }

this was the same first approach that I attempted. It won't work because

The final 'migrate' QAPI with channels string would look like

{ "execute": "migrate", "arguments": { "channels": "[ { "channel-type": "main", "addr": { "transport": "socket", "type": "inet", "host": "10.117.29.84", "port": "4000" }, "multifd-channels": 2 } ]" } }

instead of

{ "execute": "migrate", "arguments": { "channels": [ { "channel-type": "main", "addr": { "transport": "socket", "type": "inet", "host": "10.117.29.84", "port": "4000" }, "multifd-channels": 2 } ] } }

It would complain, that channels should be an array and not a string.

So, that's the reason parsing was required in qtest too.

I would be glad to hear if there are any ideas to convert string -> json object -> add it inside qdict along with uri ?

  static void do_test_validate_uri_channel(MigrateCommon *args)
  {
      QTestState *from, *to;
      g_autofree char *connect_uri = NULL;
  
      if (test_migrate_start(&from, &to, args->listen_uri, &args->start)) {
          return;
      }
  
      wait_for_serial("src_serial");
  
      if (args->result == MIG_TEST_QMP_ERROR) {
          migrate_qmp_fail(from, args->connect_uri, args->connect_channels, "{}");
      } else {
          migrate_qmp(from, args->connect_uri, args->connect_channels, "{}");
      }
  
      test_migrate_end(from, to, false);
  }

It's better to require test writers to pass in their own uri and channel
strings. Otherwise any new transport added will require people to modify
these conversion helpers.
I agree with your point here. I was thinking to have a general but a hacky version of migrate_uri_parse() but that too seemed like a overkill. I don't have a better solution to this right now
Also, using the same string as the user would use in QMP helps with
development in general. One could refer to the tests to see how to
invoke the migration or experiment with the string in the tests during
development.

For examples, I think - enough examples with 'channel' argument are provided where 'migrate' QAPI is defined. users can directly copy the qmp command from there itself.


Regards,

Het Gala


reply via email to

[Prev in Thread] Current Thread [Next in Thread]