Re: [Qemu-devel] [RFC 19/29] migration: let dst listen on port always

[Daniel P. Berrange]

On Fri, Jul 28, 2017 at 04:06:28PM +0800, Peter Xu wrote:
> Signed-off-by: Peter Xu <address@hidden>
> ---
>  migration/exec.c   | 2 +-
>  migration/fd.c     | 2 +-
>  migration/socket.c | 4 ++--
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/migration/exec.c b/migration/exec.c
> index 08b599e..b4412db 100644
> --- a/migration/exec.c
> +++ b/migration/exec.c
> @@ -49,7 +49,7 @@ static gboolean exec_accept_incoming_migration(QIOChannel 
> *ioc,
>  {
>      migration_channel_process_incoming(ioc);
>      object_unref(OBJECT(ioc));
> -    return FALSE; /* unregister */
> +    return TRUE; /* keep it registered */
>  }
>  
>  void exec_start_incoming_migration(const char *command, Error **errp)
> diff --git a/migration/fd.c b/migration/fd.c
> index 30f5258..865277a 100644
> --- a/migration/fd.c
> +++ b/migration/fd.c
> @@ -49,7 +49,7 @@ static gboolean fd_accept_incoming_migration(QIOChannel 
> *ioc,
>  {
>      migration_channel_process_incoming(ioc);
>      object_unref(OBJECT(ioc));
> -    return FALSE; /* unregister */
> +    return TRUE; /* keep it registered */
>  }
>  
>  void fd_start_incoming_migration(const char *infd, Error **errp)
> diff --git a/migration/socket.c b/migration/socket.c
> index 757d382..f2c2d01 100644
> --- a/migration/socket.c
> +++ b/migration/socket.c
> @@ -153,8 +153,8 @@ static gboolean 
> socket_accept_incoming_migration(QIOChannel *ioc,
>  
>  out:
>      /* Close listening socket as its no longer needed */
> -    qio_channel_close(ioc, NULL);
> -    return FALSE; /* unregister */
> +    // qio_channel_close(ioc, NULL);
> +    return TRUE; /* keep it registered */
>  }


This is not a very desirable approach IMHO.

There are two separate things at play - first we have the listener socket,
and second we have the I/O watch that monitors for incoming clients.

The current code here closes the listener, and returns FALSE to unregister
the event loop watch.

You're reversing both of these so that we keep the listener open and we
keep monitoring for incoming clients. Ignoring migration resume for a
minute, this means that the destination QEMU will now accept arbitrarily
many incoming clients and keep trying to start a new incoming migration.

The behaviour we need is diferent. We *want* to unregister the event
loop watch once we've accepted a client. We should only keep the socket
listener in existance, but *not* accept any more clients. Only once we
have hit a problem and want to accept a new client to do migration
recovery, should we be re-adding the event loop watch.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|