|
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 |
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 ?
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 nowstatic 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.
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
[Prev in Thread] | Current Thread | [Next in Thread] |