[PATCH blktests 2/2] nvme/{041,042,043,044,045}: require kernel config NVME_HOST_AUTH

[Hannes Reinecke]

On 11/15/23 15:18, Daniel Wagner wrote:
> On Wed, Nov 15, 2023 at 09:20:28AM +0100, Hannes Reinecke wrote:
>>> 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.
> 
> nvme-cli is detecting if a feature is present by reading
> /dev/nvme-fabrics. I think we should do something similar in blktest
> but not by calling nvme-cli in the _require() block. Though we don't
> have to rely on nvme-cli. We can do something like this:
> 
> 
> diff --git a/tests/nvme/045 b/tests/nvme/045
> index 1eb1032a3b93..954f96bedd5a 100755
> --- a/tests/nvme/045
> +++ b/tests/nvme/045
> @@ -17,6 +17,7 @@ requires() {
>          _have_kernel_option NVME_TARGET_AUTH
>          _require_nvme_trtype_is_fabrics
>          _require_nvme_cli_auth
> +       _require_kernel_nvme_feature dhchap_ctrl_secret
>          _have_driver dh_generic
>   }
> 
> diff --git a/tests/nvme/rc b/tests/nvme/rc
> index 1cff522d8543..67b812cf0c66 100644
> --- a/tests/nvme/rc
> +++ b/tests/nvme/rc
> @@ -155,6 +155,17 @@ _require_nvme_cli_auth() {
>          return 0
>   }
> 
> +_require_kernel_nvme_feature() {
> +       local feature="$1"
> +
> +       if ! [ -f /dev/nvme-fabrics ]; then
> +               return 1;
> +       fi
> +
> +       grep "${feature}" /dev/nvme-fabrics
> +       return $?
> +}
> +
>   _test_dev_nvme_ctrl() {
>          echo "/dev/char/$(cat "${TEST_DEV_SYSFS}/device/dev")"
>   }
> 
Exactly.
Far better :-)

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)