| Message ID | 20230710122750.69194-2-het.gala@nutanix.com |
|---|---|
| State | New |
| Headers | show |
| Series | migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration | expand |
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'
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.
> 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.
Acked-by: Markus Armbruster <armbru@redhat.com>
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
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: #