Re: [PATCH blktests 05/11] nvme/rc: introduce NVMET_TR_TYPES

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

 



On Apr 16, 2024 / 18:43, Chaitanya Kulkarni wrote:
> On 4/16/2024 9:23 AM, Daniel Wagner wrote:
> > On Tue, Apr 16, 2024 at 10:28:49AM +0000, Shinichiro Kawasaki wrote:
> >>>    # nvme_trtypes=rdma ./check nvme/006     ... works
> >>>    # NVMET_TRTYPES=(rdma) ./check nvme/006  ... does not work
> >>>
> >>> I will modify the descriptions above in the v2 series to note that both
> >>> nvme_trtype and NVMET_TRTYPES are supported and usable.
> >>
> >> I rethought this. Now I think it is bad that NVMET_TRTYPES can not be specified
> >> in command lines. To avoid this drawback, I think it's the better to change
> >> NVME_TRTYPES from an array to a variable with multiple items separated with
> >> spaces. For example, three types can be specified to NVMET_TRTYPES like this:
> >>
> >>     NVMET_TRTYPES="loop tcp rdma"
> >>
> >> NVMET_BLKDEV_TYPES has the same restriction then I will change it also from an
> >> array to a variable in same manner. I will send out v2 soon with this change.
> >>
> >> Daniel,
> >>
> >> I assume this change is fine for your use case. If it is not the case, please
> >> let me know.
> > 
> > Yes, it's nice that all the configure variables are of the same type.
> > 
> > On this topic, I am a bit confused about the naming scheme. We have the
> > lower case ones, e.g. 'nvme_trtypes' and now the upper case ones
> > NVMET_TRYPES. I assume you prefer the upper case to mark them they are
> > injected from the environment and the lower case ones are globals
> > variables in the framework. Should we retire the lower case ones and
> > replace them with upper case ones?

Yes, "mark them they are injected from the environment" was the one reason to
have the parameters in upper cases. The other reason was the consistency across
the all parameters described in Documentation/running-tests.md.

> 
> can we please keep the small letter similar to nvme_trtype ?

I'm fine to have small letter, lower cases for the new parameter, but I would
like to clarify the reason to have lower cases. Do you mean to indicate that
"the parameters are test group local" using the lower cases?




[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