Skip to content

Consider file config when adding controller#363

Merged
igaw merged 3 commits intolinux-nvme:masterfrom
igaw:apply-file-config
May 24, 2022
Merged

Consider file config when adding controller#363
igaw merged 3 commits intolinux-nvme:masterfrom
igaw:apply-file-config

Conversation

@igaw
Copy link
Copy Markdown
Collaborator

@igaw igaw commented May 19, 2022

fabrics: Consider config from file when adding new controller

nvme_read_config() function is attaching the configuration
to tree. But when we create a new controller via nvme_create_ctrl()
and then call nvmf_add_ctrl() we ignore this previously read
in configuration.

Hence lookup if there exist a controller/config and merge into
the fabrics config.

Note, the order of the merge is important. For example we want
the config from the command line to have higher priority
than the one from the config file.

Signed-off-by: Daniel Wagner dwagner@suse.de

Fixes: linux-nvme/nvme-cli#1520

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 19, 2022

Codecov Report

Merging #363 (2e5d70c) into master (34475e2) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

@@            Coverage Diff            @@
##           master    #363      +/-   ##
=========================================
- Coverage    3.75%   3.75%   -0.01%     
=========================================
  Files          23      23              
  Lines        3565    3573       +8     
  Branches      689     692       +3     
=========================================
  Hits          134     134              
- Misses       3360    3368       +8     
  Partials       71      71              
Impacted Files Coverage Δ
src/nvme/fabrics.c 4.91% <0.00%> (-0.05%) ⬇️
src/nvme/tree.c 8.97% <0.00%> (-0.03%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 34475e2...2e5d70c. Read the comment docs.

dwsuse added 3 commits May 19, 2022 11:06
We missed to export the nvme_ctrl_get_config() function in
version 1.0.

Signed-off-by: Daniel Wagner <dwagner@suse.de>
nvme_lookup_ctrl() will create a controller if it is not
found. In the case where we really just want the lookup
if a controller is in the tree we are missing a pure
lookup function. Hence factor out the lookup into a internal
function.

Signed-off-by: Daniel Wagner <dwagner@suse.de>
nvme_read_config() function is attaching the configuration
to tree. But when we create a new controller via nvme_create_ctrl()
and then call nvmf_add_ctrl() we ignore this previously read
in configuration.

Hence lookup if there exist a controller/config and merge into
the fabrics config.

Note, the order of the merge is important. For example we want
the config from the command line to have higher priority
than the one from the config file.

Signed-off-by: Daniel Wagner <dwagner@suse.de>
@igaw igaw force-pushed the apply-file-config branch from 662f648 to 2e5d70c Compare May 19, 2022 09:07
@igaw igaw requested a review from hreinecke May 19, 2022 12:59
@igaw igaw merged commit 0fca385 into linux-nvme:master May 24, 2022
@igaw igaw deleted the apply-file-config branch May 24, 2022 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

nvme connect -J option is broken

3 participants