Re: [kvm-unit-tests PATCH v1 1/3] s390x: add library for skey-related functions

[Claudio Imbrenda <imbrenda@linux.ibm.com>]

On Fri, 2 Dec 2022 13:48:17 +0100
Thomas Huth <thuth@redhat.com> wrote:

> On 02/12/2022 12.56, Janosch Frank wrote:
> > On 12/2/22 12:32, Thomas Huth wrote:  
> >> On 02/12/2022 11.44, Nico Boehr wrote:  
> >>> Quoting Thomas Huth (2022-12-02 10:09:03)  
> >>>> On 02/12/2022 10.03, Janosch Frank wrote:  
> >>>>> On 12/1/22 09:46, Nico Boehr wrote:  
> >>>>>> Upcoming changes will add a test which is very similar to the existing
> >>>>>> skey migration test. To reduce code duplication, move the common
> >>>>>> functions to a library which can be re-used by both tests.
> >>>>>>  
> >>>>>
> >>>>> NACK
> >>>>>
> >>>>> We're not putting test specific code into the library.  
> >>>>
> >>>> Do we need a new file (in the third patch) for the new test at all, or 
> >>>> could
> >>>> the new test simply be added to s390x/migration-skey.c instead?  
> >>>
> >>> Mh, not quite. One test wants to change storage keys *before* migrating, 
> >>> the other *while* migrating. Since we can only migrate once, it is not 
> >>> obvious to me how we could do that in one run.
> >>>
> >>> Speaking of one run, what we could do is add a command line argument 
> >>> which decides which test to run and then call the same test with 
> >>> different arguments in unittests.cfg.  
> >>
> >> Yes, that's what I had in mind - use a command line argument to select the
> >> test ... should be OK as long as both variants are listed in unittests.cfg,
> >> shouldn't it?
> >>
> >>    Thomas  
> > 
> > @Thomas @Claudio:
> > I see two possible solutions if we want a "testlib" at some point (which for 
> > the record I don't have anything against):
> > 
> > Putting the files into lib/s390x/testlib/* which will then be part of our 
> > normal lib.
> > That's a minimal effort solution. It still puts those files into lib/* but 
> > they are at least contained in a directory.
> > 
> > Putting the files into s390x/testlib/* and creating a proper new lib.
> > Which means we'd need a few more lines of makefile changes.  
> 
> Though this is an excellent topic for a Friday afternoon bikeshedding ... I 
> don't mind much either way. I maybe just got a small preference to not touch 
> the main lib/ folder here. I guess you could even call it 
> s390x/migration-skey-common.c and leave the lib logic out of the game ... 
> but I don't really mind. Up to you to decide ;-)
> 

I really like the idea of having only one test and use a commandline
parameter to decide which variant to run

this way no need to put things in external files

>   Thomas
>