Het Gala <het.g...@nutanix.com> wrote:
> On 15/05/23 4:12 pm, Daniel P. Berrangé wrote:
>> On Fri, May 12, 2023 at 02:32:40PM +0000, Het Gala wrote:
>>> Integrated MigrateChannelList with all transport backends (socket, exec
>>> and rdma) for both source and destination migration code flow.
>>>
>>> Suggested-by: Aravind Retnakaran <aravind.retnaka...@nutanix.com>
>>> Signed-off-by: Het Gala <het.g...@nutanix.com>
>>> ---
>>> migration/migration.c | 95 +++++++++++++++++++++++++++----------------
>>> migration/socket.c | 5 ++-
>>> 2 files changed, 64 insertions(+), 36 deletions(-)
>>>
>>> Error *local_err = NULL;
>>> + MigrateChannel *val = g_new0(MigrateChannel, 1);
>>> MigrateAddress *addrs = g_new0(MigrateAddress, 1);
>>> SocketAddress *saddr;
>>> InetSocketAddress *isock = &addrs->u.rdma;
>>> @@ -441,6 +442,7 @@ static bool migrate_uri_parse(const char *uri,
>>> }
>>> if (local_err) {
>>> + qapi_free_MigrateChannel(val);
>>> qapi_free_MigrateAddress(addrs);
>>> qapi_free_SocketAddress(saddr);
>>> qapi_free_InetSocketAddress(isock);
>>> @@ -448,7 +450,9 @@ static bool migrate_uri_parse(const char *uri,
>>> return false;
>>> }
>>> - *channel = addrs;
>>> + val->channeltype = MIGRATE_CHANNEL_TYPE_MAIN;
>>> + val->addr = addrs;
>>> + *channel = val;
>>> return true;
>>> }
>>> @@ -457,8 +461,9 @@ static void
>>> qemu_start_incoming_migration(const char *uri, bool has_channels,
>>> Error **errp)
>>> {
>>> Error *local_err = NULL;
>>> - MigrateAddress *channel = g_new0(MigrateAddress, 1);
>>> + MigrateAddress *addrs;
>>> SocketAddress *saddr;
>>> + MigrateChannel *channel = NULL;
>>> /*
>>> * Having preliminary checks for uri and channel
>>> @@ -467,22 +472,30 @@ static void qemu_start_incoming_migration(const char
>>> *uri, bool has_channels,
>>> error_setg(errp, "'uri' and 'channels' arguments are mutually "
>>> "exclusive; exactly one of the two should be present
>>> in "
>>> "'migrate-incoming' qmp command ");
>>> - return;
>>> - }
>>> -
>>> - /* URI is not suitable for migration? */
>>> - if (!migration_channels_and_uri_compatible(uri, errp)) {
>>> goto out;
>>> - }
>>> + } else if (channels) {
>>> + /* To verify that Migrate channel list has only item */
>>> + if (channels->next) {
>>> + error_setg(errp, "Channel list has more than one entries");
>>> + goto out;
>>> + }
>>> + channel = channels->value;
>>> + } else {
>>> + /* URI is not suitable for migration? */
>>> + if (!migration_channels_and_uri_compatible(uri, errp)) {
>>> + goto out;
>>> + }
>> THis check only gets executed when the caller uses the old
>> URI syntax. We need to it be run when using the modern
>> MigrateChannel QAPI syntax too.
>>
>> IOW, migration_channels_and_uri_compatible() needs converting
>> to take a 'MigrateChannel' object instead of URI, and then
>> the check can be run after the URI -> MigrateCHannel conversion
>
> Yes, Daniel. Got your point. Will change it in the next version.
>
> For Juan's comments, I have not explored the test side code still, so
> is the idea to have some similar test functions like
> test_precopy_tcp_plain, test_precopy_unix_plain but with the new
> syntax ? Please confirm this, and any advice on how appraoch this?
There are several places that use this syntax:
rsp = wait_command(to, "{ 'execute': 'migrate-incoming',"
" 'arguments': { 'uri': 'tcp:127.0.0.1:0' }}");
Just change a couple of them to the new syntax.
I guess you will want to do the multifd tests with the new syntax, not
sure if it makes sense to also test the old one.
I guess you will also want to check that your are catching errors (like
different number of channels on source/destination).
Later, Juan.
