Skip to contentSkip to conversation

drivers/target/sbp/ : set tport->tpg to NULL when clean up for failure.

Suggestion Chen Gang 4 participants 11 messages Dec 6, 2012 – Dec 15, 2012 SCSI

lore
Share & tools
lore

Chen Gang

author
#0 lore raw

Hello Maintainers:

in drivers/target/sbp/sbp_target.c:

tport->tpg must be NULL before process it in function sbp_make_tpg. (line 2185..2188) tport->tpg assigned a ptr (line 2198)
if processing failed, not set tport->tpg = NULL (line 2208..2212, 2217..2221)

we have done: when free tport->tpg, set it to NULL (line 2233..2234)

is it valuable to let tport->tpg = NULL, when clean up for failure ?

Regards

gchen.

2168 static struct se_portal_group *sbp_make_tpg(

2169                 struct se_wwn *wwn,
2170                 struct config_group *group,
2171                 const char *name)

2172 {
2173 struct sbp_tport *tport =
2174 container_of(wwn, struct sbp_tport, tport_wwn);
2175

2176         struct sbp_tpg *tpg;
2177         unsigned long tpgt;
2178         int ret;

2179
2180 if (strstr(name, "tpgt_") != name)
2181 return ERR_PTR(-EINVAL);
2182 if (kstrtoul(name + 5, 10, &tpgt) || tpgt > UINT_MAX)
2183 return ERR_PTR(-EINVAL);
2184
2185 if (tport->tpg) {
2186 pr_err("Only one TPG per Unit is possible.\n");
2187 return ERR_PTR(-EBUSY);
2188 }
2189
2190 tpg = kzalloc(sizeof(*tpg), GFP_KERNEL);
2191 if (!tpg) {
2192 pr_err("Unable to allocate struct sbp_tpg\n");
2193 return ERR_PTR(-ENOMEM);
2194 }
2195

2196         tpg->tport = tport;
2197         tpg->tport_tpgt = tpgt;
2198         tport->tpg = tpg;
2199 
2200         /* default attribute values */
2201         tport->enable = 0;
2202         tport->directory_id = -1;
2203         tport->mgt_orb_timeout = 15;
2204         tport->max_reconnect_timeout = 5;
2205         tport->max_logins_per_lun = 1;
2206 
2207         tport->mgt_agt = sbp_management_agent_register(tport);
2208         if (IS_ERR(tport->mgt_agt)) {
2209                 ret = PTR_ERR(tport->mgt_agt);
2210                 kfree(tpg);
2211                 return ERR_PTR(ret);

2212 }
2213
2214 ret = core_tpg_register(&sbp_fabric_configfs->tf_ops, wwn,
2215 &tpg->se_tpg, (void *)tpg,
2216 TRANSPORT_TPG_TYPE_NORMAL);
2217 if (ret < 0) {

2218                 sbp_management_agent_unregister(tport->mgt_agt);
2219                 kfree(tpg);
2220                 return ERR_PTR(ret);

2221 }
2222
2223 return &tpg->se_tpg;
2224 }
2225
2226 static void sbp_drop_tpg(struct se_portal_group *se_tpg) 2227 {
2228 struct sbp_tpg *tpg = container_of(se_tpg, struct sbp_tpg, se_tpg);
2229 struct sbp_tport *tport = tpg->tport;
2230

2231         core_tpg_deregister(se_tpg);
2232         sbp_management_agent_unregister(tport->mgt_agt);
2233         tport->tpg = NULL;
2234         kfree(tpg);

2235 }
2236

#1 lore raw
Quoted 4 lines · Chen Gang

Hello Maintainers:

in drivers/target/sbp/sbp_target.c:

Quoted 8 lines

tport->tpg must be NULL before process it in function sbp_make_tpg. (line 2185..2188) tport->tpg assigned a ptr (line 2198)
if processing failed, not set tport->tpg = NULL (line 2208..2212, 2217..2221)

we have done: when free tport->tpg, set it to NULL (line 2233..2234)

is it valuable to let tport->tpg = NULL, when clean up for failure ?

<snip>

Yes, that's a good catch. The way it is now, if we fail to set up the TPG, the port will be left in an inconsistent state. I'll prepare a patch ASAP unless you'd rather do it yourself.

Thanks,
Chris

Signature · 2 lines
Chris Boot
[email protected]
author
#2 lore raw
Quoted 13 lines · Chris Boot
Quoted 4 lines · Chen Gang

Hello Maintainers:

in drivers/target/sbp/sbp_target.c:

Quoted 8 lines

tport->tpg must be NULL before process it in function sbp_make_tpg. (line 2185..2188) tport->tpg assigned a ptr (line 2198)
if processing failed, not set tport->tpg = NULL (line 2208..2212, 2217..2221)

we have done: when free tport->tpg, set it to NULL (line 2233..2234)

is it valuable to let tport->tpg = NULL, when clean up for failure ?

Quoted 9 lines

<snip>

Yes, that's a good catch. The way it is now, if we fail to set up the TPG, the port will be left in an inconsistent state. I'll prepare a patch ASAP unless you'd rather do it yourself.

Thanks,
Chris

please help do it (better to mark me as Reported-by in your patch).

thanks.

:-)

Signature · 7 lines
Chen Gang

Asianux Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#3 lore raw

Hello Chris Boot:

need I send the relative patch ?

Regards

gchen.

Quoted 31 lines · Chen Gang
Quoted 13 lines · Chris Boot
Quoted 4 lines · Chen Gang

Hello Maintainers:

in drivers/target/sbp/sbp_target.c:

Quoted 8 lines

tport->tpg must be NULL before process it in function sbp_make_tpg. (line 2185..2188) tport->tpg assigned a ptr (line 2198)
if processing failed, not set tport->tpg = NULL (line 2208..2212, 2217..2221)

we have done: when free tport->tpg, set it to NULL (line 2233..2234)

is it valuable to let tport->tpg = NULL, when clean up for failure ?

Quoted 9 lines

<snip>

Yes, that's a good catch. The way it is now, if we fail to set up the TPG, the port will be left in an inconsistent state. I'll prepare a patch ASAP unless you'd rather do it yourself.

Thanks,
Chris

please help do it (better to mark me as Reported-by in your patch).

thanks.

:-)

Signature · 3 lines
Chen Gang

Asianux Corporation
#4 lore raw
Quoted 3 lines · Chen Gang

Hello Chris Boot:

need I send the relative patch ?

Hi Chen,

Sorry, life got in the way this weekend. I'll try to get the patch sent out today.

Cheers,
Chris

Signature · 2 lines
Chris Boot
[email protected]
author
#5 lore raw
Quoted 5 lines · Chris Boot
Quoted 3 lines · Chen Gang

Hello Chris Boot:

need I send the relative patch ?

Quoted 8 lines

Hi Chen,

Sorry, life got in the way this weekend. I'll try to get the patch sent out today.

Cheers,
Chris

ok, thank you.

mark me as Reported-by is ok, not need cc to me (I am not reviewer)

:-)

Regards

Signature · 3 lines
Chen Gang

Asianux Corporation
author
#6 lore raw
In reply to Chris Boot
Quoted 10 lines · Chris Boot
Quoted 3 lines · Chen Gang

Hello Chris Boot:

need I send the relative patch ?

Hi Chen,

Sorry, life got in the way this weekend. I'll try to get the patch sent out today.

Have you sent ?

I am already in [email protected] mailing list, but it seems
not find your patch.

Regards

gchen.

Quoted 3 lines

Cheers,
Chris

Signature · 3 lines
Chen Gang

Asianux Corporation
#7 lore raw
Quoted 8 lines · On Dec 14 Chen Gang wrote:
Quoted 7 lines · Chris Boot
Quoted 3 lines · Chen Gang

Hello Chris Boot:

need I send the relative patch ?

Hi Chen,

Quoted 8 lines
Quoted 3 lines

Sorry, life got in the way this weekend. I'll try to get the patch sent out today.

Have you sent ?

I am already in [email protected] mailing list, but it seems
not find your patch.

It was posted at target-devel:
http://thread.gmane.org/gmane.linux.scsi.target.devel/3019/focus=3020

Signature · 7 lines
Stefan Richter
-=====-===-- ==-- -===-
http://arcgraph.de/sr/
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#8 lore raw
Quoted 13 lines · Stefan Richter
Quoted 12 lines · On Dec 14 Chen Gang wrote:
Quoted 10 lines · Chris Boot
Quoted 3 lines · Chen Gang

Hello Chris Boot:

need I send the relative patch ?

Hi Chen,

Sorry, life got in the way this weekend. I'll try to get the patch sent out today.

Quoted 8 lines
Quoted 4 lines

Have you sent ?

I am already in [email protected] mailing list, but it seems
not find your patch.

It was posted at target-devel:
http://thread.gmane.org/gmane.linux.scsi.target.devel/3019/focus=3020

oh, thank you very much.

I need join [email protected], I will do.

:-)

Signature · 7 lines
Chen Gang

Asianux Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#9 lore raw
Quoted 19 lines · Chen Gang
Quoted 18 lines · Stefan Richter
Quoted 16 lines · On Dec 14 Chen Gang wrote:
Quoted 10 lines · Chris Boot
Quoted 3 lines · Chen Gang

Hello Chris Boot:

need I send the relative patch ?

Hi Chen,

Sorry, life got in the way this weekend. I'll try to get the patch sent out today.

Have you sent ?

I am already in [email protected] mailing list, but it seems
not find your patch.

Quoted 8 lines
Quoted 3 lines

It was posted at target-devel:
http://thread.gmane.org/gmane.linux.scsi.target.devel/3019/focus=3020

oh, thank you very much.

I need join [email protected], I will do.

FYI folks, this fix from Chris was included in the [GIT PULL] request for v3.8-rc1 sent out today.

Thanks!

--nab

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#10 lore raw
Quoted 4 lines · Nicholas A. Bellinger

FYI folks, this fix from Chris was included in the [GIT PULL] request for v3.8-rc1 sent out today.

thanks

and now I am in [email protected], too.

:-)

Quoted 6 lines

Thanks!

--nab

Signature · 3 lines
Chen Gang

Asianux Corporation