Re: Support for runtime parameters in injection points, for AIO tests

Started by Michael Paquier11 months ago5 messageshackers
Jump to latest
#1Michael Paquier
michael@paquier.xyz

On Thu, May 08, 2025 at 06:16:46PM +0000, Greg Burd wrote:

IMO seems like a good idea, happy to help further "cleanup the AIO
code" with some direction as to what needs to be done.

Thanks for posting a rebase. Yes, I would like to clean up that on
HEAD, but it also seems like Andres would prefer let this matter be
handled once v18 is forked on its own branch, so I am not sure how to
proceed here. Avoiding a diff for this new code between v18 and HEAD
would be nicer, IMO, and we're still in early stability period. And
that would be less conflicts when maintaining these tests across
branches.

Have others an opinion to offer here?
--
Michael

#2Andres Freund
andres@anarazel.de
In reply to: Michael Paquier (#1)

Hi,

On 2025-05-09 08:19:39 +0900, Michael Paquier wrote:

On Thu, May 08, 2025 at 06:16:46PM +0000, Greg Burd wrote:

IMO seems like a good idea, happy to help further "cleanup the AIO
code" with some direction as to what needs to be done.

Thanks for posting a rebase. Yes, I would like to clean up that on
HEAD, but it also seems like Andres would prefer let this matter be
handled once v18 is forked on its own branch, so I am not sure how to
proceed here.

I'm ok with it happening now or only in master after branching off. Either
seems justifiable to me. I really don't have much of an opinion about it - it
seems you do, so we should probably go with that.

Greetings,

Andres

#3Michael Paquier
michael@paquier.xyz
In reply to: Andres Freund (#2)

On Thu, May 08, 2025 at 09:18:15PM -0400, Andres Freund wrote:

I'm ok with it happening now or only in master after branching off. Either
seems justifiable to me. I really don't have much of an opinion about it - it
seems you do, so we should probably go with that.

Thanks for the input. Will proceed if there are no objections, then.
--
Michael

#4Michael Paquier
michael@paquier.xyz
In reply to: Michael Paquier (#3)

On Fri, May 09, 2025 at 10:48:45AM +0900, Michael Paquier wrote:

Thanks for the input. Will proceed if there are no objections, then.

By the way, Greg, v2-0003 in the patch set you have posted was
incorrect; it missed most of the wanted changes to get rid of the
workarounds in the AIO code. Compared to the original version, there
were just two conflicts with the injection point names, nothing huge.

Patch v2 seems to be hold on moderation perhaps? It has not been
published to the lists.

I will not be able to monitor the buildfarm today but I have the
attached v3 staged for commit, for later when I'll be able to do so.
--
Michael

Attachments:

v3-0001-Add-support-for-runtime-parameters-in-injection-p.patchtext/x-diff; charset=us-asciiDownload+52-48
v3-0002-injection_points-Add-support-for-runtime-argument.patchtext/x-diff; charset=us-asciiDownload+102-17
v3-0003-aio-Use-runtime-arguments-with-injections-points.patchtext/x-diff; charset=us-asciiDownload+13-85
#5Michael Paquier
michael@paquier.xyz
In reply to: Michael Paquier (#1)

On Fri, May 09, 2025 at 03:59:48PM +0000, Greg Burd wrote:

Apologies for that, somehow the wrong version of that file was
attached. I'll be more careful next time.

No problem. This was mostly the same as the original. There was a
fuzz in 0001, actually, fixed by 0003 with the definitions of the
callbacks being incorrect.

It would be nice to plant a bit more InjectionPointCallback around
these callback declarations, actually. I'll look into that later.

I have now applied all that and closed the open item, keeping an eye
on the buildfarm in case.
--
Michael