Het Gala <het.g...@nutanix.com> writes:
> On 06/03/24 9:31 pm, Fabiano Rosas wrote:
>> Het Gala<het.g...@nutanix.com> writes:
>>
>>> On 06/03/24 8:06 pm, Fabiano Rosas wrote:
>>>> Het Gala<het.g...@nutanix.com> writes:
>>>>
>>>>> Add a migrate_set_ports() function that from each QDict, 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 more than one channel.
>>>>>
>>>>> Signed-off-by: Het Gala<het.g...@nutanix.com>
>>>>> Suggested-by: Fabiano Rosas<faro...@suse.de>
>>>>> ---
>>>>> tests/qtest/migration-helpers.c | 26 ++++++++++++++++++++++++++
>>>>> 1 file changed, 26 insertions(+)
>>>>>
>>>>> diff --git a/tests/qtest/migration-helpers.c
>>>>> b/tests/qtest/migration-helpers.c
>>>>> index 478c1f259b..df4978bf17 100644
>>>>> --- a/tests/qtest/migration-helpers.c
>>>>> +++ b/tests/qtest/migration-helpers.c
>>>>> @@ -17,6 +17,8 @@
>>>>> #include "qapi/qapi-visit-sockets.h"
>>>>> #include "qapi/qobject-input-visitor.h"
>>>>> #include "qapi/error.h"
>>>>> +#include "qapi/qmp/qlist.h"
>>>>> +
>>>> Extra line here. This is unwanted because it sometimes trips git into
>>>> thinking there's a conflict here when another patch changes the
>>>> surrounding lines.
>>> Ack, that makes sense
>>>>>
>>>>> #include "migration-helpers.h"
>>>>>
>>>>> @@ -73,6 +75,29 @@ migrate_get_socket_address(QTestState *who, const char
>>>>> *parameter)
>>>>> return result;
>>>>> }
>>>>>
>>>>> +static void migrate_set_ports(QTestState *to, QList *channelList)
>>>>> +{
>>>>> + g_autofree char *addr = NULL;
>>>>> + g_autofree char *addr_port = NULL;
>>>>> + QListEntry *entry;
>>>>> +
>>>>> + addr = migrate_get_socket_address(to, "socket-address");
>>>>> + addr_port = g_strsplit(addr, ":", 3)[2];
>>>> Will this always do the right thing when the src/dst use different types
>>>> of channels? If there is some kind of mismatch (say one side uses vsock
>>>> and the other inet), it's better that this function doesn't touch the
>>>> channels dict instead of putting garbage in the port field.
>>> Yes you are right. This will fail if there is a mismatch in type of
>>> channels.
>>>
>>> Better idea would be to check if 'port' key is present in both, i.e. in
>>> 'addr'
>>> as well as 'addrdict' and only then change the port ?
>>>
>> Yep, either parse the type from string or add a version of
>> migrate_get_socket_address that returns a dict. Then check if type
>> matches and port exists.
>
> one silly question here, why are we not having tests for exec and rdma
> specifically ?
exec because we intend to deprecate it, so no one is paying too much
attention to it.
rdma because no one wants to write them.
>
> Another suggestion required: Parsing uri to qdict is easy to implement
> but (little)
> messy codewise, and the other hand migrate_get_qdict looks clean, but
> under the hood we would convert it to socketaddress and then call
> SocketAddress_to_qdict. Which one we can prefer more here ?
The latter. It's easier to work with.
static QDict *SocketAddress_to_qdict(SocketAddress *addr)
{
QDict *dict = qdict_new();
switch (addr->type) {
case SOCKET_ADDRESS_TYPE_INET:
qdict_put_str(dict, "type", "inet");
qdict_put_str(dict, "host", addr->u.inet.host);
qdict_put_str(dict, "port", addr->u.inet.port);
break;
case SOCKET_ADDRESS_TYPE_UNIX:
qdict_put_str(dict, "type", "unix");
qdict_put_str(dict, "path", addr->u.q_unix.path);
break;
case SOCKET_ADDRESS_TYPE_FD:
qdict_put_str(dict, "type", "fd");
qdict_put_str(dict, "str", addr->u.fd.str);
break;
case SOCKET_ADDRESS_TYPE_VSOCK:
qdict_put_str(dict, "type", "vsock");
qdict_put_str(dict, "cid", addr->u.vsock.cid);
qdict_put_str(dict, "port", addr->u.vsock.port);
break;
default:
g_assert_not_reached();
break;
}
return dict;
}
>
> Regards,
>
> Het Gala
