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

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

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

On Tue, Feb 04, 2025 at 01:12:09AM +0100, Stefano Brivio wrote:
> 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".

No, I haven't begun actually writing it.

> > > > [...]
> > > >
> > > >  /**
> > > > - * 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.

Ok.

-- 
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 --]