[Qemu-devel] [PATCH 0/4] Threaded tcp incoming migration.

[Qemu-devel] [PATCH 0/4] Threaded tcp incoming migration.

[Yoshiaki Tamura]

Hi,

This series add threaded tcp incoming migration.  Currently, tcp migration on
incoming side is blocked when outgoing has started on the remote side, and you
can't do anything.  With this series you can get info of incoming migration by
calling "info migrate" like on outgoing side.  Threaded tcp incoming migration
is enable only when --enable-io-thread is set.

This series apply on top of patch from Corentin posted on May 29.

http://www.mail-archive.com/qemu-devel@nongnu.org/msg33830.html

Yoshiaki Tamura (4):
  qemu-thread: add qemu_thread_join().
  migration-tcp: threaded tcp incoming migration.
  arch_init: calculate transferred bytes at ram_load().
  migration: add support to print migration info on incoming side.

 arch_init.c     |    2 +
 migration-tcp.c |   86 ++++++++++++++++++++++++++++++++++++++++++++++---------
 migration.c     |   18 +++++++++--
 migration.h     |    2 +-
 qemu-thread.c   |    9 ++++++
 qemu-thread.h   |    1 +
 6 files changed, 99 insertions(+), 19 deletions(-)

[Qemu-devel] [PATCH 1/4] qemu-thread: add qemu_thread_join().

[Yoshiaki Tamura]

Add missing function to join created thread.

Signed-off-by: Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp>
---
 qemu-thread.c |    9 +++++++++
 qemu-thread.h |    1 +
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/qemu-thread.c b/qemu-thread.c
index afc9933..21953cd 100644
--- a/qemu-thread.c
+++ b/qemu-thread.c
@@ -183,3 +183,12 @@ void qemu_thread_exit(void *retval)
 {
     pthread_exit(retval);
 }
+
+void qemu_thread_join(QemuThread *thread, void **retval)
+{
+    int err;
+
+    err = pthread_join(thread->thread, retval);
+    if (err)
+        error_exit(err, __func__);    
+}
diff --git a/qemu-thread.h b/qemu-thread.h
index 19bb30c..9225b33 100644
--- a/qemu-thread.h
+++ b/qemu-thread.h
@@ -40,5 +40,6 @@ void qemu_thread_signal(QemuThread *thread, int sig);
 void qemu_thread_self(QemuThread *thread);
 int qemu_thread_equal(QemuThread *thread1, QemuThread *thread2);
 void qemu_thread_exit(void *retval);
+void qemu_thread_join(QemuThread *thread, void **retval);
 
 #endif
-- 
1.7.0.31.g1df487

[Qemu-devel] [PATCH 2/4] migration-tcp: threaded tcp incoming migration.

[Yoshiaki Tamura]

Create a thread to handle tcp incoming migration when CONFIG_IOTHREAD
is enabled.  Spawned thread writes it's return status to th_fds[1]
before exit, and main thread joins and reads it.  In
tcp_start_incoming_migration(), allocate FdMigrationState and return
MigrationState to let migration to print incoming migration info.

Signed-off-by: Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp>
---
 migration-tcp.c |   86 ++++++++++++++++++++++++++++++++++++++++++++++---------
 migration.h     |    2 +-
 2 files changed, 73 insertions(+), 15 deletions(-)

diff --git a/migration-tcp.c b/migration-tcp.c
index 95ce722..f20e5fe 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -18,6 +18,7 @@
 #include "sysemu.h"
 #include "buffered_file.h"
 #include "block.h"
+#include "qemu-thread.h"
 
 //#define DEBUG_MIGRATION_TCP
 
@@ -29,6 +30,11 @@
     do { } while (0)
 #endif
 
+#ifdef CONFIG_IOTHREAD
+static QemuThread migration_thread;
+static int th_fds[2];
+#endif
+
 static int socket_errno(FdMigrationState *s)
 {
     return socket_error();
@@ -176,41 +182,93 @@ static void tcp_accept_incoming_migration(void *opaque)
 out_fopen:
     qemu_fclose(f);
 out:
+#ifndef CONFIG_IOTHREAD
     qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL);
+#endif
     close(s);
     close(c);
+#ifdef CONFIG_IOTHREAD
+    write(th_fds[1], &ret, sizeof(ret));
+    qemu_thread_exit(NULL);
+#endif
+}
+
+#ifdef CONFIG_IOTHREAD
+static void tcp_incoming_migration_complete(void *opaque)
+{
+    int ret, state = 0;
+    FdMigrationState *s = opaque;
+
+    qemu_thread_join(&migration_thread, NULL);
+
+    ret = read(th_fds[0], &state, sizeof(state));
+    if (ret == -1) {
+        fprintf(stderr, "failed to read from pipe\n");        
+        goto err;
+    }
+
+    s->state = state < 0 ? MIG_STATE_ERROR : MIG_STATE_COMPLETED;
+
+err:
+    qemu_set_fd_handler2(th_fds[0], NULL, NULL, NULL, NULL);
+    close(th_fds[0]);
+    close(th_fds[1]);
 }
+#endif
 
-int tcp_start_incoming_migration(const char *host_port)
+MigrationState *tcp_start_incoming_migration(const char *host_port)
 {
     struct sockaddr_in addr;
+    FdMigrationState *s;
     int val;
-    int s;
 
     if (parse_host_port(&addr, host_port) < 0) {
         fprintf(stderr, "invalid host/port combination: %s\n", host_port);
-        return -EINVAL;
+        return NULL;
     }
 
-    s = qemu_socket(PF_INET, SOCK_STREAM, 0);
-    if (s == -1)
-        return -socket_error();
+    s = qemu_mallocz(sizeof(*s));
+
+    s->get_error = socket_errno;
+    s->close = tcp_close;
+    s->mig_state.cancel = migrate_fd_cancel;
+    s->mig_state.get_status = migrate_fd_get_status;
+    s->state = MIG_STATE_ACTIVE;
+
+    s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
+    if (s->fd == -1) {
+        qemu_free(s);
+        return NULL;
+    }
 
     val = 1;
-    setsockopt(s, SOL_SOCKET, SO_REUSEADDR, (const char *)&val, sizeof(val));
+    setsockopt(s->fd, SOL_SOCKET, SO_REUSEADDR,
+               (const char *)&val, sizeof(val));
 
-    if (bind(s, (struct sockaddr *)&addr, sizeof(addr)) == -1)
+    if (bind(s->fd, (struct sockaddr *)&addr, sizeof(addr)) == -1)
         goto err;
 
-    if (listen(s, 1) == -1)
+    if (listen(s->fd, 1) == -1)
         goto err;
 
-    qemu_set_fd_handler2(s, NULL, tcp_accept_incoming_migration, NULL,
-                         (void *)(unsigned long)s);
+#ifdef CONFIG_IOTHREAD
+    if (qemu_pipe(th_fds) == -1) {
+        fprintf(stderr, "failed to create pipe\n");
+        goto err;
+    }
 
-    return 0;
+    qemu_thread_create(&migration_thread, (void *)tcp_accept_incoming_migration,
+                       (void *)(unsigned long)s->fd);
+    qemu_set_fd_handler2(th_fds[0], NULL, tcp_incoming_migration_complete, NULL,
+                         (void *)s);
+#else
+    qemu_set_fd_handler2(s->fd, NULL, tcp_accept_incoming_migration, NULL,
+                         (void *)(unsigned long)s->fd);
+#endif
+
+    return &s->mig_state;
 
 err:
-    close(s);
-    return -socket_error();
+    close(s->fd);
+    return NULL;
 }
diff --git a/migration.h b/migration.h
index 385423f..c11e6db 100644
--- a/migration.h
+++ b/migration.h
@@ -76,7 +76,7 @@ MigrationState *exec_start_outgoing_migration(Monitor *mon,
 					      int blk,
 					      int inc);
 
-int tcp_start_incoming_migration(const char *host_port);
+MigrationState *tcp_start_incoming_migration(const char *host_port);
 
 MigrationState *tcp_start_outgoing_migration(Monitor *mon,
                                              const char *host_port,
-- 
1.7.0.31.g1df487

[Qemu-devel] [PATCH 3/4] arch_init: calculate transferred bytes at ram_load().

[Yoshiaki Tamura]

Currently incoming side of migration doesn't know how many bytes are
transferred.  To print "info migrate" on incoming side, we need to
calculate transferred bytes at ram_load().

Signed-off-by: Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp>
---
 arch_init.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index cfc03ea..1b4312d 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -277,6 +277,8 @@ int ram_load(QEMUFile *f, void *opaque, int version_id)
         if (qemu_file_has_error(f)) {
             return -EIO;
         }
+
+        bytes_transferred += TARGET_PAGE_SIZE;
     } while (!(flags & RAM_SAVE_FLAG_EOS));
 
     return 0;
-- 
1.7.0.31.g1df487

[Qemu-devel] [PATCH 4/4] migration: add support to print migration info on incoming side.

[Yoshiaki Tamura]

Set current_migration after calling tcp_star_incoming_migration().  On
incoming side, we don't have to print remaining rams, so introduce
incoming flag to switch messages.

Signed-off-by: Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp>
---
 migration.c |   18 ++++++++++++++----
 1 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/migration.c b/migration.c
index 05f6cc5..32bf9c3 100644
--- a/migration.c
+++ b/migration.c
@@ -36,12 +36,14 @@ static uint32_t max_throttle = (32 << 20);
 
 static MigrationState *current_migration;
 
+static int incoming;
+
 void qemu_start_incoming_migration(const char *uri)
 {
     const char *p;
 
     if (strstart(uri, "tcp:", &p))
-        tcp_start_incoming_migration(p);
+        current_migration = tcp_start_incoming_migration(p);
 #if !defined(WIN32)
     else if (strstart(uri, "exec:", &p))
         exec_start_incoming_migration(p);
@@ -50,8 +52,12 @@ void qemu_start_incoming_migration(const char *uri)
     else if (strstart(uri, "fd:", &p))
         fd_start_incoming_migration(p);
 #endif
-    else
+    else {
         fprintf(stderr, "unknown migration protocol: %s\n", uri);
+        return;
+    }
+
+    incoming = 1;
 }
 
 int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
@@ -162,8 +168,12 @@ static void migrate_print_status(Monitor *mon, const char *name,
 
     monitor_printf(mon, "transferred %s: %" PRIu64 " kbytes\n", name,
                         qdict_get_int(qdict, "transferred") >> 10);
-    monitor_printf(mon, "remaining %s: %" PRIu64 " kbytes\n", name,
-                        qdict_get_int(qdict, "remaining") >> 10);
+    if (incoming) {
+        monitor_printf(mon, "remaining %s: n/a\n", name);
+    } else {
+        monitor_printf(mon, "remaining %s: %" PRIu64 " kbytes\n", name,
+                            qdict_get_int(qdict, "remaining") >> 10);
+    }
     monitor_printf(mon, "total %s: %" PRIu64 " kbytes\n", name,
                         qdict_get_int(qdict, "total") >> 10);
 }
-- 
1.7.0.31.g1df487

[Qemu-devel] Re: [PATCH 0/4] Threaded tcp incoming migration.

[Anthony Liguori]

On 06/01/2010 10:40 AM, Yoshiaki Tamura wrote:
> Hi,
>
> This series add threaded tcp incoming migration.  Currently, tcp migration on
> incoming side is blocked when outgoing has started on the remote side, and you
> can't do anything.  With this series you can get info of incoming migration by
> calling "info migrate" like on outgoing side.  Threaded tcp incoming migration
> is enable only when --enable-io-thread is set.
>    

I'm much less confident that threading is the answer here.  We really 
would just need to have asynchronous incoming migration.

Regards,

Anthony Liguori

> This series apply on top of patch from Corentin posted on May 29.
>
> http://www.mail-archive.com/qemu-devel@nongnu.org/msg33830.html
>
> Yoshiaki Tamura (4):
>    qemu-thread: add qemu_thread_join().
>    migration-tcp: threaded tcp incoming migration.
>    arch_init: calculate transferred bytes at ram_load().
>    migration: add support to print migration info on incoming side.
>
>   arch_init.c     |    2 +
>   migration-tcp.c |   86 ++++++++++++++++++++++++++++++++++++++++++++++---------
>   migration.c     |   18 +++++++++--
>   migration.h     |    2 +-
>   qemu-thread.c   |    9 ++++++
>   qemu-thread.h   |    1 +
>   6 files changed, 99 insertions(+), 19 deletions(-)
>
>    

[Qemu-devel] Re: [PATCH 2/4] migration-tcp: threaded tcp incoming migration.

[Anthony Liguori]

On 06/01/2010 10:40 AM, Yoshiaki Tamura wrote:
> Create a thread to handle tcp incoming migration when CONFIG_IOTHREAD
> is enabled.  Spawned thread writes it's return status to th_fds[1]
> before exit, and main thread joins and reads it.  In
> tcp_start_incoming_migration(), allocate FdMigrationState and return
> MigrationState to let migration to print incoming migration info.
>    

In the absence of any locking, I can't see how this is safe.

Regards,

Anthony Liguori

> Signed-off-by: Yoshiaki Tamura<tamura.yoshiaki@lab.ntt.co.jp>
> ---
>   migration-tcp.c |   86 ++++++++++++++++++++++++++++++++++++++++++++++---------
>   migration.h     |    2 +-
>   2 files changed, 73 insertions(+), 15 deletions(-)
>
> diff --git a/migration-tcp.c b/migration-tcp.c
> index 95ce722..f20e5fe 100644
> --- a/migration-tcp.c
> +++ b/migration-tcp.c
> @@ -18,6 +18,7 @@
>   #include "sysemu.h"
>   #include "buffered_file.h"
>   #include "block.h"
> +#include "qemu-thread.h"
>
>   //#define DEBUG_MIGRATION_TCP
>
> @@ -29,6 +30,11 @@
>       do { } while (0)
>   #endif
>
> +#ifdef CONFIG_IOTHREAD
> +static QemuThread migration_thread;
> +static int th_fds[2];
> +#endif
> +
>   static int socket_errno(FdMigrationState *s)
>   {
>       return socket_error();
> @@ -176,41 +182,93 @@ static void tcp_accept_incoming_migration(void *opaque)
>   out_fopen:
>       qemu_fclose(f);
>   out:
> +#ifndef CONFIG_IOTHREAD
>       qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL);
> +#endif
>       close(s);
>       close(c);
> +#ifdef CONFIG_IOTHREAD
> +    write(th_fds[1],&ret, sizeof(ret));
> +    qemu_thread_exit(NULL);
> +#endif
> +}
> +
> +#ifdef CONFIG_IOTHREAD
> +static void tcp_incoming_migration_complete(void *opaque)
> +{
> +    int ret, state = 0;
> +    FdMigrationState *s = opaque;
> +
> +    qemu_thread_join(&migration_thread, NULL);
> +
> +    ret = read(th_fds[0],&state, sizeof(state));
> +    if (ret == -1) {
> +        fprintf(stderr, "failed to read from pipe\n");
> +        goto err;
> +    }
> +
> +    s->state = state<  0 ? MIG_STATE_ERROR : MIG_STATE_COMPLETED;
> +
> +err:
> +    qemu_set_fd_handler2(th_fds[0], NULL, NULL, NULL, NULL);
> +    close(th_fds[0]);
> +    close(th_fds[1]);
>   }
> +#endif
>
> -int tcp_start_incoming_migration(const char *host_port)
> +MigrationState *tcp_start_incoming_migration(const char *host_port)
>   {
>       struct sockaddr_in addr;
> +    FdMigrationState *s;
>       int val;
> -    int s;
>
>       if (parse_host_port(&addr, host_port)<  0) {
>           fprintf(stderr, "invalid host/port combination: %s\n", host_port);
> -        return -EINVAL;
> +        return NULL;
>       }
>
> -    s = qemu_socket(PF_INET, SOCK_STREAM, 0);
> -    if (s == -1)
> -        return -socket_error();
> +    s = qemu_mallocz(sizeof(*s));
> +
> +    s->get_error = socket_errno;
> +    s->close = tcp_close;
> +    s->mig_state.cancel = migrate_fd_cancel;
> +    s->mig_state.get_status = migrate_fd_get_status;
> +    s->state = MIG_STATE_ACTIVE;
> +
> +    s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
> +    if (s->fd == -1) {
> +        qemu_free(s);
> +        return NULL;
> +    }
>
>       val = 1;
> -    setsockopt(s, SOL_SOCKET, SO_REUSEADDR, (const char *)&val, sizeof(val));
> +    setsockopt(s->fd, SOL_SOCKET, SO_REUSEADDR,
> +               (const char *)&val, sizeof(val));
>
> -    if (bind(s, (struct sockaddr *)&addr, sizeof(addr)) == -1)
> +    if (bind(s->fd, (struct sockaddr *)&addr, sizeof(addr)) == -1)
>           goto err;
>
> -    if (listen(s, 1) == -1)
> +    if (listen(s->fd, 1) == -1)
>           goto err;
>
> -    qemu_set_fd_handler2(s, NULL, tcp_accept_incoming_migration, NULL,
> -                         (void *)(unsigned long)s);
> +#ifdef CONFIG_IOTHREAD
> +    if (qemu_pipe(th_fds) == -1) {
> +        fprintf(stderr, "failed to create pipe\n");
> +        goto err;
> +    }
>
> -    return 0;
> +    qemu_thread_create(&migration_thread, (void *)tcp_accept_incoming_migration,
> +                       (void *)(unsigned long)s->fd);
> +    qemu_set_fd_handler2(th_fds[0], NULL, tcp_incoming_migration_complete, NULL,
> +                         (void *)s);
> +#else
> +    qemu_set_fd_handler2(s->fd, NULL, tcp_accept_incoming_migration, NULL,
> +                         (void *)(unsigned long)s->fd);
> +#endif
> +
> +    return&s->mig_state;
>
>   err:
> -    close(s);
> -    return -socket_error();
> +    close(s->fd);
> +    return NULL;
>   }
> diff --git a/migration.h b/migration.h
> index 385423f..c11e6db 100644
> --- a/migration.h
> +++ b/migration.h
> @@ -76,7 +76,7 @@ MigrationState *exec_start_outgoing_migration(Monitor *mon,
>   					      int blk,
>   					      int inc);
>
> -int tcp_start_incoming_migration(const char *host_port);
> +MigrationState *tcp_start_incoming_migration(const char *host_port);
>
>   MigrationState *tcp_start_outgoing_migration(Monitor *mon,
>                                                const char *host_port,
>    

Re: [Qemu-devel] Re: [PATCH 0/4] Threaded tcp incoming migration.

[Yoshiaki Tamura]

2010/6/2 Anthony Liguori <aliguori@linux.vnet.ibm.com>:
> On 06/01/2010 10:40 AM, Yoshiaki Tamura wrote:
>>
>> Hi,
>>
>> This series add threaded tcp incoming migration.  Currently, tcp migration
>> on
>> incoming side is blocked when outgoing has started on the remote side, and
>> you
>> can't do anything.  With this series you can get info of incoming
>> migration by
>> calling "info migrate" like on outgoing side.  Threaded tcp incoming
>> migration
>> is enable only when --enable-io-thread is set.
>>
>
> I'm much less confident that threading is the answer here.  We really would
> just need to have asynchronous incoming migration.

You mean, go back to select() in main(), and then call incoming
handler each time?
Won't it introduce more latency, resulting less throughput?
Although threading maybe a big hammer for just getting monitor
working, I think using thread for incoming handler may worth if it
doesn't heart performance on receiving data.
The downside is mutual exclusion, of course...

Thanks,

Yoshi

>
> Regards,
>
> Anthony Liguori
>
>> This series apply on top of patch from Corentin posted on May 29.
>>
>> http://www.mail-archive.com/qemu-devel@nongnu.org/msg33830.html
>>
>> Yoshiaki Tamura (4):
>>   qemu-thread: add qemu_thread_join().
>>   migration-tcp: threaded tcp incoming migration.
>>   arch_init: calculate transferred bytes at ram_load().
>>   migration: add support to print migration info on incoming side.
>>
>>  arch_init.c     |    2 +
>>  migration-tcp.c |   86
>> ++++++++++++++++++++++++++++++++++++++++++++++---------
>>  migration.c     |   18 +++++++++--
>>  migration.h     |    2 +-
>>  qemu-thread.c   |    9 ++++++
>>  qemu-thread.h   |    1 +
>>  6 files changed, 99 insertions(+), 19 deletions(-)
>>
>>
>
>
>

Re: [Qemu-devel] Re: [PATCH 0/4] Threaded tcp incoming migration.

[Anthony Liguori]

On 06/01/2010 11:18 AM, Yoshiaki Tamura wrote:
> 2010/6/2 Anthony Liguori<aliguori@linux.vnet.ibm.com>:
>    
>> On 06/01/2010 10:40 AM, Yoshiaki Tamura wrote:
>>      
>>> Hi,
>>>
>>> This series add threaded tcp incoming migration.  Currently, tcp migration
>>> on
>>> incoming side is blocked when outgoing has started on the remote side, and
>>> you
>>> can't do anything.  With this series you can get info of incoming
>>> migration by
>>> calling "info migrate" like on outgoing side.  Threaded tcp incoming
>>> migration
>>> is enable only when --enable-io-thread is set.
>>>
>>>        
>> I'm much less confident that threading is the answer here.  We really would
>> just need to have asynchronous incoming migration.
>>      
> You mean, go back to select() in main(), and then call incoming
> handler each time?
> Won't it introduce more latency, resulting less throughput?
>    

I wouldn't think so.  With threads, you've got to acquire locks and that 
lock acquisition can be a source of significant latency.  By blocking in 
select, you've got a straight dispatch path.

Really, we'd get 99% of the way there just focusing on live loading of 
ram.  There's really no need to make the final stage of migration live.

> Although threading maybe a big hammer for just getting monitor
> working, I think using thread for incoming handler may worth if it
> doesn't heart performance on receiving data.
>    

Unless I'm mistaken, the thread patches you've posted make no attempt at 
addressing the problem of locking.  I believe that properly handling 
locking would result in the patch series increasing in size and 
complexity rather significantly.

I think the simpler approach is to implement a state machine for ram 
loading.

Regards,

Anthony Liguori

> The downside is mutual exclusion, of course...
>
> Thanks,
>
> Yoshi
>
>    
>> Regards,
>>
>> Anthony Liguori
>>
>>      
>>> This series apply on top of patch from Corentin posted on May 29.
>>>
>>> http://www.mail-archive.com/qemu-devel@nongnu.org/msg33830.html
>>>
>>> Yoshiaki Tamura (4):
>>>    qemu-thread: add qemu_thread_join().
>>>    migration-tcp: threaded tcp incoming migration.
>>>    arch_init: calculate transferred bytes at ram_load().
>>>    migration: add support to print migration info on incoming side.
>>>
>>>   arch_init.c     |    2 +
>>>   migration-tcp.c |   86
>>> ++++++++++++++++++++++++++++++++++++++++++++++---------
>>>   migration.c     |   18 +++++++++--
>>>   migration.h     |    2 +-
>>>   qemu-thread.c   |    9 ++++++
>>>   qemu-thread.h   |    1 +
>>>   6 files changed, 99 insertions(+), 19 deletions(-)
>>>
>>>
>>>        
>>
>>
>>      

Re: [Qemu-devel] Re: [PATCH 2/4] migration-tcp: threaded tcp incoming migration.

[Yoshiaki Tamura]

2010/6/2 Anthony Liguori <aliguori@linux.vnet.ibm.com>:
> On 06/01/2010 10:40 AM, Yoshiaki Tamura wrote:
>>
>> Create a thread to handle tcp incoming migration when CONFIG_IOTHREAD
>> is enabled.  Spawned thread writes it's return status to th_fds[1]
>> before exit, and main thread joins and reads it.  In
>> tcp_start_incoming_migration(), allocate FdMigrationState and return
>> MigrationState to let migration to print incoming migration info.
>>
>
> In the absence of any locking, I can't see how this is safe.

Right.  If we use threading here, we need to prevent commands from
monitor that affects incoming thread.

Thanks,

Yoshi

>
> Regards,
>
> Anthony Liguori
>
>> Signed-off-by: Yoshiaki Tamura<tamura.yoshiaki@lab.ntt.co.jp>
>> ---
>>  migration-tcp.c |   86
>> ++++++++++++++++++++++++++++++++++++++++++++++---------
>>  migration.h     |    2 +-
>>  2 files changed, 73 insertions(+), 15 deletions(-)
>>
>> diff --git a/migration-tcp.c b/migration-tcp.c
>> index 95ce722..f20e5fe 100644
>> --- a/migration-tcp.c
>> +++ b/migration-tcp.c
>> @@ -18,6 +18,7 @@
>>  #include "sysemu.h"
>>  #include "buffered_file.h"
>>  #include "block.h"
>> +#include "qemu-thread.h"
>>
>>  //#define DEBUG_MIGRATION_TCP
>>
>> @@ -29,6 +30,11 @@
>>      do { } while (0)
>>  #endif
>>
>> +#ifdef CONFIG_IOTHREAD
>> +static QemuThread migration_thread;
>> +static int th_fds[2];
>> +#endif
>> +
>>  static int socket_errno(FdMigrationState *s)
>>  {
>>      return socket_error();
>> @@ -176,41 +182,93 @@ static void tcp_accept_incoming_migration(void
>> *opaque)
>>  out_fopen:
>>      qemu_fclose(f);
>>  out:
>> +#ifndef CONFIG_IOTHREAD
>>      qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL);
>> +#endif
>>      close(s);
>>      close(c);
>> +#ifdef CONFIG_IOTHREAD
>> +    write(th_fds[1],&ret, sizeof(ret));
>> +    qemu_thread_exit(NULL);
>> +#endif
>> +}
>> +
>> +#ifdef CONFIG_IOTHREAD
>> +static void tcp_incoming_migration_complete(void *opaque)
>> +{
>> +    int ret, state = 0;
>> +    FdMigrationState *s = opaque;
>> +
>> +    qemu_thread_join(&migration_thread, NULL);
>> +
>> +    ret = read(th_fds[0],&state, sizeof(state));
>> +    if (ret == -1) {
>> +        fprintf(stderr, "failed to read from pipe\n");
>> +        goto err;
>> +    }
>> +
>> +    s->state = state<  0 ? MIG_STATE_ERROR : MIG_STATE_COMPLETED;
>> +
>> +err:
>> +    qemu_set_fd_handler2(th_fds[0], NULL, NULL, NULL, NULL);
>> +    close(th_fds[0]);
>> +    close(th_fds[1]);
>>  }
>> +#endif
>>
>> -int tcp_start_incoming_migration(const char *host_port)
>> +MigrationState *tcp_start_incoming_migration(const char *host_port)
>>  {
>>      struct sockaddr_in addr;
>> +    FdMigrationState *s;
>>      int val;
>> -    int s;
>>
>>      if (parse_host_port(&addr, host_port)<  0) {
>>          fprintf(stderr, "invalid host/port combination: %s\n",
>> host_port);
>> -        return -EINVAL;
>> +        return NULL;
>>      }
>>
>> -    s = qemu_socket(PF_INET, SOCK_STREAM, 0);
>> -    if (s == -1)
>> -        return -socket_error();
>> +    s = qemu_mallocz(sizeof(*s));
>> +
>> +    s->get_error = socket_errno;
>> +    s->close = tcp_close;
>> +    s->mig_state.cancel = migrate_fd_cancel;
>> +    s->mig_state.get_status = migrate_fd_get_status;
>> +    s->state = MIG_STATE_ACTIVE;
>> +
>> +    s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
>> +    if (s->fd == -1) {
>> +        qemu_free(s);
>> +        return NULL;
>> +    }
>>
>>      val = 1;
>> -    setsockopt(s, SOL_SOCKET, SO_REUSEADDR, (const char *)&val,
>> sizeof(val));
>> +    setsockopt(s->fd, SOL_SOCKET, SO_REUSEADDR,
>> +               (const char *)&val, sizeof(val));
>>
>> -    if (bind(s, (struct sockaddr *)&addr, sizeof(addr)) == -1)
>> +    if (bind(s->fd, (struct sockaddr *)&addr, sizeof(addr)) == -1)
>>          goto err;
>>
>> -    if (listen(s, 1) == -1)
>> +    if (listen(s->fd, 1) == -1)
>>          goto err;
>>
>> -    qemu_set_fd_handler2(s, NULL, tcp_accept_incoming_migration, NULL,
>> -                         (void *)(unsigned long)s);
>> +#ifdef CONFIG_IOTHREAD
>> +    if (qemu_pipe(th_fds) == -1) {
>> +        fprintf(stderr, "failed to create pipe\n");
>> +        goto err;
>> +    }
>>
>> -    return 0;
>> +    qemu_thread_create(&migration_thread, (void
>> *)tcp_accept_incoming_migration,
>> +                       (void *)(unsigned long)s->fd);
>> +    qemu_set_fd_handler2(th_fds[0], NULL,
>> tcp_incoming_migration_complete, NULL,
>> +                         (void *)s);
>> +#else
>> +    qemu_set_fd_handler2(s->fd, NULL, tcp_accept_incoming_migration,
>> NULL,
>> +                         (void *)(unsigned long)s->fd);
>> +#endif
>> +
>> +    return&s->mig_state;
>>
>>  err:
>> -    close(s);
>> -    return -socket_error();
>> +    close(s->fd);
>> +    return NULL;
>>  }
>> diff --git a/migration.h b/migration.h
>> index 385423f..c11e6db 100644
>> --- a/migration.h
>> +++ b/migration.h
>> @@ -76,7 +76,7 @@ MigrationState *exec_start_outgoing_migration(Monitor
>> *mon,
>>                                              int blk,
>>                                              int inc);
>>
>> -int tcp_start_incoming_migration(const char *host_port);
>> +MigrationState *tcp_start_incoming_migration(const char *host_port);
>>
>>  MigrationState *tcp_start_outgoing_migration(Monitor *mon,
>>                                               const char *host_port,
>>
>
>
>

Re: [Qemu-devel] Re: [PATCH 2/4] migration-tcp: threaded tcp incoming migration.

[Anthony Liguori]

On 06/01/2010 11:23 AM, Yoshiaki Tamura wrote:
> 2010/6/2 Anthony Liguori<aliguori@linux.vnet.ibm.com>:
>    
>> On 06/01/2010 10:40 AM, Yoshiaki Tamura wrote:
>>      
>>> Create a thread to handle tcp incoming migration when CONFIG_IOTHREAD
>>> is enabled.  Spawned thread writes it's return status to th_fds[1]
>>> before exit, and main thread joins and reads it.  In
>>> tcp_start_incoming_migration(), allocate FdMigrationState and return
>>> MigrationState to let migration to print incoming migration info.
>>>
>>>        
>> In the absence of any locking, I can't see how this is safe.
>>      
> Right.  If we use threading here, we need to prevent commands from
> monitor that affects incoming thread.
>    

There's a huge number of calls that can get made during live migration 
that can also be triggered by the monitor.

Even the basic things likes qemu_set_fd_handler2 can cause havoc.

Just preventing certain monitor commands is not sufficient.

Regards,

Anthony Liguori

> Thanks,
>
> Yoshi
>
>    
>> Regards,
>>
>> Anthony Liguori
>>
>>      
>>> Signed-off-by: Yoshiaki Tamura<tamura.yoshiaki@lab.ntt.co.jp>
>>> ---
>>>   migration-tcp.c |   86
>>> ++++++++++++++++++++++++++++++++++++++++++++++---------
>>>   migration.h     |    2 +-
>>>   2 files changed, 73 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/migration-tcp.c b/migration-tcp.c
>>> index 95ce722..f20e5fe 100644
>>> --- a/migration-tcp.c
>>> +++ b/migration-tcp.c
>>> @@ -18,6 +18,7 @@
>>>   #include "sysemu.h"
>>>   #include "buffered_file.h"
>>>   #include "block.h"
>>> +#include "qemu-thread.h"
>>>
>>>   //#define DEBUG_MIGRATION_TCP
>>>
>>> @@ -29,6 +30,11 @@
>>>       do { } while (0)
>>>   #endif
>>>
>>> +#ifdef CONFIG_IOTHREAD
>>> +static QemuThread migration_thread;
>>> +static int th_fds[2];
>>> +#endif
>>> +
>>>   static int socket_errno(FdMigrationState *s)
>>>   {
>>>       return socket_error();
>>> @@ -176,41 +182,93 @@ static void tcp_accept_incoming_migration(void
>>> *opaque)
>>>   out_fopen:
>>>       qemu_fclose(f);
>>>   out:
>>> +#ifndef CONFIG_IOTHREAD
>>>       qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL);
>>> +#endif
>>>       close(s);
>>>       close(c);
>>> +#ifdef CONFIG_IOTHREAD
>>> +    write(th_fds[1],&ret, sizeof(ret));
>>> +    qemu_thread_exit(NULL);
>>> +#endif
>>> +}
>>> +
>>> +#ifdef CONFIG_IOTHREAD
>>> +static void tcp_incoming_migration_complete(void *opaque)
>>> +{
>>> +    int ret, state = 0;
>>> +    FdMigrationState *s = opaque;
>>> +
>>> +    qemu_thread_join(&migration_thread, NULL);
>>> +
>>> +    ret = read(th_fds[0],&state, sizeof(state));
>>> +    if (ret == -1) {
>>> +        fprintf(stderr, "failed to read from pipe\n");
>>> +        goto err;
>>> +    }
>>> +
>>> +    s->state = state<    0 ? MIG_STATE_ERROR : MIG_STATE_COMPLETED;
>>> +
>>> +err:
>>> +    qemu_set_fd_handler2(th_fds[0], NULL, NULL, NULL, NULL);
>>> +    close(th_fds[0]);
>>> +    close(th_fds[1]);
>>>   }
>>> +#endif
>>>
>>> -int tcp_start_incoming_migration(const char *host_port)
>>> +MigrationState *tcp_start_incoming_migration(const char *host_port)
>>>   {
>>>       struct sockaddr_in addr;
>>> +    FdMigrationState *s;
>>>       int val;
>>> -    int s;
>>>
>>>       if (parse_host_port(&addr, host_port)<    0) {
>>>           fprintf(stderr, "invalid host/port combination: %s\n",
>>> host_port);
>>> -        return -EINVAL;
>>> +        return NULL;
>>>       }
>>>
>>> -    s = qemu_socket(PF_INET, SOCK_STREAM, 0);
>>> -    if (s == -1)
>>> -        return -socket_error();
>>> +    s = qemu_mallocz(sizeof(*s));
>>> +
>>> +    s->get_error = socket_errno;
>>> +    s->close = tcp_close;
>>> +    s->mig_state.cancel = migrate_fd_cancel;
>>> +    s->mig_state.get_status = migrate_fd_get_status;
>>> +    s->state = MIG_STATE_ACTIVE;
>>> +
>>> +    s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
>>> +    if (s->fd == -1) {
>>> +        qemu_free(s);
>>> +        return NULL;
>>> +    }
>>>
>>>       val = 1;
>>> -    setsockopt(s, SOL_SOCKET, SO_REUSEADDR, (const char *)&val,
>>> sizeof(val));
>>> +    setsockopt(s->fd, SOL_SOCKET, SO_REUSEADDR,
>>> +               (const char *)&val, sizeof(val));
>>>
>>> -    if (bind(s, (struct sockaddr *)&addr, sizeof(addr)) == -1)
>>> +    if (bind(s->fd, (struct sockaddr *)&addr, sizeof(addr)) == -1)
>>>           goto err;
>>>
>>> -    if (listen(s, 1) == -1)
>>> +    if (listen(s->fd, 1) == -1)
>>>           goto err;
>>>
>>> -    qemu_set_fd_handler2(s, NULL, tcp_accept_incoming_migration, NULL,
>>> -                         (void *)(unsigned long)s);
>>> +#ifdef CONFIG_IOTHREAD
>>> +    if (qemu_pipe(th_fds) == -1) {
>>> +        fprintf(stderr, "failed to create pipe\n");
>>> +        goto err;
>>> +    }
>>>
>>> -    return 0;
>>> +    qemu_thread_create(&migration_thread, (void
>>> *)tcp_accept_incoming_migration,
>>> +                       (void *)(unsigned long)s->fd);
>>> +    qemu_set_fd_handler2(th_fds[0], NULL,
>>> tcp_incoming_migration_complete, NULL,
>>> +                         (void *)s);
>>> +#else
>>> +    qemu_set_fd_handler2(s->fd, NULL, tcp_accept_incoming_migration,
>>> NULL,
>>> +                         (void *)(unsigned long)s->fd);
>>> +#endif
>>> +
>>> +    return&s->mig_state;
>>>
>>>   err:
>>> -    close(s);
>>> -    return -socket_error();
>>> +    close(s->fd);
>>> +    return NULL;
>>>   }
>>> diff --git a/migration.h b/migration.h
>>> index 385423f..c11e6db 100644
>>> --- a/migration.h
>>> +++ b/migration.h
>>> @@ -76,7 +76,7 @@ MigrationState *exec_start_outgoing_migration(Monitor
>>> *mon,
>>>                                               int blk,
>>>                                               int inc);
>>>
>>> -int tcp_start_incoming_migration(const char *host_port);
>>> +MigrationState *tcp_start_incoming_migration(const char *host_port);
>>>
>>>   MigrationState *tcp_start_outgoing_migration(Monitor *mon,
>>>                                                const char *host_port,
>>>
>>>        
>>
>>
>>      

Re: [Qemu-devel] Re: [PATCH 0/4] Threaded tcp incoming migration.

[Yoshiaki Tamura]

2010/6/2 Anthony Liguori <aliguori@linux.vnet.ibm.com>:
> On 06/01/2010 11:18 AM, Yoshiaki Tamura wrote:
>>
>> 2010/6/2 Anthony Liguori<aliguori@linux.vnet.ibm.com>:
>>
>>>
>>> On 06/01/2010 10:40 AM, Yoshiaki Tamura wrote:
>>>
>>>>
>>>> Hi,
>>>>
>>>> This series add threaded tcp incoming migration.  Currently, tcp
>>>> migration
>>>> on
>>>> incoming side is blocked when outgoing has started on the remote side,
>>>> and
>>>> you
>>>> can't do anything.  With this series you can get info of incoming
>>>> migration by
>>>> calling "info migrate" like on outgoing side.  Threaded tcp incoming
>>>> migration
>>>> is enable only when --enable-io-thread is set.
>>>>
>>>>
>>>
>>> I'm much less confident that threading is the answer here.  We really
>>> would
>>> just need to have asynchronous incoming migration.
>>>
>>
>> You mean, go back to select() in main(), and then call incoming
>> handler each time?
>> Won't it introduce more latency, resulting less throughput?
>>
>
> I wouldn't think so.  With threads, you've got to acquire locks and that
> lock acquisition can be a source of significant latency.  By blocking in
> select, you've got a straight dispatch path.
>
> Really, we'd get 99% of the way there just focusing on live loading of ram.
>  There's really no need to make the final stage of migration live.
>
>> Although threading maybe a big hammer for just getting monitor
>> working, I think using thread for incoming handler may worth if it
>> doesn't heart performance on receiving data.
>>
>
> Unless I'm mistaken, the thread patches you've posted make no attempt at
> addressing the problem of locking.  I believe that properly handling locking
> would result in the patch series increasing in size and complexity rather
> significantly.
>
> I think the simpler approach is to implement a state machine for ram
> loading.

OK.  Making ram loading to be selectable seems to be a good start point.

Thanks,

Yoshi

>
> Regards,
>
> Anthony Liguori
>
>> The downside is mutual exclusion, of course...
>>
>> Thanks,
>>
>> Yoshi
>>
>>
>>>
>>> Regards,
>>>
>>> Anthony Liguori
>>>
>>>
>>>>
>>>> This series apply on top of patch from Corentin posted on May 29.
>>>>
>>>> http://www.mail-archive.com/qemu-devel@nongnu.org/msg33830.html
>>>>
>>>> Yoshiaki Tamura (4):
>>>>   qemu-thread: add qemu_thread_join().
>>>>   migration-tcp: threaded tcp incoming migration.
>>>>   arch_init: calculate transferred bytes at ram_load().
>>>>   migration: add support to print migration info on incoming side.
>>>>
>>>>  arch_init.c     |    2 +
>>>>  migration-tcp.c |   86
>>>> ++++++++++++++++++++++++++++++++++++++++++++++---------
>>>>  migration.c     |   18 +++++++++--
>>>>  migration.h     |    2 +-
>>>>  qemu-thread.c   |    9 ++++++
>>>>  qemu-thread.h   |    1 +
>>>>  6 files changed, 99 insertions(+), 19 deletions(-)
>>>>
>>>>
>>>>
>>>
>>>
>>>
>
>
>

Re: [Qemu-devel] Re: [PATCH 2/4] migration-tcp: threaded tcp incoming migration.

[Yoshiaki Tamura]

2010/6/2 Anthony Liguori <aliguori@linux.vnet.ibm.com>:
> On 06/01/2010 11:23 AM, Yoshiaki Tamura wrote:
>>
>> 2010/6/2 Anthony Liguori<aliguori@linux.vnet.ibm.com>:
>>
>>>
>>> On 06/01/2010 10:40 AM, Yoshiaki Tamura wrote:
>>>
>>>>
>>>> Create a thread to handle tcp incoming migration when CONFIG_IOTHREAD
>>>> is enabled.  Spawned thread writes it's return status to th_fds[1]
>>>> before exit, and main thread joins and reads it.  In
>>>> tcp_start_incoming_migration(), allocate FdMigrationState and return
>>>> MigrationState to let migration to print incoming migration info.
>>>>
>>>>
>>>
>>> In the absence of any locking, I can't see how this is safe.
>>>
>>
>> Right.  If we use threading here, we need to prevent commands from
>> monitor that affects incoming thread.
>>
>
> There's a huge number of calls that can get made during live migration that
> can also be triggered by the monitor.
>
> Even the basic things likes qemu_set_fd_handler2 can cause havoc.
>
> Just preventing certain monitor commands is not sufficient.

Ah, that was my concern.  Sorry for not understanding well.
Thanks for your advice.

Yoshi

>
> Regards,
>
> Anthony Liguori
>
>> Thanks,
>>
>> Yoshi
>>
>>
>>>
>>> Regards,
>>>
>>> Anthony Liguori
>>>
>>>
>>>>
>>>> Signed-off-by: Yoshiaki Tamura<tamura.yoshiaki@lab.ntt.co.jp>
>>>> ---
>>>>  migration-tcp.c |   86
>>>> ++++++++++++++++++++++++++++++++++++++++++++++---------
>>>>  migration.h     |    2 +-
>>>>  2 files changed, 73 insertions(+), 15 deletions(-)
>>>>
>>>> diff --git a/migration-tcp.c b/migration-tcp.c
>>>> index 95ce722..f20e5fe 100644
>>>> --- a/migration-tcp.c
>>>> +++ b/migration-tcp.c
>>>> @@ -18,6 +18,7 @@
>>>>  #include "sysemu.h"
>>>>  #include "buffered_file.h"
>>>>  #include "block.h"
>>>> +#include "qemu-thread.h"
>>>>
>>>>  //#define DEBUG_MIGRATION_TCP
>>>>
>>>> @@ -29,6 +30,11 @@
>>>>      do { } while (0)
>>>>  #endif
>>>>
>>>> +#ifdef CONFIG_IOTHREAD
>>>> +static QemuThread migration_thread;
>>>> +static int th_fds[2];
>>>> +#endif
>>>> +
>>>>  static int socket_errno(FdMigrationState *s)
>>>>  {
>>>>      return socket_error();
>>>> @@ -176,41 +182,93 @@ static void tcp_accept_incoming_migration(void
>>>> *opaque)
>>>>  out_fopen:
>>>>      qemu_fclose(f);
>>>>  out:
>>>> +#ifndef CONFIG_IOTHREAD
>>>>      qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL);
>>>> +#endif
>>>>      close(s);
>>>>      close(c);
>>>> +#ifdef CONFIG_IOTHREAD
>>>> +    write(th_fds[1],&ret, sizeof(ret));
>>>> +    qemu_thread_exit(NULL);
>>>> +#endif
>>>> +}
>>>> +
>>>> +#ifdef CONFIG_IOTHREAD
>>>> +static void tcp_incoming_migration_complete(void *opaque)
>>>> +{
>>>> +    int ret, state = 0;
>>>> +    FdMigrationState *s = opaque;
>>>> +
>>>> +    qemu_thread_join(&migration_thread, NULL);
>>>> +
>>>> +    ret = read(th_fds[0],&state, sizeof(state));
>>>> +    if (ret == -1) {
>>>> +        fprintf(stderr, "failed to read from pipe\n");
>>>> +        goto err;
>>>> +    }
>>>> +
>>>> +    s->state = state<    0 ? MIG_STATE_ERROR : MIG_STATE_COMPLETED;
>>>> +
>>>> +err:
>>>> +    qemu_set_fd_handler2(th_fds[0], NULL, NULL, NULL, NULL);
>>>> +    close(th_fds[0]);
>>>> +    close(th_fds[1]);
>>>>  }
>>>> +#endif
>>>>
>>>> -int tcp_start_incoming_migration(const char *host_port)
>>>> +MigrationState *tcp_start_incoming_migration(const char *host_port)
>>>>  {
>>>>      struct sockaddr_in addr;
>>>> +    FdMigrationState *s;
>>>>      int val;
>>>> -    int s;
>>>>
>>>>      if (parse_host_port(&addr, host_port)<    0) {
>>>>          fprintf(stderr, "invalid host/port combination: %s\n",
>>>> host_port);
>>>> -        return -EINVAL;
>>>> +        return NULL;
>>>>      }
>>>>
>>>> -    s = qemu_socket(PF_INET, SOCK_STREAM, 0);
>>>> -    if (s == -1)
>>>> -        return -socket_error();
>>>> +    s = qemu_mallocz(sizeof(*s));
>>>> +
>>>> +    s->get_error = socket_errno;
>>>> +    s->close = tcp_close;
>>>> +    s->mig_state.cancel = migrate_fd_cancel;
>>>> +    s->mig_state.get_status = migrate_fd_get_status;
>>>> +    s->state = MIG_STATE_ACTIVE;
>>>> +
>>>> +    s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
>>>> +    if (s->fd == -1) {
>>>> +        qemu_free(s);
>>>> +        return NULL;
>>>> +    }
>>>>
>>>>      val = 1;
>>>> -    setsockopt(s, SOL_SOCKET, SO_REUSEADDR, (const char *)&val,
>>>> sizeof(val));
>>>> +    setsockopt(s->fd, SOL_SOCKET, SO_REUSEADDR,
>>>> +               (const char *)&val, sizeof(val));
>>>>
>>>> -    if (bind(s, (struct sockaddr *)&addr, sizeof(addr)) == -1)
>>>> +    if (bind(s->fd, (struct sockaddr *)&addr, sizeof(addr)) == -1)
>>>>          goto err;
>>>>
>>>> -    if (listen(s, 1) == -1)
>>>> +    if (listen(s->fd, 1) == -1)
>>>>          goto err;
>>>>
>>>> -    qemu_set_fd_handler2(s, NULL, tcp_accept_incoming_migration, NULL,
>>>> -                         (void *)(unsigned long)s);
>>>> +#ifdef CONFIG_IOTHREAD
>>>> +    if (qemu_pipe(th_fds) == -1) {
>>>> +        fprintf(stderr, "failed to create pipe\n");
>>>> +        goto err;
>>>> +    }
>>>>
>>>> -    return 0;
>>>> +    qemu_thread_create(&migration_thread, (void
>>>> *)tcp_accept_incoming_migration,
>>>> +                       (void *)(unsigned long)s->fd);
>>>> +    qemu_set_fd_handler2(th_fds[0], NULL,
>>>> tcp_incoming_migration_complete, NULL,
>>>> +                         (void *)s);
>>>> +#else
>>>> +    qemu_set_fd_handler2(s->fd, NULL, tcp_accept_incoming_migration,
>>>> NULL,
>>>> +                         (void *)(unsigned long)s->fd);
>>>> +#endif
>>>> +
>>>> +    return&s->mig_state;
>>>>
>>>>  err:
>>>> -    close(s);
>>>> -    return -socket_error();
>>>> +    close(s->fd);
>>>> +    return NULL;
>>>>  }
>>>> diff --git a/migration.h b/migration.h
>>>> index 385423f..c11e6db 100644
>>>> --- a/migration.h
>>>> +++ b/migration.h
>>>> @@ -76,7 +76,7 @@ MigrationState *exec_start_outgoing_migration(Monitor
>>>> *mon,
>>>>                                              int blk,
>>>>                                              int inc);
>>>>
>>>> -int tcp_start_incoming_migration(const char *host_port);
>>>> +MigrationState *tcp_start_incoming_migration(const char *host_port);
>>>>
>>>>  MigrationState *tcp_start_outgoing_migration(Monitor *mon,
>>>>                                               const char *host_port,
>>>>
>>>>
>>>
>>>
>>>
>
>
>