Daniel P. Berrange <berra...@redhat.com> wrote:
> On Wed, Mar 07, 2018 at 11:59:49AM +0100, Juan Quintela wrote:
>> It will be used to store the uri tcp_port parameter. This is the only
>> parameter than can change and we can need to be able to connect to it.
>>
>> Signed-off-by: Juan Quintela <quint...@redhat.com>
>>
>> --
>> # TODO either fuse back into MigrationParameters, or make
>> @@ -584,7 +591,8 @@
>> '*block-incremental': 'bool',
>> '*x-multifd-channels': 'int',
>> '*x-multifd-page-count': 'int',
>> - '*xbzrle-cache-size': 'size' } }
>> + '*xbzrle-cache-size': 'size',
>> + '*x-tcp-port': 'uint16'} }
>
> This should not exist - this exposes this parameter in migate-set-parameters
> as a end user settable property, which is definitely not desirable.
>
> It is only something we should report with 'query-migrate' / 'info migrate'
Oops, my understanding was that the three places have to be in sync.
Now I stand corrected. Thanks.
>
>> # @migrate-set-parameters:
>> @@ -667,6 +675,10 @@
>> # needs to be a multiple of the target page size
>> # and a power of 2
>> # (Since 2.11)
>> +#
>> +# @x-tcp-port: Only used for tcp, to know what the real port is
>> +# (Since 2.12)
>> +#
>> # Since: 2.4
>> ##
>> { 'struct': 'MigrationParameters',
>> @@ -683,7 +695,8 @@
>> '*block-incremental': 'bool' ,
>> '*x-multifd-channels': 'uint8',
>> '*x-multifd-page-count': 'uint32',
>> - '*xbzrle-cache-size': 'size' } }
>> + '*xbzrle-cache-size': 'size',
>> + '*x-tcp-port': 'uint16'} }
>
> As mentioned in previous review, IMHO we should be reporting the full
> socket address, and allow an array of them, since we're going to have
> more than one address available. i.e.
>
> '*socket-address': ['SocketAddress']
This is weird, really weird. But was done.
- this needs to be copied, because it is a pointer, so we end needing
QAPI_CLONE(), yes, I know.
- it is really weird that I have to:
rsp_return = qdict_get_qdict(rsp, "return");
object = qdict_get(rsp_return, parameter);
iv = qobject_input_visitor_new(object);
visit_type_SocketAddress(iv, NULL, &saddr, &local_err);
result = g_strdup_printf("%s",
SocketAddress_to_str("", saddr, false, false));
QDECREF(rsp);
Remember, it is a *series* of strings in the ddict, we have code to
_print_ qdicts, as info migrate works. But I haven't found an easier
way of getting from a qdict to an string than:
* parsing it
* convert it back to text
sniff
> It doesn't cover non-socket based URIs, but that's fine, because for those
> the mgmt app already knows how the channel is setup. We just need the
> array of SocketAddress, because for socket URIs, the hostname, gets turned
> into an array of addresses, and the mgmt app can't discover them.
SocketAddress are a mess, there is not a single example that I can find
on how to use it on the whole tree. I *think* that I did that right,
will see your comments on the next post.
Later, Juan.
