Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't trigger discover from json file if it doesn't exist #2414

Merged
merged 3 commits into from
Aug 1, 2024
Merged

Don't trigger discover from json file if it doesn't exist #2414

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9174789
fabrics: check if json config is existing
igaw Jul 11, 2024
be3d098
fabrics: drop --disable-sqflow alias -d
igaw Jul 11, 2024
fff6c3e
fabrics: drop --quiet alias -S
igaw Jul 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions Documentation/nvme-config.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ SYNOPSIS
[--keep-alive-tmo=<#> | -k <#>]
[--reconnect-delay=<#> | -c <#>]
[--ctrl-loss-tmo=<#> | -l <#>]
[--duplicate-connect | -D] [--disable-sqflow | -d]
[--duplicate-connect | -D] [--disable-sqflow ]
[--hdr-digest | -g] [--data-digest | -G]
[--output-format=<fmt> | -o <fmt>] [--verbose | -v]

Expand Down Expand Up @@ -168,7 +168,6 @@ OPTIONS
Allows duplicated connections between same transport host and subsystem
port.

-d::
--disable-sqflow::
Disables SQ flow control to omit head doorbell update for submission
queues when sending nvme completions.
Expand Down
3 changes: 1 addition & 2 deletions Documentation/nvme-connect-all.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ SYNOPSIS
[--nr-poll-queues=<#> | -P <#>]
[--queue-size=<#> | -Q <#>] [--keyring=<#>]
[--tls_key=<#>] [--hdr-digest | -g] [--data-digest | -G]
[--persistent | -p] [--tls] [--concat] [--quiet | -S]
[--persistent | -p] [--tls] [--concat] [--quiet]
[--dump-config | -O] [--nbft] [--no-nbft]
[--nbft-path=<STR>] [--context=<STR>]
[--output-format=<fmt> | -o <fmt>] [--verbose | -v]
Expand Down Expand Up @@ -189,7 +189,6 @@ OPTIONS
--concat::
Enable secure concatenation (TCP).

-S::
--quiet::
Suppress error messages.

Expand Down
3 changes: 1 addition & 2 deletions Documentation/nvme-connect.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ SYNOPSIS
[--reconnect-delay=<#> | -c <#>]
[--ctrl-loss-tmo=<#> | -l <#>] [--tos=<#> | -T <#>]
[--keyring=<#>] [--tls_key=<#>]
[--duplicate-connect | -D] [--disable-sqflow | -d]
[--duplicate-connect | -D] [--disable-sqflow ]
[--hdr-digest | -g] [--data-digest | -G] [--tls]
[--concat] [--dump-config | -O] [--application=<id>]
[--output-format=<fmt> | -o <fmt>] [--verbose | -v]
Expand Down Expand Up @@ -162,7 +162,6 @@ OPTIONS
Allows duplicated connections between same transport host and subsystem
port.

-d::
--disable-sqflow::
Disables SQ flow control to omit head doorbell update for submission
queues when sending nvme completions.
Expand Down
3 changes: 1 addition & 2 deletions Documentation/nvme-discover.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ SYNOPSIS
[--nr-poll-queues=<#> | -P <#>]
[--queue-size=<#> | -Q <#>] [--keyring=<#>]
[--tls_key=<#>] [--hdr-digest | -g] [--data-digest | -G]
[--persistent | -p] [--quiet | -S] [--tls] [--concat]
[--persistent | -p] [--quiet] [--tls] [--concat]
[--dump-config | -O] [--output-format=<fmt> | -o <fmt>]
[--force] [--nbft] [--no-nbft] [--nbft-path=<STR>]
[--context=<STR>]
Expand Down Expand Up @@ -209,7 +209,6 @@ OPTIONS
--concat::
Enable secure concatenation (TCP).

-S::
--quiet::
Suppress already connected errors.

Expand Down
5 changes: 0 additions & 5 deletions completions/_nvme
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2014,7 +2014,6 @@ _nvme () {
--duplicate-connect':allow duplicate connections between same transport host and subsystem port'
-D':alias for --duplicate-connect'
--disable-sqflow':disable controller sq flow control (default false)'
-d':alias for --disable-sqflow'
--hdr-digest':enable transport protocol header digest (TCP transport)'
-g':alias for --hdr-digest'
--data-digest':enable transport protocol data digest (TCP transport)'
Expand All @@ -2027,7 +2026,6 @@ _nvme () {
--persistent':'
-p':alias for --'
--quiet':'
-S':alias for --'
--config=':Use specified JSON configuration file or none to disable'
-J':alias for --config'
--verbose':Increase logging verbosity'
Expand Down Expand Up @@ -2086,7 +2084,6 @@ _nvme () {
--duplicate-connect':allow duplicate connections between same transport host and subsystem port'
-D':alias for --duplicate-connect'
--disable-sqflow':disable controller sq flow control (default false)'
-d':alias for --disable-sqflow'
--hdr-digest':enable transport protocol header digest (TCP transport)'
-g':alias for --hdr-digest'
--data-digest':enable transport protocol data digest (TCP transport)'
Expand All @@ -2099,7 +2096,6 @@ _nvme () {
--persistent':'
-p':alias for --'
--quiet':'
-S':alias for --'
--config=':Use specified JSON configuration file or none to disable'
-J':alias for --config'
--verbose':Increase logging verbosity'
Expand Down Expand Up @@ -2156,7 +2152,6 @@ _nvme () {
--duplicate-connect':allow duplicate connections between same transport host and subsystem port'
-D':alias for --duplicate-connect'
--disable-sqflow':disable controller sq flow control (default false)'
-d':alias for --disable-sqflow'
--hdr-digest':enable transport protocol header digest (TCP transport)'
-g':alias for --hdr-digest'
--data-digest':enable transport protocol data digest (TCP transport)'
Expand Down
6 changes: 3 additions & 3 deletions completions/bash-nvme-completion.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ nvme_list_opts () {
--tos= -T --hdr-digest= -g --data-digest -G \
--nr-io-queues= -i --nr-write-queues= -W \
--nr-poll-queues= -P --queue-size= -Q \
--persistent -p --quiet -S \
--persistent -p --quiet \
--output-format= -o"
;;
"connect-all")
Expand All @@ -400,7 +400,7 @@ nvme_list_opts () {
--tos= -T --hdr-digest= -g --data-digest -G \
--nr-io-queues= -i --nr-write-queues= -W \
--nr-poll-queues= -P --queue-size= -Q \
--persistent -p --quiet -S \
--persistent -p --quiet \
--output-format= -o"
;;
"connect")
Expand All @@ -409,7 +409,7 @@ nvme_list_opts () {
--nr-poll-queues= -P --queue-size= -Q \
--keep-alive-tmo= -k --reconnect-delay= -r \
--ctrl-loss-tmo= -l --fast-io-fail-tmo= -f \
--tos= -T --duplicate-connect -D --disable-sqflow -d\
--tos= -T --duplicate-connect -D --disable-sqflow \
--hdr-digest -g --data-digest -G --output-format= -o"
;;
"dim")
Expand Down
17 changes: 13 additions & 4 deletions fabrics.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@
OPT_INT("keyring", 0, &c.keyring, nvmf_keyring), \
OPT_INT("tls_key", 0, &c.tls_key, nvmf_tls_key), \
OPT_FLAG("duplicate-connect", 'D', &c.duplicate_connect, nvmf_dup_connect), \
OPT_FLAG("disable-sqflow", 'd', &c.disable_sqflow, nvmf_disable_sqflow), \
OPT_FLAG("disable-sqflow", 0, &c.disable_sqflow, nvmf_disable_sqflow), \
OPT_FLAG("hdr-digest", 'g', &c.hdr_digest, nvmf_hdr_digest), \
OPT_FLAG("data-digest", 'G', &c.data_digest, nvmf_data_digest), \
OPT_FLAG("tls", 0, &c.tls, nvmf_tls), \
Expand Down Expand Up @@ -420,7 +420,7 @@
OPT_FMT("output-format", 'o', &format, output_format),
OPT_FILE("raw", 'r', &raw, "save raw output to file"),
OPT_FLAG("persistent", 'p', &persistent, "persistent discovery connection"),
OPT_FLAG("quiet", 'S', &quiet, "suppress already connected errors"),
OPT_FLAG("quiet", 0, &quiet, "suppress already connected errors"),
OPT_INCR("verbose", 'v', &verbose, "Increase logging verbosity"),
OPT_FLAG("force", 0, &force, "Force persistent discovery controller creation"));

Expand Down Expand Up @@ -666,6 +666,15 @@
return ret;
}

static int nvme_read_config_checked(nvme_root_t r, const char *filename)
{
if (!access(filename, F_OK))
return -ENOENT;
if (nvme_read_config(r, filename))
return -errno;
return 0;
}

int nvmf_discover(const char *desc, int argc, char **argv, bool connect)
{
char *subsysnqn = NVME_DISC_SUBSYS_NAME;
Expand Down Expand Up @@ -694,7 +703,7 @@
OPT_FMT("output-format", 'o', &format, output_format),
OPT_FILE("raw", 'r', &raw, "save raw output to file"),
OPT_FLAG("persistent", 'p', &persistent, "persistent discovery connection"),
OPT_FLAG("quiet", 'S', &quiet, "suppress already connected errors"),
OPT_FLAG("quiet", 0, &quiet, "suppress already connected errors"),

Check failure on line 706 in fabrics.c

View workflow job for this annotation

GitHub Actions / checkpatch review 

WARNING: line length of 107 exceeds 100 columns
OPT_STRING("config", 'J', "FILE", &config_file, nvmf_config_file),
OPT_INCR("verbose", 'v', &verbose, "Increase logging verbosity"),
OPT_FLAG("dump-config", 'O', &dump_config, "Dump configuration file to stdout"),
Expand Down Expand Up @@ -730,7 +739,7 @@
if (context)
nvme_root_set_application(r, context);

if (!nvme_read_config(r, config_file))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems okay to check the errno value as below.

    errno = 0;
    if (!nvme_read_config(r, config_file) && !errno)

Copy link
Collaborator Author

@igaw igaw Jul 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nvme_read_config should actually not touch errno on success. This is actually a bug in libnvme... Good catch!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes agreed. Thank you.

if (!nvme_read_config_checked(r, config_file))
json_config = true;
if (!nvme_read_volatile_config(r))
json_config = true;
Expand Down