On 12/07/23 6:25 pm, Markus Armbruster wrote:
> The subject
>
> migration: introduced 'MigrateAddress' in QAPI for migration wire protocol.
>
> is rather long. Try to limit subjects to about 60 characters. Easily
> done here:
>
> migration: New QAPI type 'MigrateAddress'
Ack. Thanks for the suggestion Markus.
> Het Gala <het.gala@nutanix.com> writes:
>
>> This patch introduces well defined MigrateAddress struct and its related
>> child objects.
>>
>> The existing argument of 'migrate' and 'migrate-incoming' QAPI - 'uri'
>> is of string type. The current migration flow follows double encoding
>> scheme for fetching migration parameters such as 'uri' and this is
>> not an ideal design.
>>
>> Motive for intoducing struct level design is to prevent double encoding
>> of QAPI arguments, as Qemu should be able to directly use the QAPI
>> arguments without any level of encoding.
> Suggest to mention that this commit only adds the type, and actual uses
> come in later commits.
Ack.
>> Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
>> Signed-off-by: Het Gala <het.gala@nutanix.com>
>> Reviewed-by: Juan Quintela <quintela@redhat.com>
>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>> ---
>> qapi/migration.json | 41 +++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 41 insertions(+)
>>
>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index 47dfef0278..b583642c2d 100644
>> --- a/qapi/migration.json
>> +++ b/qapi/migration.json
>> @@ -1417,6 +1417,47 @@
>> ##
>> { 'command': 'migrate-continue', 'data': {'state': 'MigrationStatus'} }
>>
>> +##
>> +# @MigrationAddressType:
>> +#
>> +# The migration stream transport mechanisms.
>> +#
>> +# @socket: Migrate via socket.
>> +#
>> +# @exec: Direct the migration stream to another process.
>> +#
>> +# @rdma: Migrate via RDMA.
>> +#
>> +# Since 8.1
>> +##
>> +{ 'enum': 'MigrationAddressType',
>> + 'data': ['socket', 'exec', 'rdma'] }
>> +
>> +##
>> +# @MigrationExecCommand:
>> +#
>> +# @args: command (list head) and arguments to execute.
>> +#
>> +# Since 8.1
>> +##
>> +{ 'struct': 'MigrationExecCommand',
>> + 'data': {'args': [ 'str' ] } }
>> +
>> +##
>> +# @MigrationAddress:
>> +#
>> +# Migration endpoint configuration.
>> +#
>> +# Since 8.1
>> +##
>> +{ 'union': 'MigrationAddress',
>> + 'base': { 'transport' : 'MigrationAddressType'},
>> + 'discriminator': 'transport',
>> + 'data': {
>> + 'socket': 'SocketAddress',
>> + 'exec': 'MigrationExecCommand',
>> + 'rdma': 'InetSocketAddress' } }
>> +
>> ##
>> # @migrate:
>> #
> The issues I'm pointing out don't justfy yet another respin. But if you
> need to respin the series for some other reason, plase take care of them.
Ack. I think from QAPI side, the patches look good. But from code
implementation side, I haven't received acked-by or reviewd-by from the
maintainers. So would anyway need to respin the series I think. Cc'ing
Daniel, Peter, Juan and other migration maintainers for reviews on other
patches too.
> Acked-by: Markus Armbruster <armbru@redhat.com>
>
Regards,
Het Gala