Re: [PATCH blktests v3 10/15] nvme/{006,008,010,012,014,019,023}: support NVMET_BLKDEV_TYPES

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Apr 30, 2024 / 15:00, Sagi Grimberg wrote:
> 
> 
> On 29/04/2024 2:58, Shinichiro Kawasaki wrote:
> > On Apr 28, 2024 / 16:12, Sagi Grimberg wrote:
> > > 
> > > On 28/04/2024 13:32, Shinichiro Kawasaki wrote:
> > > > On Apr 28, 2024 / 11:58, Sagi Grimberg wrote:
> > > > > On 24/04/2024 10:59, Shin'ichiro Kawasaki wrote:
> > > > > > Enable repeated test runs for the listed test cases for
> > > > > > NVMET_BLKDEV_TYPES. Modify the set_conditions() hooks to call
> > > > > > _set_nvme_trtype_and_nvmet_blkdev_type() instead of _set_nvmet_trtype()
> > > > > > so that the test cases are repeated for listed conditions in
> > > > > > NVMET_BLKDEV_TYPES and NVMET_TRTYPES.
> > > > > > 
> > > > > > The default values of NVMET_BLKDEV_TYPES is (device file). With this
> > > > > > default set up, each of the listed test cases are run twice. The second
> > > > > > runs of the test cases for 'file' blkdev type do exact same test as
> > > > > > other test cases nvme/007, 009, 011, 013, 015, 020 and 024.
> > > > > > 
> > > > > > Reviewed-by: Daniel Wagner <dwagner@xxxxxxx>
> > > > > > Reviewed-by: Chaitanya Kulkarni <kch@xxxxxxxxxx>
> > > > > > Acked-by: Nitesh Shetty <nj.shetty@xxxxxxxxxxx>
> > > > > > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@xxxxxxx>
> > > > > > ---
> > > > > >     tests/nvme/006 | 2 +-
> > > > > >     tests/nvme/008 | 2 +-
> > > > > >     tests/nvme/010 | 2 +-
> > > > > >     tests/nvme/012 | 2 +-
> > > > > >     tests/nvme/014 | 2 +-
> > > > > >     tests/nvme/019 | 2 +-
> > > > > >     tests/nvme/023 | 2 +-
> > > > > >     7 files changed, 7 insertions(+), 7 deletions(-)
> > > > > > 
> > > > > > diff --git a/tests/nvme/006 b/tests/nvme/006
> > > > > > index ff0a9eb..c543b40 100755
> > > > > > --- a/tests/nvme/006
> > > > > > +++ b/tests/nvme/006
> > > > > > @@ -16,7 +16,7 @@ requires() {
> > > > > >     }
> > > > > >     set_conditions() {
> > > > > > -	_set_nvme_trtype "$@"
> > > > > > +	_set_nvme_trtype_and_nvmet_blkdev_type "$@"
> > > > > Why not calling separate functions? having func do_a_and_b interface is not
> > > > > great.
> > > > In this case, we want to repeat the test cases to cover combination of two
> > > > conditions: M trtypes and N blkdev_types. The test case should be repeated to
> > > > cover all of M x N matrix elements, then the hook set_conditions() should
> > > > iterate the elements. I can not think of the way to handle this iteration with
> > > > separated two functions.
> > > What happens when you add another condition to iterate against, you
> > > introduce set_a_and_b_and_c interface?
> > That is my current intent.
> 
> I don't think its very maintainable.
> 
> > 
> > Another question is how it is likely to have more conditions to add on.
> 
> I expect that people will want to add more flavors moving forward. For
> example
> ADDRESS_FAMILIES="ipv4 ipv6" RDMA_TRANSPORT="siw rxe" and possibly other
> features that can grow in the future.
> 
> 
> >   I guess
> > such many, multiplied conditions will result in combination explosion and long
> > test runtime, so I'm not sure how much it will be useful.
> 
> I think that running multiple flavors of a test suite is a capability that
> is bound to be
> reused as more test flavors emerge. But that may be just my opinion.
> 
> > 
> > Do we have potential candidates of the third or fourth conditions?
> 
> Yes, see above.

Okay, the candidates look useful in the future. Fortunately, I came up with an
idea to make up the condition matrix from multiple set_conditions() hooks. Will
try to implement it and respin the series.




[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux