diff mbox series

[v4,7/8] migration: modified 'migrate-incoming' QAPI to accept 'channels' argument for migration.

Message ID 20230512143240.192504-8-het.gala@nutanix.com (mailing list archive)
State New, archived
Headers show
Series migration: Modified 'migrate' and 'migrate-incoming' QAPI commands for migration | expand

Commit Message

Het Gala May 12, 2023, 2:32 p.m. UTC
MigrateChannelList ideally allows to have multiple listener interfaces up for
connection.

Added MigrateChannelList struct as argument to 'migrate-incoming' qapi. Introduced
MigrateChannelList in hmp_migrate_incoming() and qmp_migrate_incoming() functions.

Future patchset series plans to include multiple MigrateChannels to have multiple
listening interfaces up. That is the reason, 'MigrateChannelList' is the preferred
choice of argument over 'MigrateChannel' and making 'migrate-incoming' QAPI future
proof.

For current patch, have just limit size of MigrateChannelList to 1 element as
a runtime check.

Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
Signed-off-by: Het Gala <het.gala@nutanix.com>
---
 migration/migration-hmp-cmds.c | 14 +++++++++++++-
 migration/migration.c          | 21 ++++++++++++++++----
 qapi/migration.json            | 35 +++++++++++++++++++++++++++++++++-
 softmmu/vl.c                   |  2 +-
 4 files changed, 65 insertions(+), 7 deletions(-)

Comments

Juan Quintela May 15, 2023, 10:01 a.m. UTC | #1
Het Gala <het.gala@nutanix.com> wrote:
> MigrateChannelList ideally allows to have multiple listener interfaces up for
> connection.
>
> Added MigrateChannelList struct as argument to 'migrate-incoming' qapi. Introduced
> MigrateChannelList in hmp_migrate_incoming() and qmp_migrate_incoming() functions.
>
> Future patchset series plans to include multiple MigrateChannels to have multiple
> listening interfaces up. That is the reason, 'MigrateChannelList' is the preferred
> choice of argument over 'MigrateChannel' and making 'migrate-incoming' QAPI future
> proof.
>
> For current patch, have just limit size of MigrateChannelList to 1 element as
> a runtime check.
>
> Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
> Signed-off-by: Het Gala <het.gala@nutanix.com>
> ---
>  migration/migration-hmp-cmds.c | 14 +++++++++++++-
>  migration/migration.c          | 21 ++++++++++++++++----
>  qapi/migration.json            | 35 +++++++++++++++++++++++++++++++++-
>  softmmu/vl.c                   |  2 +-
>  4 files changed, 65 insertions(+), 7 deletions(-)
>
> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> index f098d04542..8c11a8c83b 100644
> --- a/migration/migration-hmp-cmds.c
> +++ b/migration/migration-hmp-cmds.c
> @@ -518,10 +518,22 @@ void hmp_migrate_incoming(Monitor *mon, const QDict *qdict)
>  {
>      Error *err = NULL;
>      const char *uri = qdict_get_str(qdict, "uri");
> +    MigrateChannel *channel = g_new0(MigrateChannel, 1);
> +    MigrateChannelList *caps = NULL;
> +
> +    if (!migrate_channel_from_qdict(&channel, qdict, &err)) {
> +        error_setg(&err, "error in retrieving channel from qdict");
> +        goto end;
> +    }
>  
> -    qmp_migrate_incoming(uri, &err);
> +    QAPI_LIST_PREPEND(caps, channel);
> +    qmp_migrate_incoming(uri, !!caps, caps, &err);
> +    qapi_free_MigrateChannelList(caps);
>  
> +end:
> +    qapi_free_MigrateChannel(channel);
>      hmp_handle_error(mon, err);
> +    return;

Useless return?


The rest seeams ok.
Daniel P. Berrangé May 15, 2023, 10:38 a.m. UTC | #2
On Fri, May 12, 2023 at 02:32:39PM +0000, Het Gala wrote:
> MigrateChannelList ideally allows to have multiple listener interfaces up for
> connection.
> 
> Added MigrateChannelList struct as argument to 'migrate-incoming' qapi. Introduced
> MigrateChannelList in hmp_migrate_incoming() and qmp_migrate_incoming() functions.
> 
> Future patchset series plans to include multiple MigrateChannels to have multiple
> listening interfaces up. That is the reason, 'MigrateChannelList' is the preferred
> choice of argument over 'MigrateChannel' and making 'migrate-incoming' QAPI future
> proof.
> 
> For current patch, have just limit size of MigrateChannelList to 1 element as
> a runtime check.
> 
> Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
> Signed-off-by: Het Gala <het.gala@nutanix.com>
> ---
>  migration/migration-hmp-cmds.c | 14 +++++++++++++-
>  migration/migration.c          | 21 ++++++++++++++++----
>  qapi/migration.json            | 35 +++++++++++++++++++++++++++++++++-
>  softmmu/vl.c                   |  2 +-
>  4 files changed, 65 insertions(+), 7 deletions(-)
> 
> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> index f098d04542..8c11a8c83b 100644
> --- a/migration/migration-hmp-cmds.c
> +++ b/migration/migration-hmp-cmds.c
> @@ -518,10 +518,22 @@ void hmp_migrate_incoming(Monitor *mon, const QDict *qdict)
>  {
>      Error *err = NULL;
>      const char *uri = qdict_get_str(qdict, "uri");
> +    MigrateChannel *channel = g_new0(MigrateChannel, 1);
> +    MigrateChannelList *caps = NULL;
> +
> +    if (!migrate_channel_from_qdict(&channel, qdict, &err)) {
> +        error_setg(&err, "error in retrieving channel from qdict");
> +        goto end;
> +    }

Again adding a bunch more parameters to HMP but nothing is documented.

>  
> -    qmp_migrate_incoming(uri, &err);
> +    QAPI_LIST_PREPEND(caps, channel);
> +    qmp_migrate_incoming(uri, !!caps, caps, &err);
> +    qapi_free_MigrateChannelList(caps);
>  
> +end:
> +    qapi_free_MigrateChannel(channel);
>      hmp_handle_error(mon, err);
> +    return;
>  }
>  
>  void hmp_migrate_recover(Monitor *mon, const QDict *qdict)
> diff --git a/migration/migration.c b/migration/migration.c
> index 78f16e5041..de058643a6 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -452,12 +452,24 @@ static bool migrate_uri_parse(const char *uri,
>      return true;
>  }
>  
> -static void qemu_start_incoming_migration(const char *uri, Error **errp)
> +static void qemu_start_incoming_migration(const char *uri, bool has_channels,
> +                                          MigrateChannelList *channels,
> +                                          Error **errp)
>  {
>      Error *local_err = NULL;
>      MigrateAddress *channel = g_new0(MigrateAddress, 1);
>      SocketAddress *saddr;
>  
> +    /*
> +     * Having preliminary checks for uri and channel
> +     */
> +    if (uri && has_channels) {
> +        error_setg(errp, "'uri' and 'channels' arguments are mutually "
> +                   "exclusive; exactly one of the two should be present in "
> +                   "'migrate-incoming' qmp command ");
> +        return;
> +    }
> +
>      /* URI is not suitable for migration? */
>      if (!migration_channels_and_uri_compatible(uri, errp)) {
>          goto out;
> @@ -1507,7 +1519,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, bool has_channels,
> +                          MigrateChannelList *channels, Error **errp)
>  {
>      Error *local_err = NULL;
>      static bool once = true;
> @@ -1525,7 +1538,7 @@ void qmp_migrate_incoming(const char *uri, Error **errp)
>          return;
>      }
>  
> -    qemu_start_incoming_migration(uri, &local_err);
> +    qemu_start_incoming_migration(uri, has_channels, channels, &local_err);
>  
>      if (local_err) {
>          yank_unregister_instance(MIGRATION_YANK_INSTANCE);
> @@ -1561,7 +1574,7 @@ void qmp_migrate_recover(const char *uri, Error **errp)
>       * only re-setup the migration stream and poke existing migration
>       * to continue using that newly established channel.
>       */
> -    qemu_start_incoming_migration(uri, errp);
> +    qemu_start_incoming_migration(uri, false, NULL, errp);
>  }
>  
>  void qmp_migrate_pause(Error **errp)
> diff --git a/qapi/migration.json b/qapi/migration.json
> index a71af87ffe..9faecdd048 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -1578,6 +1578,10 @@
>  # @uri: The Uniform Resource Identifier identifying the source or
>  #       address to listen on
>  #
> +# @channels: Struct containing list of migration channel types, with all
> +#            the information regarding destination interfaces required for
> +#            initiating a migration stream.
> +#
>  # Returns: nothing on success
>  #
>  # Since: 2.3
> @@ -1593,14 +1597,43 @@
>  #
>  # 3. The uri format is the same as for -incoming
>  #
> +# 4. The 'uri' and 'channel' arguments are mutually exclusive; exactly one
> +#    of the two should be present.
> +#
>  # Example:
>  #
>  # -> { "execute": "migrate-incoming",
>  #      "arguments": { "uri": "tcp::4446" } }
>  # <- { "return": {} }
>  #
> +# -> { "execute": "migrate",
> +#      "arguments": {
> +#          "channels": [ { "channeltype": "main",
> +#                          "addr": { "transport": "socket", "type": "inet",
> +#                                    "host": "10.12.34.9",
> +#                                    "port": "1050" } } ] } }
> +# <- { "return": {} }
> +#
> +# -> { "execute": "migrate",
> +#      "arguments": {
> +#          "channels": [ { "channeltype": "main",
> +#                          "addr": { "transport": "exec",
> +#                                    "args": [ "/bin/nc", "-p", "6000",
> +#                                              "/some/sock" ] } } ] } }
> +# <- { "return": {} }
> +#
> +# -> { "execute": "migrate",
> +#      "arguments": {
> +#          "channels": [ { "channeltype": "main",
> +#                          "addr": { "transport": "rdma",
> +#                                    "host": "10.12.34.9",
> +#                                    "port": "1050" } } ] } }
> +# <- { "return": {} }
> +#
>  ##
> -{ 'command': 'migrate-incoming', 'data': {'uri': 'str' } }
> +{ 'command': 'migrate-incoming',
> +             'data': {'*uri': 'str',
> +                      '*channels': [ 'MigrateChannel' ] } }
>  
>  ##
>  # @xen-save-devices-state:
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 6c2427262b..ade411eb4f 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -2633,7 +2633,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, false, NULL, &local_err);
>              if (local_err) {
>                  error_reportf_err(local_err, "-incoming %s: ", incoming);
>                  exit(1);
> -- 
> 2.22.3
> 

With regards,
Daniel
diff mbox series

Patch

diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index f098d04542..8c11a8c83b 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -518,10 +518,22 @@  void hmp_migrate_incoming(Monitor *mon, const QDict *qdict)
 {
     Error *err = NULL;
     const char *uri = qdict_get_str(qdict, "uri");
+    MigrateChannel *channel = g_new0(MigrateChannel, 1);
+    MigrateChannelList *caps = NULL;
+
+    if (!migrate_channel_from_qdict(&channel, qdict, &err)) {
+        error_setg(&err, "error in retrieving channel from qdict");
+        goto end;
+    }
 
-    qmp_migrate_incoming(uri, &err);
+    QAPI_LIST_PREPEND(caps, channel);
+    qmp_migrate_incoming(uri, !!caps, caps, &err);
+    qapi_free_MigrateChannelList(caps);
 
+end:
+    qapi_free_MigrateChannel(channel);
     hmp_handle_error(mon, err);
+    return;
 }
 
 void hmp_migrate_recover(Monitor *mon, const QDict *qdict)
diff --git a/migration/migration.c b/migration/migration.c
index 78f16e5041..de058643a6 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -452,12 +452,24 @@  static bool migrate_uri_parse(const char *uri,
     return true;
 }
 
-static void qemu_start_incoming_migration(const char *uri, Error **errp)
+static void qemu_start_incoming_migration(const char *uri, bool has_channels,
+                                          MigrateChannelList *channels,
+                                          Error **errp)
 {
     Error *local_err = NULL;
     MigrateAddress *channel = g_new0(MigrateAddress, 1);
     SocketAddress *saddr;
 
+    /*
+     * Having preliminary checks for uri and channel
+     */
+    if (uri && has_channels) {
+        error_setg(errp, "'uri' and 'channels' arguments are mutually "
+                   "exclusive; exactly one of the two should be present in "
+                   "'migrate-incoming' qmp command ");
+        return;
+    }
+
     /* URI is not suitable for migration? */
     if (!migration_channels_and_uri_compatible(uri, errp)) {
         goto out;
@@ -1507,7 +1519,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, bool has_channels,
+                          MigrateChannelList *channels, Error **errp)
 {
     Error *local_err = NULL;
     static bool once = true;
@@ -1525,7 +1538,7 @@  void qmp_migrate_incoming(const char *uri, Error **errp)
         return;
     }
 
-    qemu_start_incoming_migration(uri, &local_err);
+    qemu_start_incoming_migration(uri, has_channels, channels, &local_err);
 
     if (local_err) {
         yank_unregister_instance(MIGRATION_YANK_INSTANCE);
@@ -1561,7 +1574,7 @@  void qmp_migrate_recover(const char *uri, Error **errp)
      * only re-setup the migration stream and poke existing migration
      * to continue using that newly established channel.
      */
-    qemu_start_incoming_migration(uri, errp);
+    qemu_start_incoming_migration(uri, false, NULL, errp);
 }
 
 void qmp_migrate_pause(Error **errp)
diff --git a/qapi/migration.json b/qapi/migration.json
index a71af87ffe..9faecdd048 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1578,6 +1578,10 @@ 
 # @uri: The Uniform Resource Identifier identifying the source or
 #       address to listen on
 #
+# @channels: Struct containing list of migration channel types, with all
+#            the information regarding destination interfaces required for
+#            initiating a migration stream.
+#
 # Returns: nothing on success
 #
 # Since: 2.3
@@ -1593,14 +1597,43 @@ 
 #
 # 3. The uri format is the same as for -incoming
 #
+# 4. The 'uri' and 'channel' arguments are mutually exclusive; exactly one
+#    of the two should be present.
+#
 # Example:
 #
 # -> { "execute": "migrate-incoming",
 #      "arguments": { "uri": "tcp::4446" } }
 # <- { "return": {} }
 #
+# -> { "execute": "migrate",
+#      "arguments": {
+#          "channels": [ { "channeltype": "main",
+#                          "addr": { "transport": "socket", "type": "inet",
+#                                    "host": "10.12.34.9",
+#                                    "port": "1050" } } ] } }
+# <- { "return": {} }
+#
+# -> { "execute": "migrate",
+#      "arguments": {
+#          "channels": [ { "channeltype": "main",
+#                          "addr": { "transport": "exec",
+#                                    "args": [ "/bin/nc", "-p", "6000",
+#                                              "/some/sock" ] } } ] } }
+# <- { "return": {} }
+#
+# -> { "execute": "migrate",
+#      "arguments": {
+#          "channels": [ { "channeltype": "main",
+#                          "addr": { "transport": "rdma",
+#                                    "host": "10.12.34.9",
+#                                    "port": "1050" } } ] } }
+# <- { "return": {} }
+#
 ##
-{ 'command': 'migrate-incoming', 'data': {'uri': 'str' } }
+{ 'command': 'migrate-incoming',
+             'data': {'*uri': 'str',
+                      '*channels': [ 'MigrateChannel' ] } }
 
 ##
 # @xen-save-devices-state:
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 6c2427262b..ade411eb4f 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2633,7 +2633,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, false, NULL, &local_err);
             if (local_err) {
                 error_reportf_err(local_err, "-incoming %s: ", incoming);
                 exit(1);