Re: [PATCH 6/6] migrate: Make migration handlers simpler and more flexible

[Stefano Brivio <sbrivio@redhat.com>]

On Tue, 4 Feb 2025 10:55:28 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Mon, Feb 03, 2025 at 11:20:03AM +0100, Stefano Brivio wrote:
> > On Mon,  3 Feb 2025 20:26:15 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> >   
> > > +/* Stages for version 1 */
> > > +static const struct migrate_stage stages_v1[] = {
> > > +	{
> > > +		.name = "flow pre",
> > > +		.source = flow_migrate_source_pre,
> > > +		.target = NULL,
> > > +	},
> > > +	DATA_STAGE(flow_first_free),
> > > +	DATA_STAGE(flowtab),
> > > +	DATA_STAGE(flow_hashtab),
> > > +	DATA_STAGE(g_hash_secret),  
> > 
> > ...so this one for g_hash_secret (which is the abomination I wanted
> > to drop) would eventually turn into a function?  
> 
> Yes.
> 
> > It looks neat, I'm just not sure if we'll really have "data stages"
> > after I change the approach to only transfer the data we need as you
> > suggested.  
> 
> I agree, that may well be the case, but we can just drop the macro and
> helepr functions then.
> 
> > Is that part of your pending queue by the way, or should I go ahead with
> > it?  
> 
> Is which part of my pending queue?  g_hash_secret as a function?  No,
> I've thought of it, but I haven't written it yet.

No, I meant: "transfer the data we need".

> > > [...]
> > >
> > >  /**
> > > - * struct migrate_data - Data sections for given source version
> > > - * @v:			Source version this applies to, host order
> > > - * @sections:		Array of data sections, NULL-terminated
> > > + * migrate_cb_t - Callback function to implement one migration stage
> > >   */
> > > -struct migrate_data {
> > > -	uint32_t v;
> > > -	struct iovec *sections;
> > > -};
> > > +typedef int (*migrate_cb_t)(struct ctx *c, struct migrate_meta *m,
> > > +			    const struct migrate_stage *stage, int fd);  
> > 
> > typedef is really annoying, we already have a flow_sidx which kind of
> > made sense, but this has really no advantage over something like:  
> 
> Eh, I guess.  I find the extra . or -> a little annoying, but we can
> do this if you'd prefer.

Well, I find not finding things because of the typedef quite annoying.

Actually, we don't even need a struct if you prefer (I actually do, but
I know not everybody finds the syntax for function pointers convenient).

I'm re-posting this squashed onto previous patches but it would be nice
if you could re-post a version of the whole thing without the typedef.

-- 
Stefano