From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36933) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dcUyw-0003LS-4k for qemu-devel@nongnu.org; Tue, 01 Aug 2017 07:05:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dcUyr-00049R-8J for qemu-devel@nongnu.org; Tue, 01 Aug 2017 07:04:58 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35478) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dcUyq-00048j-WB for qemu-devel@nongnu.org; Tue, 01 Aug 2017 07:04:53 -0400 Date: Tue, 1 Aug 2017 11:56:10 +0100 From: "Daniel P. Berrange" Message-ID: <20170801105610.GG12670@redhat.com> Reply-To: "Daniel P. Berrange" References: <1501229198-30588-1-git-send-email-peterx@redhat.com> <1501229198-30588-20-git-send-email-peterx@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1501229198-30588-20-git-send-email-peterx@redhat.com> Subject: Re: [Qemu-devel] [RFC 19/29] migration: let dst listen on port always List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Xu Cc: qemu-devel@nongnu.org, Laurent Vivier , Andrea Arcangeli , Juan Quintela , Alexey Perevalov , "Dr . David Alan Gilbert" On Fri, Jul 28, 2017 at 04:06:28PM +0800, Peter Xu wrote: > Signed-off-by: Peter Xu > --- > 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 :|