Re: [PATCH v3 14/20] migrate: Don't handle the migration channel through epoll

[Stefano Brivio <sbrivio@redhat.com>]

On Mon, 3 Feb 2025 12:50:20 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Fri, Jan 31, 2025 at 08:39:47PM +0100, Stefano Brivio wrote:
> > From: David Gibson <david@gibson.dropbear.id.au>
> > 
> > Currently, once a migration device state fd is assigned, we wait for
> > EPOLLIN or EPOLLOUT events on it to actually perform the migration.  Change
> > it so that once a migration is requested it we complete it synchronously
> > at the end of the current epoll cycle.  This has several advantages:
> > 
> >   1. It makes it clear that everything about the migration must be dealt
> >      with at once, not split between multiple epoll events on the channel
> >   2. It ensures the migration always takes place between epoll cycles,
> >      rather than, for example, between handling TCP events and their
> >      deferred handling in post_handler().
> >   3. It reduces code setting up the epoll watch on the fd.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  epoll_type.h |  2 --
> >  migrate.c    | 44 +++++++++++---------------------------------
> >  migrate.h    |  2 +-
> >  passt.c      |  6 ++----
> >  passt.h      |  2 ++
> >  vu_common.c  | 27 +++++++++++++++++++++++++++
> >  6 files changed, 43 insertions(+), 40 deletions(-)
> > 
> > diff --git a/epoll_type.h b/epoll_type.h
> > index b981d30..7f2a121 100644
> > --- a/epoll_type.h
> > +++ b/epoll_type.h
> > @@ -40,8 +40,6 @@ enum epoll_type {
> >  	EPOLL_TYPE_VHOST_CMD,
> >  	/* vhost-user kick event socket */
> >  	EPOLL_TYPE_VHOST_KICK,
> > -	/* migration device state channel */
> > -	EPOLL_TYPE_DEVICE_STATE,
> >  	/* TCP_REPAIR helper listening socket */
> >  	EPOLL_TYPE_REPAIR_LISTEN,
> >  	/* TCP_REPAIR helper socket */
> > diff --git a/migrate.c b/migrate.c
> > index fc6a043..faa7841 100644
> > --- a/migrate.c
> > +++ b/migrate.c
> > @@ -50,7 +50,6 @@ static union migrate_header header = {
> >  
> >  /* Data sections for version 1 */
> >  static struct iovec sections_v1[] = {
> > -	{ &header,	sizeof(header) },  
> 
> This hunk seems to have migrated in during rebase somehow, it wasn't
> in my original patch, and it breaks compile.

Oh, oops, yes. I did one last rebase round before posting, and I was
convinced I was just rewording messages, so I didn't even try to
rebuild, but obviously I moved some code here and there as well.

Do you prefer I post a v4 fixing this up and addressing your other
comments (and some of my open points), or should I rather wait, if
you're working on patches based on v3? It makes no difference to me.

-- 
Stefano