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

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3058 bytes --]

On Mon, Feb 03, 2025 at 06:38:56AM +0100, Stefano Brivio wrote:
> 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.

Wait, I have some patches in the queue.

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]