[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