[PATCH blktests 2/2] nvme/{041,042,043,044,045}: require kernel config NVME_HOST_AUTH
Hannes Reinecke
hare at suse.de
Wed Nov 15 00:20:28 PST 2023
On 11/15/23 08:46, Shinichiro Kawasaki wrote:
> On Nov 15, 2023 / 06:59, Hannes Reinecke wrote:
> [...]
>>> diff --git a/tests/nvme/045 b/tests/nvme/045
>>> index 1eb1032..126060c 100755
>>> --- a/tests/nvme/045
>>> +++ b/tests/nvme/045
>>> @@ -15,6 +15,7 @@ requires() {
>>> _have_loop
>>> _have_kernel_option NVME_AUTH
>>> _have_kernel_option NVME_TARGET_AUTH
>>> + _kver_gt_or_eq 6 7 && _have_kernel_option NVME_HOST_AUTH
>>> _require_nvme_trtype_is_fabrics
>>> _require_nvme_cli_auth
>>> _have_driver dh_generic
>>
>> Why do we need to check for the kernel version?
>> Any kernel not having the NVME_HOST_AUTH config symbol clearly will
>> have it unset, no?
>> I'd rather update _have_kernel_option to handle the case where
>> a config symbol is not present, treating it as unset.
>
> At this point, _have_kernel_option() already handles NVME_HOST_AUTH as unset
> when the kernel does not have the config symbol.
>
>> That way we can drop the dependency on the kernel version (which, btw, is
>> kinda pointless for the development branches anyway).
>
> For the newer kernel, the test cases require NVME_HOST_AUTH is set. In other
> words, the test cases are skipped when NVME_HOST_AUTH is unset. If we follow
> your idea and drop the kernel version dependncy, the test cases will be skipped
> on older kernels which do not have NVME_HOST_AUTH symbol. I wanted to allow
> running the test cases on older kernels, then added the kernel version check.
>
> I agree that kernel version dependency is not the best. As another solution,
> I considered introducing a helper function _kernel_option_exists() which
> checks if one of strings "# CONFIG_NVME_HOST_AUTH is not set" or
> "# CONFIG_NVME_HOST_AUTH=[ym]" exists in kernel config files. With this, we
> can do as follows:
>
> _kernel_option_exists NVME_HOST_AUTH && _have_kernel_option NVME_HOST_AUTH
>
> This assumes that one of the strings always exists in kernel configs. I was not
> sure about the assumption, then chose the way to check kernel version. (Any
> advice on this assumption will be appreciated...)
None of this is really a good solution. Probably we should strive to
make nvme-cli handling this situation correctly; after all, it would
know if the fabrics option is supported or not.
Daniel?
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare at suse.de +49 911 74053 688
SUSE Software Solutions Germany GmbH, Frankenstr. 146, 90461 Nürnberg
Managing Directors: I. Totev, A. Myers, A. McDonald, M. B. Moerman
(HRB 36809, AG Nürnberg)
More information about the Linux-nvme
mailing list