[PATCH] scsi: do not print 'reservation conflict' for TEST UNIT READY

[PATCH] scsi: do not print 'reservation conflict' for TEST UNIT READY

[Hannes Reinecke]

SPC-2 and SPC-3 (or later) differ in the handling of reservation
conflict for TEST UNIT READY. SPC-2 will return 'reservation conflict',
whereas SPC-3 will return GOOD status.
On a mixed system with both SPC-2 and SPC-3 targets one will
see lots of 'reservation conflict' messages from the SPC-2 system but
no messages from the SPC-3 system when eg multipath path checkers.
These messages might confuse the unsuspecting user although in fact
they just signal normal operation.
So we should not be printing out 'reservation conflict' for
TEST UNIT READY responses.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/scsi_error.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 106a6ad..3040fe5 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -1923,8 +1923,9 @@ int scsi_decide_disposition(struct scsi_cmnd *scmd)
 		return SUCCESS;
 
 	case RESERVATION_CONFLICT:
-		sdev_printk(KERN_INFO, scmd->device,
-			    "reservation conflict\n");
+		if (scmd->cmnd[0] != TEST_UNIT_READY)
+			sdev_printk(KERN_INFO, scmd->device,
+				    "reservation conflict\n");
 		set_host_byte(scmd, DID_NEXUS_FAILURE);
 		return SUCCESS; /* causes immediate i/o error */
 	default:
-- 
1.8.5.6

Re: [PATCH] scsi: do not print 'reservation conflict' for TEST UNIT READY

[Laurence Oberman]

----- Original Message -----
> From: "Hannes Reinecke" <hare@suse.de>
> To: "Martin K. Petersen" <martin.petersen@oracle.com>
> Cc: "Christoph Hellwig" <hch@lst.de>, "James Bottomley" <james.bottomley@hansenpartnership.com>,
> linux-scsi@vger.kernel.org, "Hannes Reinecke" <hare@suse.de>, "Hannes Reinecke" <hare@suse.com>
> Sent: Monday, September 12, 2016 4:20:53 AM
> Subject: [PATCH] scsi: do not print 'reservation conflict' for TEST UNIT READY
> 
> SPC-2 and SPC-3 (or later) differ in the handling of reservation
> conflict for TEST UNIT READY. SPC-2 will return 'reservation conflict',
> whereas SPC-3 will return GOOD status.
> On a mixed system with both SPC-2 and SPC-3 targets one will
> see lots of 'reservation conflict' messages from the SPC-2 system but
> no messages from the SPC-3 system when eg multipath path checkers.
> These messages might confuse the unsuspecting user although in fact
> they just signal normal operation.
> So we should not be printing out 'reservation conflict' for
> TEST UNIT READY responses.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
>  drivers/scsi/scsi_error.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> index 106a6ad..3040fe5 100644
> --- a/drivers/scsi/scsi_error.c
> +++ b/drivers/scsi/scsi_error.c
> @@ -1923,8 +1923,9 @@ int scsi_decide_disposition(struct scsi_cmnd *scmd)
>  		return SUCCESS;
>  
>  	case RESERVATION_CONFLICT:
> -		sdev_printk(KERN_INFO, scmd->device,
> -			    "reservation conflict\n");
> +		if (scmd->cmnd[0] != TEST_UNIT_READY)
> +			sdev_printk(KERN_INFO, scmd->device,
> +				    "reservation conflict\n");
>  		set_host_byte(scmd, DID_NEXUS_FAILURE);
>  		return SUCCESS; /* causes immediate i/o error */
>  	default:
> --
> 1.8.5.6
> 
> --
> 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
> 
Looks good to me.
Reviewed-by Laurence Oberman <loberman@redhat.com>

Re: [PATCH] scsi: do not print 'reservation conflict' for TEST UNIT READY

[James Bottomley]

On Mon, 2016-09-12 at 10:20 +0200, Hannes Reinecke wrote:
> SPC-2 and SPC-3 (or later) differ in the handling of reservation
> conflict for TEST UNIT READY. SPC-2 will return 'reservation 
> conflict', whereas SPC-3 will return GOOD status.
> On a mixed system with both SPC-2 and SPC-3 targets one will
> see lots of 'reservation conflict' messages from the SPC-2 system but
> no messages from the SPC-3 system when eg multipath path checkers.
> These messages might confuse the unsuspecting user although in fact
> they just signal normal operation.

I don't agree with this: a SCSI-2 device will not get properly
configured if it's reserved by something else, so you get other strange
artifacts of this condition.

> So we should not be printing out 'reservation conflict' for
> TEST UNIT READY responses.

This doesn't sound like a good rationale to me.  The way I see it, if
this message is actually useful, people would like to see it when they
get a reservation conflict.  That does mean even when SCSI-2
reservations give one where SCSI-3 would not.  The other reason is that
it tells you why your device didn't get configured properly: both Test
Unit Ready and Read Capacity get conflicts with SCSI-2, whereas they do
with SPC-3+ devices (trying to forget SPC-2).

You could argue that the entire message needs removing, since it's
reporting stuff that mostly only shows when systems using reservations
correctly are in operation.

James

Re: [PATCH] scsi: do not print 'reservation conflict' for TEST UNIT READY

[Hannes Reinecke]

On 09/13/2016 04:04 PM, James Bottomley wrote:
> On Mon, 2016-09-12 at 10:20 +0200, Hannes Reinecke wrote:
>> SPC-2 and SPC-3 (or later) differ in the handling of reservation
>> conflict for TEST UNIT READY. SPC-2 will return 'reservation 
>> conflict', whereas SPC-3 will return GOOD status.
>> On a mixed system with both SPC-2 and SPC-3 targets one will
>> see lots of 'reservation conflict' messages from the SPC-2 system but
>> no messages from the SPC-3 system when eg multipath path checkers.
>> These messages might confuse the unsuspecting user although in fact
>> they just signal normal operation.
> 
> I don't agree with this: a SCSI-2 device will not get properly
> configured if it's reserved by something else, so you get other strange
> artifacts of this condition.
> 
It's not about SCSI-2, it's for later arrays. See below.

>> So we should not be printing out 'reservation conflict' for
>> TEST UNIT READY responses.
> 
> This doesn't sound like a good rationale to me.  The way I see it, if
> this message is actually useful, people would like to see it when they
> get a reservation conflict.  That does mean even when SCSI-2
> reservations give one where SCSI-3 would not.  The other reason is that
> it tells you why your device didn't get configured properly: both Test
> Unit Ready and Read Capacity get conflicts with SCSI-2, whereas they do
> with SPC-3+ devices (trying to forget SPC-2).
> 
This specific issue occurred with a customer which had a mix of SPC-2
and SPC-3 arrays.
So I can't just 'forget' SPC-2 :-)
The system works just as designed, _except_ that one array is printing
this message (the SPC-2 one) whereas the other one doesn't.
So the message has zero value in this case.

> You could argue that the entire message needs removing, since it's
> reporting stuff that mostly only shows when systems using reservations
> correctly are in operation.
> 
Oh, I'm perfectly fine with that.
I'm happy to send a patch removing that line altogether.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [PATCH] scsi: do not print 'reservation conflict' for TEST UNIT READY

[James Bottomley]

On Tue, 2016-09-13 at 16:24 +0200, Hannes Reinecke wrote:
> On 09/13/2016 04:04 PM, James Bottomley wrote:
> > You could argue that the entire message needs removing, since it's
> > reporting stuff that mostly only shows when systems using 
> > reservations correctly are in operation.
> > 
> Oh, I'm perfectly fine with that.
> I'm happy to send a patch removing that line altogether.

OK, how about lowering the priority to KERN_DEBUG so it can still be
seen, just not usually.

James

Re: [PATCH] scsi: do not print 'reservation conflict' for TEST UNIT READY

[Ewan D. Milne]

On Tue, 2016-09-13 at 08:06 -0700, James Bottomley wrote:
> On Tue, 2016-09-13 at 16:24 +0200, Hannes Reinecke wrote:
> > On 09/13/2016 04:04 PM, James Bottomley wrote:
> > > You could argue that the entire message needs removing, since it's
> > > reporting stuff that mostly only shows when systems using 
> > > reservations correctly are in operation.
> > > 
> > Oh, I'm perfectly fine with that.
> > I'm happy to send a patch removing that line altogether.
> 
> OK, how about lowering the priority to KERN_DEBUG so it can still be
> seen, just not usually.
> 
> James
> 

So, if we do this, and someone else does a SCSI-2 RESERVE on the LUN
while we have a file system mounted, what error will we end up getting?
I would assume that we would still see the usual SCSI error message
with the CDB printed, and that the "reservation conflict" message was
supplemental?  Or does the message get suppressed?  I'll have to try
this...

-Ewan

Re: [PATCH] scsi: do not print 'reservation conflict' for TEST UNIT READY

[James Bottomley]

On Tue, 2016-09-13 at 15:05 -0400, Ewan D. Milne wrote:
> On Tue, 2016-09-13 at 08:06 -0700, James Bottomley wrote:
> > On Tue, 2016-09-13 at 16:24 +0200, Hannes Reinecke wrote:
> > > On 09/13/2016 04:04 PM, James Bottomley wrote:
> > > > You could argue that the entire message needs removing, since
> > > > it's
> > > > reporting stuff that mostly only shows when systems using 
> > > > reservations correctly are in operation.
> > > > 
> > > Oh, I'm perfectly fine with that.
> > > I'm happy to send a patch removing that line altogether.
> > 
> > OK, how about lowering the priority to KERN_DEBUG so it can still
> > be
> > seen, just not usually.
> > 
> > James
> > 
> 
> So, if we do this, and someone else does a SCSI-2 RESERVE on the LUN
> while we have a file system mounted, what error will we end up
> getting?
> I would assume that we would still see the usual SCSI error message
> with the CDB printed, and that the "reservation conflict" message was
> supplemental?  Or does the message get suppressed?  I'll have to try
> this...

It will print out the CDB and the return code, which should say Read
(or Write) [ACTION_FAIL in scsi_io_completion()].  However, it looks
like scsi_print_result() doesn't actually print the device byte, which
is where the reservation conflict is, so perhaps that should also be
fixed.

James