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

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

[Chen Gang]

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 

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

[Chris Boot]

On 06/12/12 04:24, Chen Gang wrote:
> 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 ?
>
<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

-- 
Chris Boot
bootc@bootc.net

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

[Chen Gang]

于 2012年12月06日 16:34, Chris Boot 写道:
> On 06/12/12 04:24, Chen Gang wrote:
>> 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 ?
>>
> <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.

  :-)


-- 
Chen Gang

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

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

[Chen Gang]

Hello Chris Boot:

  need I send the relative patch ?

  Regards

gchen.


于 2012年12月06日 16:40, Chen Gang 写道:
> 于 2012年12月06日 16:34, Chris Boot 写道:
>> On 06/12/12 04:24, Chen Gang wrote:
>>> 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 ?
>>>
>> <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.
> 
>   :-)
> 
> 


-- 
Chen Gang

Asianux Corporation

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

[Chris Boot]

On 10/12/12 02:39, Chen Gang wrote:
> 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

-- 
Chris Boot
bootc@bootc.net

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

[Chen Gang]

于 2012年12月10日 16:02, Chris Boot 写道:
> On 10/12/12 02:39, Chen Gang wrote:
>> 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
> 

  ok, thank you.

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

  :-)

  Regards

-- 
Chen Gang

Asianux Corporation

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

[Chen Gang]

于 2012年12月10日 16:02, Chris Boot 写道:
> On 10/12/12 02:39, Chen Gang wrote:
>> 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 linux-scsi@vger.kernel.org mailing list, but it seems
not find your patch.

  Regards

gchen.

> Cheers,
> Chris
> 


-- 
Chen Gang

Asianux Corporation

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

[Stefan Richter]

On Dec 14 Chen Gang wrote:
> 于 2012年12月10日 16:02, Chris Boot 写道:
> > On 10/12/12 02:39, Chen Gang wrote:
> >> 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 linux-scsi@vger.kernel.org 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
-- 
Stefan Richter
-=====-===-- ==-- -===-
http://arcgraph.de/sr/
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Chen Gang]

于 2012年12月14日 14:57, Stefan Richter 写道:
> On Dec 14 Chen Gang wrote:
>> 于 2012年12月10日 16:02, Chris Boot 写道:
>>> On 10/12/12 02:39, Chen Gang wrote:
>>>> 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 linux-scsi@vger.kernel.org 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 target-devel@vger.kernel.org, I will do.

  :-)

-- 
Chen Gang

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

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

[Nicholas A. Bellinger]

On Fri, 2012-12-14 at 15:22 +0800, Chen Gang wrote:
> 于 2012年12月14日 14:57, Stefan Richter 写道:
> > On Dec 14 Chen Gang wrote:
> >> 于 2012年12月10日 16:02, Chris Boot 写道:
> >>> On 10/12/12 02:39, Chen Gang wrote:
> >>>> 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 linux-scsi@vger.kernel.org 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 target-devel@vger.kernel.org, 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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Chen Gang]

于 2012年12月15日 08:33, 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 target-devel@vger.kernel.org, too.

  :-)


> Thanks!
> 
> --nab
> 
> 
> 


-- 
Chen Gang

Asianux Corporation