diff mbox series

[v10,2/5] migration: Create socket-address parameter

Message ID 20181128113337.7238-3-quintela@redhat.com
State New
Headers show
Series Add make check tests for Migration | expand

Commit Message

Juan Quintela Nov. 28, 2018, 11:33 a.m. UTC 
It will be used to store the uri parameters. We want this only for
tcp, so we don't set it for other uris.  We need it to know what port
is migration running.

Signed-off-by: Juan Quintela <quintela@redhat.com>

--

This used to be uri parameter, but it has so many troubles to
reproduce that it don't just make sense.

This used to be a port parameter.  I was asked to move to
SocketAddress, done.
I also merged the setting of the migration tcp port in this one
because now I need to free the address, and this makes it easier.
This used to be x-socket-address with a single direction, now it is a
list of addresses.
Move SocketAddress_to_str here.  I used to try to generalize the one
in chardev/char-socket.c, but it is not worth it.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hmp.c                 | 31 +++++++++++++++++++++++++++++++
 migration/migration.c | 24 ++++++++++++++++++++++++
 migration/migration.h |  4 ++++
 migration/socket.c    | 11 +++++++++++
 qapi/migration.json   |  6 +++++-
 qapi/sockets.json     | 13 +++++++++++++
 6 files changed, 88 insertions(+), 1 deletion(-)

Comments

Eric Blake Nov. 28, 2018, 2:27 p.m. UTC | #1 
On 11/28/18 5:33 AM, Juan Quintela wrote:
> It will be used to store the uri parameters. We want this only for
> tcp, so we don't set it for other uris.  We need it to know what port
> is migration running.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> 
> --
> 

That's not the typical --- separator that git-am would recognize.

> This used to be uri parameter, but it has so many troubles to
> reproduce that it don't just make sense.
> 
> This used to be a port parameter.  I was asked to move to
> SocketAddress, done.
> I also merged the setting of the migration tcp port in this one
> because now I need to free the address, and this makes it easier.
> This used to be x-socket-address with a single direction, now it is a
> list of addresses.
> Move SocketAddress_to_str here.  I used to try to generalize the one
> in chardev/char-socket.c, but it is not worth it.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>

the double S-o-b is odd, especially if the second one was intended to be 
stripped.


> +++ b/qapi/migration.json
> @@ -6,6 +6,7 @@
>   ##
>   
>   { 'include': 'common.json' }
> +{ 'include': 'sockets.json' }
>   
>   ##
>   # @MigrationStats:
> @@ -196,6 +197,8 @@
>   # @compression: migration compression statistics, only returned if compression
>   #           feature is on and status is 'active' or 'completed' (Since 3.1)
>   #
> +# @socket-address: Only used for tcp, to know what the real port is (Since 3.1)
> +#

s/3.1/4.0/ now...

> +++ b/qapi/sockets.json
> @@ -152,3 +152,16 @@
>               'unix': 'UnixSocketAddress',
>               'vsock': 'VsockSocketAddress',
>               'fd': 'String' } }
> +
> +##
> +# @DummyStruct:
> +#
> +# Both block-core and migration needs SocketAddressList
> +# I am open to comments about how to share it
> +#
> +# @dummy-list: A dummy list
> +#
> +# Since: 3.1
> +##
> +{ 'struct': 'DummyStruct',
> +  'data': { 'dummy-list': ['SocketAddress'] } }

We've had a couple of threads about this in the last couple of months; 
maybe it's time for a qapi generator patch where we add an optional 
'gen-list':true marker to any struct that needs to have a list generated 
for C code even when no QMP command uses the list form.
Markus Armbruster Nov. 29, 2018, 9:30 a.m. UTC | #2 
Eric Blake <eblake@redhat.com> writes:

[...]
>> +++ b/qapi/sockets.json
>> @@ -152,3 +152,16 @@
>>               'unix': 'UnixSocketAddress',
>>               'vsock': 'VsockSocketAddress',
>>               'fd': 'String' } }
>> +
>> +##
>> +# @DummyStruct:
>> +#
>> +# Both block-core and migration needs SocketAddressList
>> +# I am open to comments about how to share it
>> +#
>> +# @dummy-list: A dummy list
>> +#
>> +# Since: 3.1
>> +##
>> +{ 'struct': 'DummyStruct',
>> +  'data': { 'dummy-list': ['SocketAddress'] } }
>
> We've had a couple of threads about this in the last couple of months;
> maybe it's time for a qapi generator patch where we add an optional
> 'gen-list':true marker to any struct that needs to have a list
> generated for C code even when no QMP command uses the list form.

Maybe.

As long as we need just a few such lists, stuffing them all into
DummyForceArrays should do fine.  There's no need for a fresh dummy type
for each list.

Beyond a certain point, the DummyForceArrays hack may become bothersome
enough to justify development and maintenance of a proper QAPI feature.

You mentioned "a couple of threads".  Have they resulted in fresh dummy
types?  Or, to ask the same question differently, for which types do we
force generation of list types?
diff mbox series

Patch

diff --git a/hmp.c b/hmp.c
index 7828f93a39..2fd12a4a81 100644
--- a/hmp.c
+++ b/hmp.c
@@ -166,6 +166,26 @@  void hmp_info_mice(Monitor *mon, const QDict *qdict)
     qapi_free_MouseInfoList(mice_list);
 }
 
+static char *SocketAddress_to_str(SocketAddress *addr)
+{
+    switch (addr->type) {
+    case SOCKET_ADDRESS_TYPE_INET:
+        return g_strdup_printf("tcp:%s:%s",
+                               addr->u.inet.host,
+                               addr->u.inet.port);
+        break;
+    case SOCKET_ADDRESS_TYPE_UNIX:
+        return g_strdup_printf("unix:%s",
+                               addr->u.q_unix.path);
+        break;
+    case SOCKET_ADDRESS_TYPE_FD:
+        return g_strdup_printf("fd:%s", addr->u.fd.str);
+        break;
+    default:
+        abort();
+    }
+}
+
 void hmp_info_migrate(Monitor *mon, const QDict *qdict)
 {
     MigrationInfo *info;
@@ -304,6 +324,17 @@  void hmp_info_migrate(Monitor *mon, const QDict *qdict)
         g_free(str);
         visit_free(v);
     }
+    if (info->has_socket_address) {
+        SocketAddressList *addr;
+
+        monitor_printf(mon, "socket address: [\n");
+
+        for (addr = info->socket_address; addr; addr = addr->next) {
+            char *s = SocketAddress_to_str(addr->value);
+            monitor_printf(mon, "\t%s\n", s);
+        }
+        monitor_printf(mon, "]\n");
+    }
     qapi_free_MigrationInfo(info);
     qapi_free_MigrationCapabilityStatusList(caps);
 }
diff --git a/migration/migration.c b/migration/migration.c
index 49ffb9997a..22954dcabc 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -31,6 +31,8 @@ 
 #include "migration/vmstate.h"
 #include "block/block.h"
 #include "qapi/error.h"
+#include "qapi/clone-visitor.h"
+#include "qapi/qapi-visit-sockets.h"
 #include "qapi/qapi-commands-migration.h"
 #include "qapi/qapi-events-migration.h"
 #include "qapi/qmp/qerror.h"
@@ -197,6 +199,11 @@  void migration_incoming_state_destroy(void)
     }
 
     qemu_event_reset(&mis->main_thread_load_event);
+
+    if (mis->socket_address) {
+        qapi_free_SocketAddressList(mis->socket_address);
+        mis->socket_address = NULL;
+    }
 }
 
 static void migrate_generate_event(int new_state)
@@ -312,6 +319,17 @@  void migration_incoming_enable_colo(void)
     migration_colo_enabled = true;
 }
 
+void migrate_add_address(SocketAddress *address)
+{
+    MigrationIncomingState *mis = migration_incoming_get_current();
+    SocketAddressList *addrs;
+
+    addrs = g_new0(SocketAddressList, 1);
+    addrs->next = mis->socket_address;
+    mis->socket_address = addrs;
+    addrs->value = QAPI_CLONE(SocketAddress, address);
+}
+
 void qemu_start_incoming_migration(const char *uri, Error **errp)
 {
     const char *p;
@@ -960,6 +978,12 @@  static void fill_destination_migration_info(MigrationInfo *info)
 {
     MigrationIncomingState *mis = migration_incoming_get_current();
 
+    if (mis->socket_address) {
+        info->has_socket_address = true;
+        info->socket_address =
+            QAPI_CLONE(SocketAddressList, mis->socket_address);
+    }
+
     switch (mis->state) {
     case MIGRATION_STATUS_NONE:
         return;
diff --git a/migration/migration.h b/migration/migration.h
index e413d4d8b6..324970433c 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -80,6 +80,9 @@  struct MigrationIncomingState {
     bool postcopy_recover_triggered;
     QemuSemaphore postcopy_pause_sem_dst;
     QemuSemaphore postcopy_pause_sem_fault;
+
+    /* List of listening socket addresses  */
+    SocketAddressList *socket_address;
 };
 
 MigrationIncomingState *migration_incoming_get_current(void);
@@ -292,6 +295,7 @@  void migrate_send_rp_resume_ack(MigrationIncomingState *mis, uint32_t value);
 
 void dirty_bitmap_mig_before_vm_start(void);
 void init_dirty_bitmap_incoming_migration(void);
+void migrate_add_address(SocketAddress *address);
 
 #define qemu_ram_foreach_block \
   #warning "Use qemu_ram_foreach_block_migratable in migration code"
diff --git a/migration/socket.c b/migration/socket.c
index f4c8174400..239527fb1f 100644
--- a/migration/socket.c
+++ b/migration/socket.c
@@ -15,6 +15,7 @@ 
  */
 
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
 
 #include "qemu-common.h"
 #include "qemu/error-report.h"
@@ -177,6 +178,7 @@  static void socket_start_incoming_migration(SocketAddress *saddr,
                                             Error **errp)
 {
     QIONetListener *listener = qio_net_listener_new();
+    size_t i;
 
     qio_net_listener_set_name(listener, "migration-socket-listener");
 
@@ -189,6 +191,15 @@  static void socket_start_incoming_migration(SocketAddress *saddr,
                                           socket_accept_incoming_migration,
                                           NULL, NULL,
                                           g_main_context_get_thread_default());
+
+    for (i = 0; i < listener->nsioc; i++)  {
+        SocketAddress *address =
+            qio_channel_socket_get_local_address(listener->sioc[i], errp);
+        if (!address) {
+            return;
+        }
+        migrate_add_address(address);
+    }
 }
 
 void tcp_start_incoming_migration(const char *host_port, Error **errp)
diff --git a/qapi/migration.json b/qapi/migration.json
index 38d4c41d88..631949796d 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -6,6 +6,7 @@ 
 ##
 
 { 'include': 'common.json' }
+{ 'include': 'sockets.json' }
 
 ##
 # @MigrationStats:
@@ -196,6 +197,8 @@ 
 # @compression: migration compression statistics, only returned if compression
 #           feature is on and status is 'active' or 'completed' (Since 3.1)
 #
+# @socket-address: Only used for tcp, to know what the real port is (Since 3.1)
+#
 # Since: 0.14.0
 ##
 { 'struct': 'MigrationInfo',
@@ -210,7 +213,8 @@ 
            '*error-desc': 'str',
            '*postcopy-blocktime' : 'uint32',
            '*postcopy-vcpu-blocktime': ['uint32'],
-           '*compression': 'CompressionStats'} }
+           '*compression': 'CompressionStats',
+           '*socket-address': ['SocketAddress'] } }
 
 ##
 # @query-migrate:
diff --git a/qapi/sockets.json b/qapi/sockets.json
index fc81d8d5e8..d7f77984af 100644
--- a/qapi/sockets.json
+++ b/qapi/sockets.json
@@ -152,3 +152,16 @@ 
             'unix': 'UnixSocketAddress',
             'vsock': 'VsockSocketAddress',
             'fd': 'String' } }
+
+##
+# @DummyStruct:
+#
+# Both block-core and migration needs SocketAddressList
+# I am open to comments about how to share it
+#
+# @dummy-list: A dummy list
+#
+# Since: 3.1
+##
+{ 'struct': 'DummyStruct',
+  'data': { 'dummy-list': ['SocketAddress'] } }