On Tue, Aug 01, 2017 at 11:56:10AM +0100, Daniel P. Berrange wrote: > On Fri, Jul 28, 2017 at 04:06:28PM +0800, Peter Xu wrote: > > Signed-off-by: Peter Xu <pet...@redhat.com> > > --- > > 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.
I replied with another approach in the other thread: how about we re-enable the listen port duing "postcopy-pause" state, and disable the listen port when get out of that migration state? Here "listen port" I mean both the IO watch and the socket object. Now what I can think of: we keep these objects always there, meanwhile we introduce a new bit for migration, say, "accept_incoming", to decide whether we will really accept one connection. Then we drop the new connection if that bit is not set. (Or a new QIOChannelFeature to temporarily refuse incoming connection? E.g., QIO_CHANNEL_FEATURE_LISTEN_REFUSE?) Thanks, -- Peter Xu