* [PATCH target] target: Add initiatorname to NON_EXISTENT_LUN error
@ 2020-05-14 4:01 Lance Digby
2020-05-15 23:50 ` Mike Christie
0 siblings, 1 reply; 5+ messages in thread
From: Lance Digby @ 2020-05-14 4:01 UTC (permalink / raw)
To: martin.petersen; +Cc: linux-scsi, target-devel, linux-kernel
The NON_EXISTENT_LUN error can be written without an error condition
on the initiator responsible. Adding the initiatorname to this message
will reduce the effort required to fix this when many initiators are
supported by a target.
Signed-off-by: Lance Digby <lance.digby@gmail.com>
---
drivers/target/target_core_device.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index 4cee113..604dea0 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -100,9 +100,10 @@
*/
if (unpacked_lun != 0) {
pr_err("TARGET_CORE[%s]: Detected NON_EXISTENT_LUN"
- " Access for 0x%08llx\n",
+ " Access for 0x%08llx from %s\n",
se_cmd->se_tfo->fabric_name,
- unpacked_lun);
+ unpacked_lun,
+ se_sess->se_node_acl->initiatorname);
return TCM_NON_EXISTENT_LUN;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH target] target: Add initiatorname to NON_EXISTENT_LUN error
2020-05-14 4:01 [PATCH target] target: Add initiatorname to NON_EXISTENT_LUN error Lance Digby
@ 2020-05-15 23:50 ` Mike Christie
2020-05-16 23:29 ` Lance Digby
0 siblings, 1 reply; 5+ messages in thread
From: Mike Christie @ 2020-05-15 23:50 UTC (permalink / raw)
To: Lance Digby, martin.petersen; +Cc: linux-scsi, target-devel, linux-kernel
On 5/13/20 11:01 PM, Lance Digby wrote:
> The NON_EXISTENT_LUN error can be written without an error condition
> on the initiator responsible. Adding the initiatorname to this message
> will reduce the effort required to fix this when many initiators are
> supported by a target.
>
> Signed-off-by: Lance Digby <lance.digby@gmail.com>
> ---
> drivers/target/target_core_device.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
> index 4cee113..604dea0 100644
> --- a/drivers/target/target_core_device.c
> +++ b/drivers/target/target_core_device.c
> @@ -100,9 +100,10 @@
> */
> if (unpacked_lun != 0) {
> pr_err("TARGET_CORE[%s]: Detected NON_EXISTENT_LUN"
> - " Access for 0x%08llx\n",
> + " Access for 0x%08llx from %s\n",
> se_cmd->se_tfo->fabric_name,
> - unpacked_lun);
> + unpacked_lun,
> + se_sess->se_node_acl->initiatorname);
You can do nacl->initiatorname.
Do you also want add the name to the tmr case? It's probably not common,
but the error message would be consistent.
> return TCM_NON_EXISTENT_LUN;
> }
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH target] target: Add initiatorname to NON_EXISTENT_LUN error
2020-05-15 23:50 ` Mike Christie
@ 2020-05-16 23:29 ` Lance Digby
2020-05-17 19:16 ` Mike Christie
0 siblings, 1 reply; 5+ messages in thread
From: Lance Digby @ 2020-05-16 23:29 UTC (permalink / raw)
To: Mike Christie; +Cc: martin.petersen, linux-scsi, target-devel, linux-kernel
Mike, Thanks for the review!
The pr_err Detected NON_EXISTENT_LUN is the error messages issued
for the TCM_NON_EXISTENT_LUN retcode so I believe they are the same.
Simply scanning for the wrong lun on an initiator will generate this
error on the target but not generate an error on the initiator. And I
have seen installs, with a lot of initiators, automate the scanning of
such luns incorrectly deemed missing.
While this looks like a simple problem it can take days to get
access or the tcp traces to sort it out.
Within the same routine there is another pr_err for
TCM_WRITE_PROTECTED that I did not add the initiatorname to as I
thought this would leave a heavy footprint on the initiator. If you
believe this should be changed for consistency please let me know and
I will add this and change to nacl->initiatorname.
On Sat, May 16, 2020 at 9:50 AM Mike Christie <mchristi@redhat.com> wrote:
>
> On 5/13/20 11:01 PM, Lance Digby wrote:
> > The NON_EXISTENT_LUN error can be written without an error condition
> > on the initiator responsible. Adding the initiatorname to this message
> > will reduce the effort required to fix this when many initiators are
> > supported by a target.
> >
> > Signed-off-by: Lance Digby <lance.digby@gmail.com>
> > ---
> > drivers/target/target_core_device.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
> > index 4cee113..604dea0 100644
> > --- a/drivers/target/target_core_device.c
> > +++ b/drivers/target/target_core_device.c
> > @@ -100,9 +100,10 @@
> > */
> > if (unpacked_lun != 0) {
> > pr_err("TARGET_CORE[%s]: Detected NON_EXISTENT_LUN"
> > - " Access for 0x%08llx\n",
> > + " Access for 0x%08llx from %s\n",
> > se_cmd->se_tfo->fabric_name,
> > - unpacked_lun);
> > + unpacked_lun,
> > + se_sess->se_node_acl->initiatorname);
>
> You can do nacl->initiatorname.
>
> Do you also want add the name to the tmr case? It's probably not common,
> but the error message would be consistent.
>
> > return TCM_NON_EXISTENT_LUN;
> > }
> >
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH target] target: Add initiatorname to NON_EXISTENT_LUN error
2020-05-16 23:29 ` Lance Digby
@ 2020-05-17 19:16 ` Mike Christie
2020-05-18 1:02 ` Lance Digby
0 siblings, 1 reply; 5+ messages in thread
From: Mike Christie @ 2020-05-17 19:16 UTC (permalink / raw)
To: Lance Digby; +Cc: martin.petersen, linux-scsi, target-devel, linux-kernel
On 5/16/20 6:29 PM, Lance Digby wrote:
> Mike, Thanks for the review!
> The pr_err Detected NON_EXISTENT_LUN is the error messages issued
> for the TCM_NON_EXISTENT_LUN retcode so I believe they are the same.
> Simply scanning for the wrong lun on an initiator will generate this
> error on the target but not generate an error on the initiator. And I
> have seen installs, with a lot of initiators, automate the scanning of
> such luns incorrectly deemed missing.
> While this looks like a simple problem it can take days to get
> access or the tcp traces to sort it out.
>
> Within the same routine there is another pr_err for
> TCM_WRITE_PROTECTED that I did not add the initiatorname to as I
> thought this would leave a heavy footprint on the initiator. If you
I'm not sure what you mean by heavy footprint on the initiator part means.
I would say do whatever is helpful to you to debug the problem. For
TCM_WRITE_PROTECTED I'm not sure the initiatorname is helpful. I think
the target name and tpg would be useful, because I think you sometimes
set it at the tpg level then it gets inherited by the LU. But I think
it's a pain to get to the target name from this code path, so I wouldn't
worry about adding it now.
> believe this should be changed for consistency please let me know and
> I will add this and change to nacl->initiatorname.
Just to make sure we are on the same page. I was just commenting about
the other NON_EXISTENT_LUN instace in transport_lookup_tmr_lun. I just
thought we would want/need the same info there.
>
>
>
>
>
>
>
>
>
> On Sat, May 16, 2020 at 9:50 AM Mike Christie <mchristi@redhat.com> wrote:
>>
>> On 5/13/20 11:01 PM, Lance Digby wrote:
>>> The NON_EXISTENT_LUN error can be written without an error condition
>>> on the initiator responsible. Adding the initiatorname to this message
>>> will reduce the effort required to fix this when many initiators are
>>> supported by a target.
>>>
>>> Signed-off-by: Lance Digby <lance.digby@gmail.com>
>>> ---
>>> drivers/target/target_core_device.c | 5 +++--
>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
>>> index 4cee113..604dea0 100644
>>> --- a/drivers/target/target_core_device.c
>>> +++ b/drivers/target/target_core_device.c
>>> @@ -100,9 +100,10 @@
>>> */
>>> if (unpacked_lun != 0) {
>>> pr_err("TARGET_CORE[%s]: Detected NON_EXISTENT_LUN"
>>> - " Access for 0x%08llx\n",
>>> + " Access for 0x%08llx from %s\n",
>>> se_cmd->se_tfo->fabric_name,
>>> - unpacked_lun);
>>> + unpacked_lun,
>>> + se_sess->se_node_acl->initiatorname);
>>
>> You can do nacl->initiatorname.
>>
>> Do you also want add the name to the tmr case? It's probably not common,
>> but the error message would be consistent.
>>
>>> return TCM_NON_EXISTENT_LUN;
>>> }
>>>
>>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH target] target: Add initiatorname to NON_EXISTENT_LUN error
2020-05-17 19:16 ` Mike Christie
@ 2020-05-18 1:02 ` Lance Digby
0 siblings, 0 replies; 5+ messages in thread
From: Lance Digby @ 2020-05-18 1:02 UTC (permalink / raw)
To: Mike Christie; +Cc: martin.petersen, linux-scsi, target-devel, linux-kernel
Thanks again Mike will send out a new version.
On Mon, May 18, 2020 at 5:16 AM Mike Christie <mchristi@redhat.com> wrote:
>
> On 5/16/20 6:29 PM, Lance Digby wrote:
> > Mike, Thanks for the review!
> > The pr_err Detected NON_EXISTENT_LUN is the error messages issued
> > for the TCM_NON_EXISTENT_LUN retcode so I believe they are the same.
> > Simply scanning for the wrong lun on an initiator will generate this
> > error on the target but not generate an error on the initiator. And I
> > have seen installs, with a lot of initiators, automate the scanning of
> > such luns incorrectly deemed missing.
> > While this looks like a simple problem it can take days to get
> > access or the tcp traces to sort it out.
> >
> > Within the same routine there is another pr_err for
> > TCM_WRITE_PROTECTED that I did not add the initiatorname to as I
> > thought this would leave a heavy footprint on the initiator. If you
>
> I'm not sure what you mean by heavy footprint on the initiator part means.
>
> I would say do whatever is helpful to you to debug the problem. For
> TCM_WRITE_PROTECTED I'm not sure the initiatorname is helpful. I think
> the target name and tpg would be useful, because I think you sometimes
> set it at the tpg level then it gets inherited by the LU. But I think
> it's a pain to get to the target name from this code path, so I wouldn't
> worry about adding it now.
>
> > believe this should be changed for consistency please let me know and
> > I will add this and change to nacl->initiatorname.
>
> Just to make sure we are on the same page. I was just commenting about
> the other NON_EXISTENT_LUN instace in transport_lookup_tmr_lun. I just
> thought we would want/need the same info there.
>
>
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > On Sat, May 16, 2020 at 9:50 AM Mike Christie <mchristi@redhat.com> wrote:
> >>
> >> On 5/13/20 11:01 PM, Lance Digby wrote:
> >>> The NON_EXISTENT_LUN error can be written without an error condition
> >>> on the initiator responsible. Adding the initiatorname to this message
> >>> will reduce the effort required to fix this when many initiators are
> >>> supported by a target.
> >>>
> >>> Signed-off-by: Lance Digby <lance.digby@gmail.com>
> >>> ---
> >>> drivers/target/target_core_device.c | 5 +++--
> >>> 1 file changed, 3 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
> >>> index 4cee113..604dea0 100644
> >>> --- a/drivers/target/target_core_device.c
> >>> +++ b/drivers/target/target_core_device.c
> >>> @@ -100,9 +100,10 @@
> >>> */
> >>> if (unpacked_lun != 0) {
> >>> pr_err("TARGET_CORE[%s]: Detected NON_EXISTENT_LUN"
> >>> - " Access for 0x%08llx\n",
> >>> + " Access for 0x%08llx from %s\n",
> >>> se_cmd->se_tfo->fabric_name,
> >>> - unpacked_lun);
> >>> + unpacked_lun,
> >>> + se_sess->se_node_acl->initiatorname);
> >>
> >> You can do nacl->initiatorname.
> >>
> >> Do you also want add the name to the tmr case? It's probably not common,
> >> but the error message would be consistent.
> >>
> >>> return TCM_NON_EXISTENT_LUN;
> >>> }
> >>>
> >>
> >
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-05-18 1:02 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-14 4:01 [PATCH target] target: Add initiatorname to NON_EXISTENT_LUN error Lance Digby
2020-05-15 23:50 ` Mike Christie
2020-05-16 23:29 ` Lance Digby
2020-05-17 19:16 ` Mike Christie
2020-05-18 1:02 ` Lance Digby
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).