Fabiano Rosas <faro...@suse.de> writes:
> Het Gala <het.g...@nutanix.com> writes:
>
>> On 27/02/24 1:04 am, Het Gala wrote:
>>>
>>>
>>> On 26/02/24 6:31 pm, Fabiano Rosas wrote:
>>>> Het Gala<het.g...@nutanix.com> writes:
>>>>
>>>>> On 24/02/24 1:42 am, Fabiano Rosas wrote:
>>>>> 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...
>>>
>>> I think, here in qtest_qmp_assert_success, we actually can pass the
>>> whole QMP command, and it just asserts that return key is present in
>>> the response, though I am not very much familiar with qtest codebase
>>> to verify how swiftly we can convert string into an actual QObject.
>>>
>>> [...]
>>>
>> I tried with qobject_from_json type of utility functions and the error I
>> got was this :
>>
>> migration-test: /rpmbuild/SOURCES/qemu/include/qapi/qmp/qobject.h:126:
>> qobject_type: Assertion `QTYPE_NONE < obj->base.type && obj->base.type <
>> QTYPE__MAX' failed.
>>
>> And I suppose this was the case because, there are only limited types of
>> QTYPE available
>>
>> typedefenumQType{
>> QTYPE_NONE,
>> QTYPE_QNULL,
>> QTYPE_QNUM,
>> QTYPE_QSTRING,
>> QTYPE_QDICT,
>> QTYPE_QLIST,
>> QTYPE_QBOOL,
>> QTYPE__MAX,
>> } QType;
>>
>> And 'channels' is a mixture of QDICT and QLIST and hence it is not able
>> to easily convert from string to json.
>>
>> Thoughts on this ?
>
> I'm not sure what you tried. This works:
>
> g_assert(!qdict_haskey(args, "channels"));
> if (channels) {
> channels_obj = qobject_from_json(channels, errp);
> qdict_put_obj(args, "channels", channels_obj);
> }
>
> And in the test:
>
> .connect_channels = "[ { 'channel-type': 'main',"
> " 'addr': { 'transport': 'socket',"
> " 'type': 'inet',"
> " 'host': '127.0.0.1',"
> " 'port': '0' } } ]",
> .listen_uri = "tcp:127.0.0.1:0",
> .result = MIG_TEST_QMP_ERROR
>
> However, the real issue is how to inject the port for the source
> migration. The example above only works for the tests that are expected
> to fail. For a test that should pass, 0 as a port does not work.
>
> I'm thinking it might be better to alter migrate_qmp like this:
>
> void migrate_qmp(QTestState *from, QTestState *to, const char *channels,
> const char *fmt, ...)
>
> Invocations would be:
>
> migrate_qmp(from, to, NULL, "{uri: %s}", connect_uri);
> migrate_qmp(from, to, args->channels, "{}");
>
> In this last case, if the test provided a port, we use it, otherwise we
> resolve it from the 'to' instance and put it in the QDict directly.
>
> I'll play with this a bit more tomorrow, let me know what you think.
Ok, so here's what I think we should do:
1) Add the 'to' object into migrate_qmp(), so we can use
migrate_get_socket_address() to get the port. Leave the other
migrate_qmp* alone because they don't need the port;
2) Move the calls to migrate_get_socket_address() into migrate_qmp() and
get rid of that connect_uri, use args->connect_uri instead everywhere;
3) Add a new function SocketAddress_to_qdict() that does the same as
SocketAddress_to_str(), but fills in a QDict instead;
4) Add args->connect_channels and pass it to migrate_qmp() and
migrate_qmp_fail(). Convert the string to a dict;
if (channels) {
channels_obj = qobject_from_json(channels, &error_abort);
qdict_put_obj(args, "channels", channels_obj);
}
5) Add a migrate_set_ports() function that uses 3) and fills in the port
in case it was 0 in the test. Handle a list of channels so we can add a
negative test that passes two channels;
addr = migrate_get_socket_address(to);
for each channel:
if channel["port"] && channel["port"] == "0":
channel["port"] = SocketAddress_to_qdict(addr->value)["port"]
(optionally iterate over addr to be prepared for when we add more
channels)
6) Alter migrate_qmp_fail() to allow both uri and channels
independently. No dealing with migrate_get_socket_address() because we
will fail before starting the migration anyway;
if (uri) {
qdict_put_str(args, "uri", uri);
}
if (channels) {
channels_obj = qobject_from_json(channels, &error_abort);
qdict_put_obj(args, "channels", channels_obj);
}
7) Alter migrate_qmp() to only fill the uri if there are no
channels. Here we don't want to allow the wrong cases of having both or
none;
if (uri) {
qdict_put_str(args, "uri", uri);
} else if (!channels) {
qdict_put_str(args, "uri", migrate_get_socket_address(to));
}
if (channels) {
channels_obj = qobject_from_json(channels, &error_abort);
migrate_set_ports(to, qobject_to(QList, channels_obj));
qdict_put_obj(args, "channels", channels_obj);
}
8) Write the tests to pass the list of channels;
.connect_channels = "[ { 'channel-type': 'main',"
" 'addr': { 'transport': 'socket',"
" 'type': 'inet',"
" 'host': '127.0.0.1',"
" 'port': '0' } } ]",
With this we can test multiple channels, test other types of channel,
add more channel types in the future, etc and not need to change the
test infra.
