[PATH libnvme v1 1/2] fabrics: Shorten/rename supported option function/vars names

Sagi Grimberg sagi at grimberg.me
Tue Jul 25 03:55:00 PDT 2023


> No need to be extra verbose and using so many chars per line.
> 
> Also rename to __nvmf_supported_options to nvmf_read_options
> which describes better what the function actually is doing.
> 
> Signed-off-by: Daniel Wagner <dwagner at suse.de>
> ---
>   src/nvme/fabrics.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/src/nvme/fabrics.c b/src/nvme/fabrics.c
> index 800293e2a8e7..2f26c2b7f39a 100644
> --- a/src/nvme/fabrics.c
> +++ b/src/nvme/fabrics.c
> @@ -202,7 +202,7 @@ const char *nvmf_cms_str(__u8 cm)
>    * Not all of these options may actually be supported,
>    * but we retain the old behavior of passing all that might be.
>    */
> -static const struct nvme_fabric_options default_supported_options = {
> +static const struct nvme_fabric_options default_options = {
>   	.ctrl_loss_tmo = true,
>   	.data_digest = true,
>   	.disable_sqflow = true,
> @@ -652,7 +652,7 @@ static int build_options(nvme_host_t h, nvme_ctrl_t c, char **argstr)
>   		continue;		   \
>   	}
>   
> -static  int __nvmf_supported_options(nvme_root_t r)
> +static int nvmf_read_options(nvme_root_t r)
>   {
>   	char buf[0x1000], *options, *p, *v;
>   	int fd, ret;
> @@ -683,7 +683,7 @@ static  int __nvmf_supported_options(nvme_root_t r)
>   			nvme_msg(r, LOG_DEBUG,
>   			         "Cannot read %s, using default options\n",
>   			         nvmf_dev);
> -			*r->options = default_supported_options;
> +			*r->options = default_options;
>   			ret = 0;
>   			goto out_close;
>   		}
> @@ -913,7 +913,7 @@ int nvmf_add_ctrl(nvme_host_t h, nvme_ctrl_t c,
>   		free(traddr);
>   	}
>   
> -	ret = __nvmf_supported_options(h->r);
> +	ret = nvmf_read_options(h->r);
>   	if (ret)
>   		return ret;
>   	ret = build_options(h, c, &argstr);

Why do we need an explicit call here?
Why it cannot be read on the first capability check?

Something like:
--
diff --git a/src/nvme/fabrics.c b/src/nvme/fabrics.c
index 800293e2a8e7..bb4bf2979eb0 100644
--- a/src/nvme/fabrics.c
+++ b/src/nvme/fabrics.c
@@ -357,10 +357,18 @@ static int __add_argument(char **argstr, const 
char *tok, const char *arg)
         return 0;
  }

+static int __nvmf_supported_options(nvme_root_t r);
+#define nvmf_check_option(r, tok)                                      \
+({                                                                     \
+       if (!(r)->options)                                              \
+               __nvmf_supported_options(r);                            \
+       (r)->options->tok;                                              \
+})
+
  #define add_bool_argument(o, argstr, tok, arg)                         \
  ({                                                                     \
         int ret;                                                        \
-       if (r->options->tok) {                                          \
+       if (nvmf_check_option(r, tok)) {                                \
                 ret = __add_bool_argument(argstr,                       \
                                           stringify(tok),               \
                                           arg);                         \
@@ -376,7 +384,7 @@ static int __add_argument(char **argstr, const char 
*tok, const char *arg)
  #define add_int_argument(o, argstr, tok, arg, allow_zero) \
  ({                                                                     \
         int ret;                                                        \
-       if (r->options->tok) {                                          \
+       if (nvmf_check_option(r, tok)) {                                \
                 ret = __add_int_argument(argstr,                        \
                                         stringify(tok),                 \
                                         arg,                            \
@@ -393,7 +401,7 @@ static int __add_argument(char **argstr, const char 
*tok, const char *arg)
  #define add_int_or_minus_one_argument(o, argstr, tok, arg)             \
  ({                                                                     \
         int ret;                                                        \
-       if (r->options->tok) {                                          \
+       if (nvmf_check_option(r, tok)) {                                \
                 ret = __add_int_or_minus_one_argument(argstr,           \
                                                      stringify(tok),    \
                                                      arg);              \
@@ -409,7 +417,7 @@ static int __add_argument(char **argstr, const char 
*tok, const char *arg)
  #define add_argument(r, argstr, tok, arg)
  ({                                                                     \
         int ret;                                                        \
-       if (r->options->tok) {                                          \
+       if (nvmf_check_option(r, tok)) {                                \
                 ret = __add_argument(argstr,                            \
                                      stringify(tok),                    \
                                      arg);                              \
@@ -913,9 +921,6 @@ int nvmf_add_ctrl(nvme_host_t h, nvme_ctrl_t c,
                 free(traddr);
         }

-       ret = __nvmf_supported_options(h->r);
-       if (ret)
-               return ret;
         ret = build_options(h, c, &argstr);
         if (ret)
                 return ret;
--

Then, we can spray options check wherever we want...
--
@@ -1026,8 +1031,10 @@ nvme_ctrl_t nvmf_connect_disc_entry(nvme_host_t h,
                 return NULL;
         }

-       if (e->treq & NVMF_TREQ_DISABLE_SQFLOW)
+       if (e->treq & NVMF_TREQ_DISABLE_SQFLOW && 
nvmf_check_option(h->r, disable_sqflow))
                 c->cfg.disable_sqflow = true;
+       else
+               c->cfg.disable_sqflow = false;

         if (e->trtype == NVMF_TRTYPE_TCP &&
             (e->treq & NVMF_TREQ_REQUIRED ||
--



More information about the Linux-nvme mailing list