linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Tejun Heo <tj@kernel.org>
Cc: jeff@garzik.org, linux-ide@vger.kernel.org, axboe@kernel.dk,
	linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org,
	kay.sievers@vrfy.org, jack@suse.cz,
	James.Bottomley@HansenPartnership.com,
	Rolf Eike Beer <eike-kernel@sf-tec.de>
Subject: [PATCH UPDATED 5/8] scsi: fix TUR error handling in sr_media_change()
Date: Thu, 09 Dec 2010 11:18:42 +0100	[thread overview]
Message-ID: <4D00AD02.8080504@kernel.org> (raw)
In-Reply-To: <1291838262-21274-6-git-send-email-tj@kernel.org>

sr_test_unit_ready() returns 0 iff TUR succeeded - IOW, when media is
present and the device is actually ready, so the return value wouldn't
be zero when TUR ends with sense data. sr_media_change() incorrectly
tests (retval || (scsi_sense_valid(sshdr)...)) when it tries to test
whether TUR failed without sense data or with sense data indicating
media-not-present.

Fix the test using scsi_status_is_good() and update comments.

- Fixed a comment typo spotted by Eike.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Rolf Eike Beer <eike-kernel@sf-tec.de>
---
Typo fixed.  git tree updated accordingly.  Thanks.

 drivers/scsi/sr.c |   18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

Index: work/drivers/scsi/sr.c
===================================================================
--- work.orig/drivers/scsi/sr.c
+++ work/drivers/scsi/sr.c
@@ -214,13 +214,17 @@ static int sr_media_change(struct cdrom_

 	sshdr =  kzalloc(sizeof(*sshdr), GFP_KERNEL);
 	retval = sr_test_unit_ready(cd->device, sshdr);
-	if (retval || (scsi_sense_valid(sshdr) &&
-		       /* 0x3a is medium not present */
-		       sshdr->asc == 0x3a)) {
-		/* Media not present or unable to test, unit probably not
-		 * ready. This usually means there is no disc in the drive.
-		 * Mark as changed, and we will figure it out later once
-		 * the drive is available again.
+	/*
+	 * Media is considered to be present if TUR succeeds or fails with
+	 * sense data indicating something other than media-not-present
+	 * (ASC 0x3a).
+	 */
+	if (!scsi_status_is_good(retval) &&
+	    (!scsi_sense_valid(sshdr) || sshdr->asc == 0x3a)) {
+		/*
+		 * Probably no media in the device.  Mark as changed, and
+		 * we will figure it out later once the drive is available
+		 * again.
 		 */
 		cd->device->changed = 1;
 		/* This will force a flush, if called from check_disk_change */

  parent reply	other threads:[~2010-12-09 10:19 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-12-08 19:57 [PATCHSET] block/SCSI: implement in-kernel disk event handling, take#2 Tejun Heo
2010-12-08 19:57 ` [PATCH 1/8] block: kill genhd_media_change_notify() Tejun Heo
2010-12-08 19:57 ` [PATCH 2/8] block: move register_disk() and del_gendisk() to block/genhd.c Tejun Heo
2010-12-08 19:57 ` [PATCH 3/8] implement in-kernel gendisk events handling Tejun Heo
2010-12-08 19:57 ` [PATCH 4/8] cdrom: add ->check_events() support Tejun Heo
2010-12-08 19:57 ` [PATCH 5/8] scsi: fix TUR error handling in sr_media_change() Tejun Heo
2010-12-08 20:14   ` Rolf Eike Beer
2010-12-09 10:18   ` Tejun Heo [this message]
2010-12-09 18:20   ` Sergei Shtylyov
2010-12-09 18:53     ` Tejun Heo
2010-12-08 19:57 ` [PATCH 6/8] scsi: replace sr_test_unit_ready() with scsi_test_unit_ready() Tejun Heo
2010-12-08 19:57 ` [PATCH 7/8] sr: implement sr_check_events() Tejun Heo
2011-01-30  1:26   ` Simon Arlott
2011-01-30  1:31     ` [PATCH] cdrom: support devices that have check_events but not media_changed Simon Arlott
2011-01-31 10:12       ` Tejun Heo
2011-01-31 18:26         ` [PATCH (v2)] " Simon Arlott
2011-01-31 11:22       ` [PATCH] " Sergei Shtylyov
2010-12-08 19:57 ` [PATCH 8/8] sd: implement sd_check_events() Tejun Heo
2010-12-16 16:31 ` [PATCHSET] block/SCSI: implement in-kernel disk event handling, take#2 Tejun Heo
2010-12-16 16:36   ` Jens Axboe
2010-12-16 16:38     ` James Bottomley
2010-12-16 16:44       ` Kay Sievers
2010-12-16 16:41     ` Kay Sievers
2010-12-16 16:43       ` Jens Axboe
2010-12-16 16:45         ` Tejun Heo
2010-12-16 17:00           ` Jens Axboe
2010-12-16 18:11             ` Tejun Heo
2010-12-16 16:55         ` Jens Axboe
2010-12-16 17:00   ` Christoph Hellwig
2010-12-16 18:04     ` Tejun Heo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4D00AD02.8080504@kernel.org \
    --to=tj@kernel.org \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=axboe@kernel.dk \
    --cc=eike-kernel@sf-tec.de \
    --cc=jack@suse.cz \
    --cc=jeff@garzik.org \
    --cc=kay.sievers@vrfy.org \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).