Re: [Qemu-devel] [PATCH v11 2/4] migration: Create socket-address parameter

[Juan Quintela <quintela@redhat.com>]

Eric Blake <eblake@redhat.com> wrote:
> On 1/30/19 6:06 AM, Juan Quintela wrote:
>> It will be used to store the uri parameters. We want this only for
>> tcp, so we don't set it for other uris.  We need it to know what port
>> is migration running.
>> 
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> 
>> +        return g_strdup_printf("fd:%s", addr->u.fd.str);
>> +        break;
>> +    default:
>> +        abort();
>
> Is abort()ing on SOCKET_ADDRESS_TYPE_VSOCK intentional?

No.  Althought migration has no clue about how to handle VSOCK.  Changed
it to return something sensible for vsock and an "unknown" message for default.

>
>> +    }
>> +}
>> +
>>  void hmp_info_migrate(Monitor *mon, const QDict *qdict)
>>  {
>>      MigrationInfo *info;
>> @@ -306,6 +326,17 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
>>          g_free(str);
>>          visit_free(v);
>>      }
>> +    if (info->has_socket_address) {
>> +        SocketAddressList *addr;
>> +
>> +        monitor_printf(mon, "socket address: [\n");
>> +
>> +        for (addr = info->socket_address; addr; addr = addr->next) {
>> +            char *s = SocketAddress_to_str(addr->value);
>> +            monitor_printf(mon, "\t%s\n", s);
>> +        }
>> +        monitor_printf(mon, "]\n");
>> +    }
>
> Leaks 's'.

Good catch.

>> @@ -199,6 +200,8 @@
>>  # @compression: migration compression statistics, only returned if compression
>>  #           feature is on and status is 'active' or 'completed' (Since 3.1)
>>  #
>> +# @socket-address: Only used for tcp, to know what the real port is (Since 3.1)
>> +#
>
> s/3.1/4.0/

Done.

>> +##
>> +# @DummyStruct:
>> +#
>> +# Both block-core and migration needs SocketAddressList
>> +# I am open to comments about how to share it
>> +#
>> +# @dummy-list: A dummy list
>> +#
>> +# Since: 3.1
>
> s/3.1/4.0/
> This matches how we force list types elsewhere for use in C code even
> when the type is not exposed through QMP. But do you still need to force
> the generation, given that ['SocketAddress'] used in MigrationInfo is
> visible through QMP?

I think so but I am not an expert here.

Later, Juan.