[PATCH libnvme 2/1] nvme: Add generic connect parameter support detection

[Daniel Wagner]

On Wed, Sep 07, 2022 at 05:24:09PM +0300, Sagi Grimberg wrote:
> --- a/src/libnvme.map
> +++ b/src/libnvme.map
> @@ -352,6 +352,7 @@ LIBNVME_1_0 {
>  		nvmf_treq_str;
>  		nvmf_trtype_str;
>  		nvmf_update_config;
> +		nvmf_check_param_support;

New entries go to the 'unreleased' section, this would be LIBNVME_1_2.

> +int nvmf_check_param_support(const char *param, bool *supported)
> +{

I was wondering if t would be better to return all supported features in
one go. Not really sure about it though.

> +	int ret = 0, fd, len;
> +	char buf[0x1000];
> +
> +	fd = open(nvmf_dev, O_RDWR);
> +	if (fd < 0) {
> +		nvme_msg(NULL, LOG_ERR, "Failed to open %s: %s\n",
> +			 nvmf_dev, strerror(errno));

'nvme_msg(NULL,' will always write to the stderr which is kind of bad
for libraries. I think it would okay just to return the error codes in
this function and have the caller do the logging part.

> +		return -ENVME_CONNECT_OPEN;
> +	}
> +
> +	memset(buf, 0x0, sizeof(buf));
> +	len = read(fd, buf, sizeof(buf) - 1);
> +	if (len < 0) {
> +		nvme_msg(NULL, LOG_ERR, "Failed to read from %s: %s\n",
> +			 nvmf_dev, strerror(errno));
> +		ret = -ENVME_CONNECT_READ;
> +		goto out_close;
> +	}
> +	nvme_msg(NULL, LOG_DEBUG, "supported opts: '%.*s'\n",
> (int)strcspn(buf, "\n"), buf);

This triggered my question above why we don't return all supported
features up and have the caller added something like this?

Thanks for doing this! This feature is rotting on my TODO list for a
while...