| Message ID | 20230208093600.242665-6-het.gala@nutanix.com |
|---|---|
| State | New |
| Headers | show |
| Series | migration: Modified 'migrate' QAPI command for migration | expand |
On Wed, Feb 08, 2023 at 09:35:59AM +0000, Het Gala wrote: > 'migrate-incoming' QAPI design have been modified into well-defined > MigrateChannel struct to prevent multiple encoding of uri strings on > the destination side.'uri' parameter is kept for backward compatibility. > > Suggested-by: Daniel P. Berrange <berrange@redhat.com> > Suggested-by: Manish Mishra <manish.mishra@nutanix.com> > Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com> > Signed-off-by: Het Gala <het.gala@nutanix.com> > --- > migration/migration-hmp-cmds.c | 8 +++++++- > migration/migration.c | 3 ++- > qapi/migration.json | 22 ++++++++++++++++++++-- > softmmu/vl.c | 2 +- > 4 files changed, 30 insertions(+), 5 deletions(-) > +++ b/qapi/migration.json > @@ -1623,7 +1623,11 @@ > # with -incoming defer > # > # @uri: The Uniform Resource Identifier identifying the source or > -# address to listen on > +# the address to listen on > +# > +# @channel: Struct containing migration channel type, along with > +# all the details of the destination interface required > +# for the address to listen on for migration stream. Missing a '(since 8.0)' tag. > # > # Returns: nothing on success > # > @@ -1640,14 +1644,28 @@ > # > # 3. The uri format is the same as for -incoming > # > +# 4. The 'uri' and 'channel' arguments are mutually exclusive but, atleast > +# one of the two arguments should be present. Grammar: The 'uri' and 'channel' arguments are mutually exclusive; exactly one of the two should be present.
On 09/02/23 1:49 am, Eric Blake wrote: > On Wed, Feb 08, 2023 at 09:35:59AM +0000, Het Gala wrote: >> 'migrate-incoming' QAPI design have been modified into well-defined >> MigrateChannel struct to prevent multiple encoding of uri strings on >> the destination side.'uri' parameter is kept for backward compatibility. >> >> Suggested-by: Daniel P. Berrange <berrange@redhat.com> >> Suggested-by: Manish Mishra <manish.mishra@nutanix.com> >> Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com> >> Signed-off-by: Het Gala <het.gala@nutanix.com> >> --- >> migration/migration-hmp-cmds.c | 8 +++++++- >> migration/migration.c | 3 ++- >> qapi/migration.json | 22 ++++++++++++++++++++-- >> softmmu/vl.c | 2 +- >> 4 files changed, 30 insertions(+), 5 deletions(-) >> +++ b/qapi/migration.json >> @@ -1623,7 +1623,11 @@ >> # with -incoming defer >> # >> # @uri: The Uniform Resource Identifier identifying the source or >> -# address to listen on >> +# the address to listen on >> +# >> +# @channel: Struct containing migration channel type, along with >> +# all the details of the destination interface required >> +# for the address to listen on for migration stream. > Missing a '(since 8.0)' tag. Ack. >> # >> # Returns: nothing on success >> # >> @@ -1640,14 +1644,28 @@ >> # >> # 3. The uri format is the same as for -incoming >> # >> +# 4. The 'uri' and 'channel' arguments are mutually exclusive but, atleast >> +# one of the two arguments should be present. > Grammar: > > The 'uri' and 'channel' arguments are mutually exclusive; exactly one > of the two should be present. Ack. Regards, Het Gala.
On Wed, Feb 08, 2023 at 09:35:59AM +0000, Het Gala wrote: > 'migrate-incoming' QAPI design have been modified into well-defined > MigrateChannel struct to prevent multiple encoding of uri strings on > the destination side.'uri' parameter is kept for backward compatibility. > > Suggested-by: Daniel P. Berrange <berrange@redhat.com> > Suggested-by: Manish Mishra <manish.mishra@nutanix.com> > Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com> > Signed-off-by: Het Gala <het.gala@nutanix.com> > --- > migration/migration-hmp-cmds.c | 8 +++++++- > migration/migration.c | 3 ++- > qapi/migration.json | 22 ++++++++++++++++++++-- > softmmu/vl.c | 2 +- > 4 files changed, 30 insertions(+), 5 deletions(-) > > diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c > index a9f65ded7a..ae3c5ea5b2 100644 > --- a/migration/migration-hmp-cmds.c > +++ b/migration/migration-hmp-cmds.c > @@ -500,8 +500,14 @@ void hmp_migrate_incoming(Monitor *mon, const QDict *qdict) > Error *err = NULL; > const char *uri = qdict_get_str(qdict, "uri"); > > - qmp_migrate_incoming(uri, &err); > + MigrateChannel *channel = g_new0(MigrateChannel, 1); > + if (!migrate_channel_from_qdict(&channel, qdict, &err)) { > + error_setg(&err, "error in retrieving channel from qdict"); > + return; > + } > > + qmp_migrate_incoming(uri, channel, &err); > + qapi_free_MigrateChannel(channel); > hmp_handle_error(mon, err); > } > > diff --git a/migration/migration.c b/migration/migration.c > index accbf72a18..e22ce2dd15 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -2314,7 +2314,8 @@ void migrate_del_blocker(Error *reason) > migration_blockers = g_slist_remove(migration_blockers, reason); > } > > -void qmp_migrate_incoming(const char *uri, Error **errp) > +void qmp_migrate_incoming(const char *uri, MigrateChannel *channel, > + Error **errp) > { > Error *local_err = NULL; > static bool once = true; > diff --git a/qapi/migration.json b/qapi/migration.json > index 79acfcfe4e..3a88912f4d 100644 > --- a/qapi/migration.json > +++ b/qapi/migration.json > @@ -1623,7 +1623,11 @@ > # with -incoming defer > # > # @uri: The Uniform Resource Identifier identifying the source or > -# address to listen on > +# the address to listen on > +# > +# @channel: Struct containing migration channel type, along with > +# all the details of the destination interface required > +# for the address to listen on for migration stream. > # > # Returns: nothing on success > # > @@ -1640,14 +1644,28 @@ > # > # 3. The uri format is the same as for -incoming > # > +# 4. The 'uri' and 'channel' arguments are mutually exclusive but, atleast > +# one of the two arguments should be present. > +# > # Example: > # > # -> { "execute": "migrate-incoming", > # "arguments": { "uri": "tcp::4446" } } > # <- { "return": {} } > # > +# -> { "execute": "migrate-incoming", > +# "arguments": { > +# "channel": { "channeltype": "main", > +# "addr": { "transport": "socket", > +# "socket-type": { "type": "inet", > +# "host": "10.12.34.9", > +# "port": "1050" } } } } } > +# <- { "return": {} } > +# > ## > -{ 'command': 'migrate-incoming', 'data': {'uri': 'str' } } > +{ 'command': 'migrate-incoming', > + 'data': {'*uri': 'str', > + '*channel': 'MigrateChannel'} } Same question of whether we need to future proof now by making this '*channels': ['MigrateChannel'] ? Otherwise we'll have to add this 'channels' field later, and end up with 'channel' and 'channels' and 'uri' With regards, Daniel
On 09/02/23 4:01 pm, Daniel P. Berrangé wrote: > On Wed, Feb 08, 2023 at 09:35:59AM +0000, Het Gala wrote: >> 'migrate-incoming' QAPI design have been modified into well-defined >> MigrateChannel struct to prevent multiple encoding of uri strings on >> the destination side.'uri' parameter is kept for backward compatibility. >> >> Suggested-by: Daniel P. Berrange <berrange@redhat.com> >> Suggested-by: Manish Mishra <manish.mishra@nutanix.com> >> Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com> >> Signed-off-by: Het Gala <het.gala@nutanix.com> >> --- >> migration/migration-hmp-cmds.c | 8 +++++++- >> migration/migration.c | 3 ++- >> qapi/migration.json | 22 ++++++++++++++++++++-- >> softmmu/vl.c | 2 +- >> 4 files changed, 30 insertions(+), 5 deletions(-) >> >> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c >> index a9f65ded7a..ae3c5ea5b2 100644 >> --- a/migration/migration-hmp-cmds.c >> +++ b/migration/migration-hmp-cmds.c >> @@ -500,8 +500,14 @@ void hmp_migrate_incoming(Monitor *mon, const QDict *qdict) >> Error *err = NULL; >> const char *uri = qdict_get_str(qdict, "uri"); >> >> - qmp_migrate_incoming(uri, &err); >> + MigrateChannel *channel = g_new0(MigrateChannel, 1); >> + if (!migrate_channel_from_qdict(&channel, qdict, &err)) { >> + error_setg(&err, "error in retrieving channel from qdict"); >> + return; >> + } >> >> + qmp_migrate_incoming(uri, channel, &err); >> + qapi_free_MigrateChannel(channel); >> hmp_handle_error(mon, err); >> } >> >> diff --git a/migration/migration.c b/migration/migration.c >> index accbf72a18..e22ce2dd15 100644 >> --- a/migration/migration.c >> +++ b/migration/migration.c >> @@ -2314,7 +2314,8 @@ void migrate_del_blocker(Error *reason) >> migration_blockers = g_slist_remove(migration_blockers, reason); >> } >> >> -void qmp_migrate_incoming(const char *uri, Error **errp) >> +void qmp_migrate_incoming(const char *uri, MigrateChannel *channel, >> + Error **errp) >> { >> Error *local_err = NULL; >> static bool once = true; >> diff --git a/qapi/migration.json b/qapi/migration.json >> index 79acfcfe4e..3a88912f4d 100644 >> --- a/qapi/migration.json >> +++ b/qapi/migration.json >> @@ -1623,7 +1623,11 @@ >> # with -incoming defer >> # >> # @uri: The Uniform Resource Identifier identifying the source or >> -# address to listen on >> +# the address to listen on >> +# >> +# @channel: Struct containing migration channel type, along with >> +# all the details of the destination interface required >> +# for the address to listen on for migration stream. >> # >> # Returns: nothing on success >> # >> @@ -1640,14 +1644,28 @@ >> # >> # 3. The uri format is the same as for -incoming >> # >> +# 4. The 'uri' and 'channel' arguments are mutually exclusive but, atleast >> +# one of the two arguments should be present. >> +# >> # Example: >> # >> # -> { "execute": "migrate-incoming", >> # "arguments": { "uri": "tcp::4446" } } >> # <- { "return": {} } >> # >> +# -> { "execute": "migrate-incoming", >> +# "arguments": { >> +# "channel": { "channeltype": "main", >> +# "addr": { "transport": "socket", >> +# "socket-type": { "type": "inet", >> +# "host": "10.12.34.9", >> +# "port": "1050" } } } } } >> +# <- { "return": {} } >> +# >> ## >> -{ 'command': 'migrate-incoming', 'data': {'uri': 'str' } } >> +{ 'command': 'migrate-incoming', >> + 'data': {'*uri': 'str', >> + '*channel': 'MigrateChannel'} } > Same question of whether we need to future proof now by making this > > '*channels': ['MigrateChannel'] > > ? > > Otherwise we'll have to add this 'channels' field later, and > end up with 'channel' and 'channels' and 'uri' Why do we need 3 fields ? We can directly convert 'channel' into 'channels'. '*channels' will be of the form ['MigrateChannel']. Ex: In the list the first channel will be of type 'main' and other channel could be of type 'data' for multifd. > With regards, > Daniel Regards, Het Gala
On Thu, Feb 09, 2023 at 06:44:40PM +0530, Het Gala wrote: > > On 09/02/23 4:01 pm, Daniel P. Berrangé wrote: > > On Wed, Feb 08, 2023 at 09:35:59AM +0000, Het Gala wrote: > > > 'migrate-incoming' QAPI design have been modified into well-defined > > > MigrateChannel struct to prevent multiple encoding of uri strings on > > > the destination side.'uri' parameter is kept for backward compatibility. > > > > > > Suggested-by: Daniel P. Berrange <berrange@redhat.com> > > > Suggested-by: Manish Mishra <manish.mishra@nutanix.com> > > > Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com> > > > Signed-off-by: Het Gala <het.gala@nutanix.com> > > > --- > > > migration/migration-hmp-cmds.c | 8 +++++++- > > > migration/migration.c | 3 ++- > > > qapi/migration.json | 22 ++++++++++++++++++++-- > > > softmmu/vl.c | 2 +- > > > 4 files changed, 30 insertions(+), 5 deletions(-) > > > > > > diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c > > > index a9f65ded7a..ae3c5ea5b2 100644 > > > --- a/migration/migration-hmp-cmds.c > > > +++ b/migration/migration-hmp-cmds.c > > > @@ -500,8 +500,14 @@ void hmp_migrate_incoming(Monitor *mon, const QDict *qdict) > > > Error *err = NULL; > > > const char *uri = qdict_get_str(qdict, "uri"); > > > - qmp_migrate_incoming(uri, &err); > > > + MigrateChannel *channel = g_new0(MigrateChannel, 1); > > > + if (!migrate_channel_from_qdict(&channel, qdict, &err)) { > > > + error_setg(&err, "error in retrieving channel from qdict"); > > > + return; > > > + } > > > + qmp_migrate_incoming(uri, channel, &err); > > > + qapi_free_MigrateChannel(channel); > > > hmp_handle_error(mon, err); > > > } > > > diff --git a/migration/migration.c b/migration/migration.c > > > index accbf72a18..e22ce2dd15 100644 > > > --- a/migration/migration.c > > > +++ b/migration/migration.c > > > @@ -2314,7 +2314,8 @@ void migrate_del_blocker(Error *reason) > > > migration_blockers = g_slist_remove(migration_blockers, reason); > > > } > > > -void qmp_migrate_incoming(const char *uri, Error **errp) > > > +void qmp_migrate_incoming(const char *uri, MigrateChannel *channel, > > > + Error **errp) > > > { > > > Error *local_err = NULL; > > > static bool once = true; > > > diff --git a/qapi/migration.json b/qapi/migration.json > > > index 79acfcfe4e..3a88912f4d 100644 > > > --- a/qapi/migration.json > > > +++ b/qapi/migration.json > > > @@ -1623,7 +1623,11 @@ > > > # with -incoming defer > > > # > > > # @uri: The Uniform Resource Identifier identifying the source or > > > -# address to listen on > > > +# the address to listen on > > > +# > > > +# @channel: Struct containing migration channel type, along with > > > +# all the details of the destination interface required > > > +# for the address to listen on for migration stream. > > > # > > > # Returns: nothing on success > > > # > > > @@ -1640,14 +1644,28 @@ > > > # > > > # 3. The uri format is the same as for -incoming > > > # > > > +# 4. The 'uri' and 'channel' arguments are mutually exclusive but, atleast > > > +# one of the two arguments should be present. > > > +# > > > # Example: > > > # > > > # -> { "execute": "migrate-incoming", > > > # "arguments": { "uri": "tcp::4446" } } > > > # <- { "return": {} } > > > # > > > +# -> { "execute": "migrate-incoming", > > > +# "arguments": { > > > +# "channel": { "channeltype": "main", > > > +# "addr": { "transport": "socket", > > > +# "socket-type": { "type": "inet", > > > +# "host": "10.12.34.9", > > > +# "port": "1050" } } } } } > > > +# <- { "return": {} } > > > +# > > > ## > > > -{ 'command': 'migrate-incoming', 'data': {'uri': 'str' } } > > > +{ 'command': 'migrate-incoming', > > > + 'data': {'*uri': 'str', > > > + '*channel': 'MigrateChannel'} } > > Same question of whether we need to future proof now by making this > > > > '*channels': ['MigrateChannel'] > > > > ? > > > > Otherwise we'll have to add this 'channels' field later, and > > end up with 'channel' and 'channels' and 'uri' > > Why do we need 3 fields ? We can directly convert 'channel' into 'channels'. That isnt guaranteed to be permitted Once QEMU is released, the API guarantee applies (unless it was marked 'feature: unsable'. IOW, if we release QEMU with 'channel' parameter and then the 'channels' parameter isn't merged until the next release, we're stuck with both and have to go through the deprecation process for 'channel'. With regards, Daniel
diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c index a9f65ded7a..ae3c5ea5b2 100644 --- a/migration/migration-hmp-cmds.c +++ b/migration/migration-hmp-cmds.c @@ -500,8 +500,14 @@ void hmp_migrate_incoming(Monitor *mon, const QDict *qdict) Error *err = NULL; const char *uri = qdict_get_str(qdict, "uri"); - qmp_migrate_incoming(uri, &err); + MigrateChannel *channel = g_new0(MigrateChannel, 1); + if (!migrate_channel_from_qdict(&channel, qdict, &err)) { + error_setg(&err, "error in retrieving channel from qdict"); + return; + } + qmp_migrate_incoming(uri, channel, &err); + qapi_free_MigrateChannel(channel); hmp_handle_error(mon, err); } diff --git a/migration/migration.c b/migration/migration.c index accbf72a18..e22ce2dd15 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -2314,7 +2314,8 @@ void migrate_del_blocker(Error *reason) migration_blockers = g_slist_remove(migration_blockers, reason); } -void qmp_migrate_incoming(const char *uri, Error **errp) +void qmp_migrate_incoming(const char *uri, MigrateChannel *channel, + Error **errp) { Error *local_err = NULL; static bool once = true; diff --git a/qapi/migration.json b/qapi/migration.json index 79acfcfe4e..3a88912f4d 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -1623,7 +1623,11 @@ # with -incoming defer # # @uri: The Uniform Resource Identifier identifying the source or -# address to listen on +# the address to listen on +# +# @channel: Struct containing migration channel type, along with +# all the details of the destination interface required +# for the address to listen on for migration stream. # # Returns: nothing on success # @@ -1640,14 +1644,28 @@ # # 3. The uri format is the same as for -incoming # +# 4. The 'uri' and 'channel' arguments are mutually exclusive but, atleast +# one of the two arguments should be present. +# # Example: # # -> { "execute": "migrate-incoming", # "arguments": { "uri": "tcp::4446" } } # <- { "return": {} } # +# -> { "execute": "migrate-incoming", +# "arguments": { +# "channel": { "channeltype": "main", +# "addr": { "transport": "socket", +# "socket-type": { "type": "inet", +# "host": "10.12.34.9", +# "port": "1050" } } } } } +# <- { "return": {} } +# ## -{ 'command': 'migrate-incoming', 'data': {'uri': 'str' } } +{ 'command': 'migrate-incoming', + 'data': {'*uri': 'str', + '*channel': 'MigrateChannel'} } ## # @xen-save-devices-state: diff --git a/softmmu/vl.c b/softmmu/vl.c index 9177d95d4e..16b8bdcf9b 100644 --- a/softmmu/vl.c +++ b/softmmu/vl.c @@ -2617,7 +2617,7 @@ void qmp_x_exit_preconfig(Error **errp) if (incoming) { Error *local_err = NULL; if (strcmp(incoming, "defer") != 0) { - qmp_migrate_incoming(incoming, &local_err); + qmp_migrate_incoming(incoming, NULL, &local_err); if (local_err) { error_reportf_err(local_err, "-incoming %s: ", incoming); exit(1);
'migrate-incoming' QAPI design have been modified into well-defined MigrateChannel struct to prevent multiple encoding of uri strings on the destination side.'uri' parameter is kept for backward compatibility. Suggested-by: Daniel P. Berrange <berrange@redhat.com> Suggested-by: Manish Mishra <manish.mishra@nutanix.com> Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com> Signed-off-by: Het Gala <het.gala@nutanix.com> --- migration/migration-hmp-cmds.c | 8 +++++++- migration/migration.c | 3 ++- qapi/migration.json | 22 ++++++++++++++++++++-- softmmu/vl.c | 2 +- 4 files changed, 30 insertions(+), 5 deletions(-)