Skip to content

Commit c6bc204

Browse files
igawigaw
committed
[WIP] fabrics: do not allocate new host in lookup function
It is very surprising for casual readers that the lookup function always returns a new host object. It also has the potential to hide problems. Let's split the functionality into two functions a nvme_create_host and a nvme_lookup_host. TODO: lookup ctrl too Signed-off-by: Daniel Wagner <[email protected]>
1 parent e80be76 commit c6bc204

4 files changed

Lines changed: 68 additions & 28 deletions

File tree

‎libnvme/src/libnvme.map‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ LIBNVME_2_0 {
1616
nvme_close;
1717
nvme_create_ctrl;
1818
nvme_create_global_ctx;
19+
nvme_create_host;
1920
nvme_ctrl_config_match;
2021
nvme_ctrl_find;
2122
nvme_ctrl_first_ns;

‎libnvme/src/nvme/fabrics.c‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2045,7 +2045,7 @@ static nvme_ctrl_t lookup_ctrl(nvme_host_t h, struct fabric_args *trcfg)
20452045
return NULL;
20462046
}
20472047

2048-
static int lookup_host(struct nvme_global_ctx *ctx,
2048+
static int lookup_or_create_host(struct nvme_global_ctx *ctx,
20492049
struct nvmf_context *fctx, struct nvme_host **host)
20502050
{
20512051
_cleanup_free_ char *hnqn = NULL;
@@ -2058,9 +2058,14 @@ static int lookup_host(struct nvme_global_ctx *ctx,
20582058
return err;
20592059

20602060
h = nvme_lookup_host(ctx, hnqn, hid);
2061-
if (!h)
2061+
if (h)
2062+
goto out;
2063+
2064+
err = nvme_create_host(ctx, fctx->hostnqn, fctx->hostid, &h);
2065+
if (err)
20622066
return -ENOMEM;
20632067

2068+
out:
20642069
*host = h;
20652070

20662071
return 0;
@@ -2437,7 +2442,7 @@ int nvmf_discovery_config_json(struct nvme_global_ctx *ctx,
24372442
struct nvme_ctrl *c;
24382443
int ret = 0, err;
24392444

2440-
err = lookup_host(ctx, fctx, &h);
2445+
err = lookup_or_create_host(ctx, fctx, &h);
24412446
if (err)
24422447
return err;
24432448

@@ -2486,7 +2491,7 @@ int nvmf_connect_config_json(struct nvme_global_ctx *ctx,
24862491
nvme_ctrl_t c, _c;
24872492
int ret = 0, err;
24882493

2489-
err = lookup_host(ctx, fctx, &h);
2494+
err = lookup_or_create_host(ctx, fctx, &h);
24902495
if (err)
24912496
return err;
24922497

@@ -2542,7 +2547,7 @@ int nvmf_discovery_config_file(struct nvme_global_ctx *ctx,
25422547
struct nvme_ctrl *c;
25432548
int err;
25442549

2545-
err = lookup_host(ctx, fctx, &h);
2550+
err = lookup_or_create_host(ctx, fctx, &h);
25462551
if (err)
25472552
return err;
25482553

@@ -2856,7 +2861,7 @@ int nvmf_discovery_nbft(struct nvme_global_ctx *ctx,
28562861
struct nvme_host *h;
28572862
int ret, rr, i;
28582863

2859-
ret = lookup_host(ctx, fctx, &h);
2864+
ret = lookup_or_create_host(ctx, fctx, &h);
28602865
if (ret)
28612866
return ret;
28622867

@@ -2899,8 +2904,11 @@ int nvmf_discovery_nbft(struct nvme_global_ctx *ctx,
28992904

29002905
h = nvme_lookup_host(ctx, hostnqn, hostid);
29012906
if (!h) {
2902-
ret = -ENOENT;
2903-
goto out_free;
2907+
ret = nvme_create_host(ctx, hostnqn, hostid, &h);
2908+
if (ret) {
2909+
ret = -ENOENT;
2910+
goto out_free;
2911+
}
29042912
}
29052913

29062914
/* Subsystem Namespace Descriptor List */
@@ -3154,7 +3162,7 @@ int nvmf_discovery(struct nvme_global_ctx *ctx, struct nvmf_context *fctx,
31543162
struct nvme_host *h;
31553163
int ret;
31563164

3157-
ret = lookup_host(ctx, fctx, &h);
3165+
ret = lookup_or_create_host(ctx, fctx, &h);
31583166
if (ret)
31593167
return ret;
31603168

@@ -3289,7 +3297,7 @@ int nvmf_connect(struct nvme_global_ctx *ctx, struct nvmf_context *fctx)
32893297
struct nvme_ctrl *c;
32903298
int err;
32913299

3292-
err = lookup_host(ctx, fctx, &h);
3300+
err = lookup_or_create_host(ctx, fctx, &h);
32933301
if (err)
32943302
return err;
32953303

‎libnvme/src/nvme/tree.c‎

Lines changed: 35 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ int nvme_host_get_ids(struct nvme_global_ctx *ctx,
196196
return 0;
197197
}
198198

199-
int nvme_default_host(struct nvme_global_ctx *ctx, nvme_host_t *hp)
199+
int nvme_default_host(struct nvme_global_ctx *ctx, struct nvme_host **host)
200200
{
201201
_cleanup_free_ char *hostnqn = NULL;
202202
_cleanup_free_ char *hostid = NULL;
@@ -208,11 +208,16 @@ int nvme_default_host(struct nvme_global_ctx *ctx, nvme_host_t *hp)
208208
return err;
209209

210210
h = nvme_lookup_host(ctx, hostnqn, hostid);
211+
if (!h) {
212+
err = nvme_create_host(ctx, hostnqn, hostid, &h);
213+
if (err)
214+
return err;
215+
}
211216

212217
nvme_host_set_hostsymname(h, NULL);
213218

214219
default_host = h;
215-
*hp = h;
220+
*host = h;
216221
return 0;
217222
}
218223

@@ -774,34 +779,48 @@ void nvme_free_host(struct nvme_host *h)
774779
__nvme_free_host(h);
775780
}
776781

777-
struct nvme_host *nvme_lookup_host(struct nvme_global_ctx *ctx, const char *hostnqn,
778-
const char *hostid)
782+
int nvme_create_host(struct nvme_global_ctx *ctx, const char *hostnqn,
783+
const char *hostid, struct nvme_host **host)
779784
{
780785
struct nvme_host *h;
781786

782-
if (!hostnqn)
783-
return NULL;
784-
nvme_for_each_host(ctx, h) {
785-
if (strcmp(h->hostnqn, hostnqn))
786-
continue;
787-
if (hostid && (!h->hostid ||
788-
strcmp(h->hostid, hostid)))
789-
continue;
790-
return h;
791-
}
792787
h = calloc(1,sizeof(*h));
793788
if (!h)
794-
return NULL;
789+
return -ENOMEM;
790+
795791
h->hostnqn = strdup(hostnqn);
796792
if (hostid)
797793
h->hostid = strdup(hostid);
794+
798795
list_head_init(&h->subsystems);
799796
list_node_init(&h->entry);
797+
800798
h->ctx = ctx;
801799
list_add_tail(&ctx->hosts, &h->entry);
802800
ctx->modified = true;
803801

804-
return h;
802+
*host = h;
803+
804+
return 0;
805+
}
806+
807+
struct nvme_host *nvme_lookup_host(struct nvme_global_ctx *ctx,
808+
const char *hostnqn, const char *hostid)
809+
{
810+
struct nvme_host *h;
811+
812+
if (!hostnqn)
813+
return NULL;
814+
nvme_for_each_host(ctx, h) {
815+
if (strcmp(h->hostnqn, hostnqn))
816+
continue;
817+
if (hostid && (!h->hostid ||
818+
strcmp(h->hostid, hostid)))
819+
continue;
820+
return h;
821+
}
822+
823+
return NULL;
805824
}
806825

807826
static int nvme_subsystem_scan_namespaces(struct nvme_global_ctx *ctx, nvme_subsystem_t s)

‎libnvme/src/nvme/tree.h‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,14 +96,26 @@ nvme_host_t nvme_next_host(struct nvme_global_ctx *ctx, nvme_host_t h);
9696
*/
9797
struct nvme_global_ctx *nvme_host_get_global_ctx(nvme_host_t h);
9898

99+
/**
100+
* nvme_create_host() - Lookup nvme_host_t object
101+
* @ctx: struct nvme_global_ctx object
102+
* @hostnqn: Host NQN
103+
* @hostid: Host ID
104+
* @host: Newly allocated host object
105+
*
106+
* Create nvme_host_t object based on @hostnqn and @hostid.
107+
*
108+
* Return: 0 on success or negative error code otherwise
109+
*/
110+
int nvme_create_host(struct nvme_global_ctx *ctx, const char *hostnqn,
111+
const char *hostid, struct nvme_host **host);
99112
/**
100113
* nvme_lookup_host() - Lookup nvme_host_t object
101114
* @ctx: struct nvme_global_ctx object
102115
* @hostnqn: Host NQN
103116
* @hostid: Host ID
104117
*
105-
* Lookup a nvme_host_t object based on @hostnqn and @hostid
106-
* or create one if not found.
118+
* Lookup a nvme_host_t object based on @hostnqn and @hostid.
107119
*
108120
* Return: &nvme_host_t object
109121
*/

0 commit comments

Comments
 (0)