* Re: phantom empty cd
2010-04-15 1:54 phantom empty cd Mike Brudevold
@ 2010-04-15 7:10 ` Martin Pitt
2010-04-15 18:35 ` Kay Sievers
2010-04-15 23:34 ` Robby Workman
2 siblings, 0 replies; 4+ messages in thread
From: Martin Pitt @ 2010-04-15 7:10 UTC (permalink / raw)
To: linux-hotplug
[-- Attachment #1.1: Type: text/plain, Size: 2323 bytes --]
Hello Mike,
Mike Brudevold [2010-04-14 20:54 -0500]:
> if ((cmd->sg_io.info & SG_INFO_OK_MASK) != SG_INFO_OK) {
> errno = EIO;
> ret = -1;
> if (cmd->sg_io.masked_status & CHECK_CONDITION) {
> ret = ERRCODE(cmd->_sense.u);
> if (ret == 0)
> ret = -1;
> }
> }
>
> I do not have a copy of the spec, so I can only guess, but if
> CHECK_CONDITION is set, the function can only return error (-1) if
> cmd->_sense.u is zero (ERRCODE can never be negative).
This doesn't even depend on the spec, it's quite obvious from the
logic in cdrom_id itself. Good catch!
I'm not sure whether this will help to fix the problem for everyone
(see below), but it does look like a valid and obvious bug to me. I
have a potential patch for this attached, can you test it please? Kay,
does it make sense to you?
I also uploaded this to my personal package archive, so if you are on
Ubuntu you can test it easily (should be built within the next hour):
https://.launchpad.net/~pitti/+archive/ppa
> I experience the phantom empty cd problem, and when I return -1 from
> this function rather than checking for CHECK_CONDITION, the phantom
> CD problem goes away, as cdrom_id errors out trying to read the TOC
> and simply prints the results.
For the record, this is https://launchpad.net/bugs/562978; the recent
fixes to properly zero out data helped for most people, but not all;
some people still get output like
main: probing: '/dev/sr1'
cd_media_compat: CDROM_DRIVE_STATUS != CDS_DISC_OK
cd_inquiry: INQUIRY: [LITE-ON ][CD-RW SOHR-5238S][4S06]
cd_profiles: GET CONFIGURATION: number of profiles 184
cd_profiles: profile 0x0a
cd_profiles: current profile 0x02
cd_media_toc: READ TOC: len: 2
cd_media_info: disk type 00
ID_CDROM=1
ID_CDROM_CD_R=1
ID_CDROM_CD_RW=1
ID_CDROM_MRW=1
ID_CDROM_MRW_W=1
ID_CDROM_MEDIA=1
ID_CDROM_MEDIA_STATE=blank
The root problem here is that while the CDROM_DRIVE_STATUS ioctl
already said that there's no CD, we second-guess that in
cd_profiles(), and set "cd_media = 1;" if we have a non-zero current
profile. This seems a bit brittle to me?
Thanks,
Martin
--
Martin Pitt | http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
[-- Attachment #1.2: 0001-cdrom_id-Do-not-ignore-errors-from-scsi_cmd_run.patch --]
[-- Type: text/x-diff, Size: 3122 bytes --]
From 0b8bf46653a22b5befe4af0b035134260bdc24be Mon Sep 17 00:00:00 2001
From: Martin Pitt <martin.pitt@ubuntu.com>
Date: Thu, 15 Apr 2010 08:56:48 +0200
Subject: [PATCH] cdrom_id: Do not ignore errors from scsi_cmd_run()
scsi_cmd_run() can return positive error messages if we have CHECK_CONDITION
set and get the error code from the SCSI command result. So check the result
for non-zero, not for being negative.
This should fix another cause for "phantom" media in empty CD-ROM drives.
Thanks to Mike Brudevold <mike@brudevold.com> for spotting this!
https://launchpad.net/bugs/562978
---
| 14 +++++++-------
1 files changed, 7 insertions(+), 7 deletions(-)
--git a/extras/cdrom_id/cdrom_id.c b/extras/cdrom_id/cdrom_id.c
index afb481d..5119013 100644
--- a/extras/cdrom_id/cdrom_id.c
+++ b/extras/cdrom_id/cdrom_id.c
@@ -242,7 +242,7 @@ static int cd_inquiry(struct udev *udev, int fd) {
scsi_cmd_set(udev, &sc, 4, 36);
scsi_cmd_set(udev, &sc, 5, 0);
err = scsi_cmd_run(udev, &sc, fd, inq, 36);
- if ((err < 0)) {
+ if ((err != 0)) {
info_scsi_cmd_err(udev, "INQUIRY", err);
return -1;
}
@@ -272,7 +272,7 @@ static int cd_profiles(struct udev *udev, int fd)
scsi_cmd_set(udev, &sc, 8, sizeof(header));
scsi_cmd_set(udev, &sc, 9, 0);
err = scsi_cmd_run(udev, &sc, fd, header, sizeof(header));
- if ((err < 0)) {
+ if ((err != 0)) {
info_scsi_cmd_err(udev, "GET CONFIGURATION", err);
return -1;
}
@@ -292,7 +292,7 @@ static int cd_profiles(struct udev *udev, int fd)
scsi_cmd_set(udev, &sc, 8, len);
scsi_cmd_set(udev, &sc, 9, 0);
err = scsi_cmd_run(udev, &sc, fd, profiles, len);
- if ((err < 0)) {
+ if ((err != 0)) {
info_scsi_cmd_err(udev, "GET CONFIGURATION", err);
return -1;
}
@@ -448,7 +448,7 @@ static int cd_media_info(struct udev *udev, int fd)
scsi_cmd_set(udev, &sc, 8, sizeof(header));
scsi_cmd_set(udev, &sc, 9, 0);
err = scsi_cmd_run(udev, &sc, fd, header, sizeof(header));
- if ((err < 0)) {
+ if ((err != 0)) {
info_scsi_cmd_err(udev, "READ DISC INFORMATION", err);
return -1;
};
@@ -482,7 +482,7 @@ static int cd_media_toc(struct udev *udev, int fd)
scsi_cmd_set(udev, &sc, 8, sizeof(header));
scsi_cmd_set(udev, &sc, 9, 0);
err = scsi_cmd_run(udev, &sc, fd, header, sizeof(header));
- if ((err < 0)) {
+ if ((err != 0)) {
info_scsi_cmd_err(udev, "READ TOC", err);
return -1;
}
@@ -505,7 +505,7 @@ static int cd_media_toc(struct udev *udev, int fd)
scsi_cmd_set(udev, &sc, 8, len);
scsi_cmd_set(udev, &sc, 9, 0);
err = scsi_cmd_run(udev, &sc, fd, toc, len);
- if ((err < 0)) {
+ if ((err != 0)) {
info_scsi_cmd_err(udev, "READ TOC (tracks)", err);
return -1;
}
@@ -532,7 +532,7 @@ static int cd_media_toc(struct udev *udev, int fd)
scsi_cmd_set(udev, &sc, 8, 12);
scsi_cmd_set(udev, &sc, 9, 0);
err = scsi_cmd_run(udev, &sc, fd, header, sizeof(header));
- if ((err < 0)) {
+ if ((err != 0)) {
info_scsi_cmd_err(udev, "READ TOC (multi session)", err);
return -1;
}
--
1.7.0.4
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply related [flat|nested] 4+ messages in thread