Het Gala <het.g...@nutanix.com> writes:
> On 24/02/24 1:42 am, Fabiano Rosas wrote:
>> Het Gala<het.g...@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/ ?
>
Isn't this what the various qobject_from_json do? How does it work with
the existing tests?
qtest_qmp_assert_success(to, "{ 'execute': 'migrate-incoming',"
" 'arguments': { "
" 'channels': [ { 'channel-type': 'main',"
" 'addr': { 'transport': 'socket',"
" 'type': 'inet',"
" 'host': '127.0.0.1',"
" 'port': '0' } } ] } }");
We can pass this^ string successfully to QMP somehow...
>> 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
