Date: Mon, 26 Aug 2002 22:16:42 -0600 From: "Kenneth D. Merry" <ken@kdm.org> To: Bruce Evans <bde@zeta.org.au> Cc: "M. Warner Losh" <imp@bsdimp.com>, johan@FreeBSD.ORG, nsayer@quack.kfu.com, freebsd-scsi@FreeBSD.ORG, freebsd-standards@FreeBSD.ORG, sos@FreeBSD.ORG Subject: Re: kern/15608: acd0 / cd0 give inconsistent errors on empty tray open() Message-ID: <20020826221642.A39886@panzer.kdm.org> In-Reply-To: <20020825164629.H14756-100000@gamplex.bde.org>; from bde@zeta.org.au on Sun, Aug 25, 2002 at 05:07:32PM %2B1000 References: <20020822223203.A13222@panzer.kdm.org> <20020825164629.H14756-100000@gamplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--qMm9M+Fa2AknHoGS
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
[ freebsd-gnats-submit taken out for now, so we don't clutter up the PR
database too much with in-progress patches ]
On Sun, Aug 25, 2002 at 17:07:32 +1000, Bruce Evans wrote:
> On Thu, 22 Aug 2002, Kenneth D. Merry wrote:
> > On Fri, Aug 23, 2002 at 12:41:28 +1000, Bruce Evans wrote:
> > > I think the bug is that the open doesn't succeed. The device is
> > > reported as being there at boot time, and there is enough of it there
> > > to tell which parts of it aren't there, so why not open() it so that
> > > you do things like ioctl() on it to close its door and make it there?
> >
> > There is no ioctl in the interface for loading a CD. None of the other
> > ioctls, I think, make much sense without media in the drive.
>
> There is a CDIOCCLOSE which seems to be supported by acd and by some
> unmaintained cdrom drivers by not by the scsi cdrom driver.
Ahh, I missed that.
> > The reason we need to do a read capacity in the open() routine, which is
> > why the open fails when there is no media, is so we can fill in the
> > d_secsize and d_secperunit fields in the disklabel.
>
> acdopen() calls acd_read_toc() which does similar things. When there is
> no media, acd_read_toc() fails with the not quite right error EBUSY, but
> acdopen() ignores this error and the open succeeds and you can try
> CDIOCCLOSE() to attempt to load media. So it seems that acdopen() already
> works like I want, and the EIO error reported earlier doesn't actually
> occur for acdopen().
Well, here is a first pass at it. Let me know what you think.
> dsopen() has similar issues. It attempts to read MBRs and disk labels
> and can probably return EIO for read errors when there is no media or
> bad media. One reason why the fd driver doesn't use the slice layer
> is that I never got this working well enough for floppies. It is hard
> to issue formatting ioctls when the open fails because the MBR is
> unreadable.
FWIW, if the slice code is setup to attempt to read labels off the disk, but
there is no media in the drive (i.e. sector size and sectors per unit are
0, or it could be because p_size for the first partition is 0), it panics.
(DSO_COMPATLABEL is set, and DSO_NOLABELS is cleared)
If the slice code is setup not to attempt to read labels off the disk, it
doesn't panic at least.
Behavior of things like dd(1) is different, obviously, because it no longer
fails in open() with an empty drive. e.g., new behavior is:
================
{nargothrond:/usr/home/ken:2:0} dd if=/dev/cd0c of=/dev/null bs=2k
dd: /dev/cd0c: Device not configured
0+0 records in
0+0 records out
0 bytes transferred in 0.004522 secs (0 bytes/sec)
================
current behavior is:
================
{gondolin:/usr/home/ken:6:1} dd if=/dev/cd0c of=/dev/null bs=2k
dd: /dev/cd0c: Device not configured
================
cdcontrol seems to get a better idea of what is going on when open() fails:
================
{gondolin:/usr/home/ken:7:1} cdcontrol
cdcontrol: no CD device name specified, defaulting to /dev/cd0c
Compact Disc Control utility, version 2.0
Type `?' for command list
cdcontrol> info
cdcontrol: no disc in drive /dev/cd0c
cdcontrol> quit
================
When the ioctl fails instead of the open, it isn't quite as clear about
what is going on:
================
{nargothrond:/usr/home/ken:3:1} cdcontrol
cdcontrol: no CD device name specified, defaulting to /dev/cd0c
Compact Disc Control utility, version 2.0
Type `?' for command list
cdcontrol> info
cdcontrol: getting toc header: Device not configured
cdcontrol: Device not configured
cdcontrol> quit
================
Ken
--
Kenneth Merry
ken@kdm.org
--qMm9M+Fa2AknHoGS
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="scsi_cd.c.20020826"
==== //depot/FreeBSD-ken/src/sys/cam/scsi/scsi_cd.c#23 - /usr/home/ken/perforce/FreeBSD-ken/src/sys/cam/scsi/scsi_cd.c ====
--- /tmp/tmp.15286.0 Mon Aug 26 21:59:38 2002
+++ /usr/home/ken/perforce/FreeBSD-ken/src/sys/cam/scsi/scsi_cd.c Mon Aug 26 21:56:21 2002
@@ -1,6 +1,6 @@
/*
* Copyright (c) 1997 Justin T. Gibbs.
- * Copyright (c) 1997, 1998, 1999, 2000, 2001 Kenneth D. Merry.
+ * Copyright (c) 1997, 1998, 1999, 2000, 2001, 2002 Kenneth D. Merry.
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -94,7 +94,8 @@
CD_FLAG_CHANGER = 0x040,
CD_FLAG_ACTIVE = 0x080,
CD_FLAG_SCHED_ON_COMP = 0x100,
- CD_FLAG_RETRY_UA = 0x200
+ CD_FLAG_RETRY_UA = 0x200,
+ CD_FLAG_VALID_MEDIA = 0x400
} cd_flags;
typedef enum {
@@ -204,7 +205,8 @@
static int cderror(union ccb *ccb, u_int32_t cam_flags,
u_int32_t sense_flags);
static void cdprevent(struct cam_periph *periph, int action);
-static int cdsize(dev_t dev, u_int32_t *size);
+static int cdcheckmedia(struct cam_periph *periph);
+static int cdsize(struct cam_periph *periph, u_int32_t *size);
static int cdfirsttrackisdata(struct cam_periph *periph);
static int cdreadtoc(struct cam_periph *periph, u_int32_t mode,
u_int32_t start, struct cd_toc_entry *data,
@@ -229,7 +231,7 @@
u_int32_t etrack, u_int32_t eindex);
static int cdpause(struct cam_periph *periph, u_int32_t go);
static int cdstopunit(struct cam_periph *periph, u_int32_t eject);
-static int cdstartunit(struct cam_periph *periph);
+static int cdstartunit(struct cam_periph *periph, int load);
static int cdreportkey(struct cam_periph *periph,
struct dvd_authinfo *authinfo);
static int cdsendkey(struct cam_periph *periph,
@@ -858,11 +860,8 @@
static int
cdopen(dev_t dev, int flags, int fmt, struct thread *td)
{
- struct disklabel *label;
struct cam_periph *periph;
struct cd_softc *softc;
- struct ccb_getdev cgd;
- u_int32_t size;
int error;
int s;
@@ -891,72 +890,12 @@
if (cam_periph_acquire(periph) != CAM_REQ_CMP)
return(ENXIO);
- cdprevent(periph, PR_PREVENT);
-
- /* find out the size */
- if ((error = cdsize(dev, &size)) != 0) {
- cdprevent(periph, PR_ALLOW);
- cam_periph_unlock(periph);
- cam_periph_release(periph);
- return(error);
- }
-
- /*
- * If we get a non-zero return, revert back to not reading the
- * label off the disk. The first track is likely audio, which
- * won't have a disklabel.
- */
- if ((error = cdfirsttrackisdata(periph)) != 0) {
- softc->disk.d_dsflags &= ~DSO_COMPATLABEL;
- softc->disk.d_dsflags |= DSO_NOLABELS;
- error = 0;
- }
-
- /*
- * Build prototype label for whole disk.
- * Should take information about different data tracks from the
- * TOC and put it in the partition table.
- */
- label = &softc->disk.d_label;
- bzero(label, sizeof(*label));
- label->d_type = DTYPE_SCSI;
-
- /*
- * Grab the inquiry data to get the vendor and product names.
- * Put them in the typename and packname for the label.
- */
- xpt_setup_ccb(&cgd.ccb_h, periph->path, /*priority*/ 1);
- cgd.ccb_h.func_code = XPT_GDEV_TYPE;
- xpt_action((union ccb *)&cgd);
-
- strncpy(label->d_typename, cgd.inq_data.vendor,
- min(SID_VENDOR_SIZE, sizeof(label->d_typename)));
- strncpy(label->d_packname, cgd.inq_data.product,
- min(SID_PRODUCT_SIZE, sizeof(label->d_packname)));
-
- label->d_secsize = softc->params.blksize;
- label->d_secperunit = softc->params.disksize;
- label->d_flags = D_REMOVABLE;
- /*
- * Make partition 'a' cover the whole disk. This is a temporary
- * compatibility hack. The 'a' partition should not exist, so
- * the slice code won't create it. The slice code will make
- * partition (RAW_PART + 'a') cover the whole disk and fill in
- * some more defaults.
- */
- label->d_partitions[0].p_size = label->d_secperunit;
- label->d_partitions[0].p_fstype = FS_OTHER;
-
/*
- * We unconditionally (re)set the blocksize each time the
- * CD device is opened. This is because the CD can change,
- * and therefore the blocksize might change.
- * XXX problems here if some slice or partition is still
- * open with the old size?
+ * Check for media, and set the appropriate flags. We don't bail
+ * if we don't have media, but then we don't allow anything but the
+ * CDIOCEJECT/CDIOCCLOSE ioctls if there is no media.
*/
- if ((softc->device_stats.flags & DEVSTAT_BS_UNAVAILABLE) != 0)
- softc->device_stats.flags &= ~DEVSTAT_BS_UNAVAILABLE;
- softc->device_stats.block_size = softc->params.blksize;
+ cdcheckmedia(periph);
cam_periph_unlock(periph);
@@ -1373,6 +1312,21 @@
}
/*
+ * If we don't have valid media, look for it before trying to
+ * schedule the I/O.
+ */
+ if ((softc->flags & CD_FLAG_VALID_MEDIA) == 0) {
+ int error;
+
+ error = cdcheckmedia(periph);
+ if (error != 0) {
+ splx(s);
+ biofinish(bp, NULL, error);
+ return;
+ }
+ }
+
+ /*
* Place it in the queue of disk activities for this disk
*/
bioqdisksort(&softc->bio_queue, bp);
@@ -1804,6 +1758,20 @@
if (error != 0)
return(error);
+ /*
+ * If we don't have media loaded, check for it. If still don't
+ * have media loaded, we can only do a load or eject.
+ */
+ if (((softc->flags & CD_FLAG_VALID_MEDIA) == 0)
+ && ((cmd != CDIOCCLOSE)
+ && (cmd != CDIOCEJECT))) {
+ error = cdcheckmedia(periph);
+ if (error != 0) {
+ cam_periph_unlock(periph);
+ return (error);
+ }
+ }
+
switch (cmd) {
case CDIOCPLAYTRACKS:
@@ -2381,7 +2349,10 @@
error = cdpause(periph, 0);
break;
case CDIOCSTART:
- error = cdstartunit(periph);
+ error = cdstartunit(periph, 0);
+ break;
+ case CDIOCCLOSE:
+ error = cdstartunit(periph, 1);
break;
case CDIOCSTOP:
error = cdstopunit(periph, 0);
@@ -2482,18 +2453,103 @@
}
static int
-cdsize(dev_t dev, u_int32_t *size)
+cdcheckmedia(struct cam_periph *periph)
+{
+ struct disklabel *label;
+ struct cd_softc *softc;
+ struct ccb_getdev cgd;
+ u_int32_t size;
+ int error;
+
+ softc = (struct cd_softc *)periph->softc;
+
+ cdprevent(periph, PR_PREVENT);
+
+ /*
+ * Get the disc size and block size. If we can't get it, we don't
+ * have media, most likely. So we clear the valid media flag and
+ * make sure we're setup to use fake labels.
+ */
+ if ((error = cdsize(periph, &size)) != 0) {
+ softc->flags &= ~CD_FLAG_VALID_MEDIA;
+ softc->disk.d_dsflags &= ~DSO_COMPATLABEL;
+ softc->disk.d_dsflags |= DSO_NOLABELS;
+ cdprevent(periph, PR_ALLOW);
+ return (error);
+ } else
+ softc->flags |= CD_FLAG_VALID_MEDIA;
+
+ /*
+ * If we get a non-zero return, revert back to not reading the
+ * label off the disk. The first track is likely audio, which
+ * won't have a disklabel.
+ */
+ if ((error = cdfirsttrackisdata(periph)) != 0) {
+ softc->disk.d_dsflags &= ~DSO_COMPATLABEL;
+ softc->disk.d_dsflags |= DSO_NOLABELS;
+ error = 0;
+ } else {
+ softc->disk.d_dsflags |= DSO_COMPATLABEL;
+ softc->disk.d_dsflags &= ~DSO_NOLABELS;
+ }
+
+ /*
+ * Build prototype label for whole disk.
+ * Should take information about different data tracks from the
+ * TOC and put it in the partition table.
+ */
+ label = &softc->disk.d_label;
+ bzero(label, sizeof(*label));
+ label->d_type = DTYPE_SCSI;
+
+ /*
+ * Grab the inquiry data to get the vendor and product names.
+ * Put them in the typename and packname for the label.
+ */
+ xpt_setup_ccb(&cgd.ccb_h, periph->path, /*priority*/ 1);
+ cgd.ccb_h.func_code = XPT_GDEV_TYPE;
+ xpt_action((union ccb *)&cgd);
+
+ strncpy(label->d_typename, cgd.inq_data.vendor,
+ min(SID_VENDOR_SIZE, sizeof(label->d_typename)));
+ strncpy(label->d_packname, cgd.inq_data.product,
+ min(SID_PRODUCT_SIZE, sizeof(label->d_packname)));
+
+ label->d_secsize = softc->params.blksize;
+ label->d_secperunit = softc->params.disksize;
+ label->d_flags = D_REMOVABLE;
+ /*
+ * Make partition 'a' cover the whole disk. This is a temporary
+ * compatibility hack. The 'a' partition should not exist, so
+ * the slice code won't create it. The slice code will make
+ * partition (RAW_PART + 'a') cover the whole disk and fill in
+ * some more defaults.
+ */
+ label->d_partitions[0].p_size = label->d_secperunit;
+ label->d_partitions[0].p_fstype = FS_OTHER;
+
+ /*
+ * We unconditionally (re)set the blocksize each time the
+ * CD device is opened. This is because the CD can change,
+ * and therefore the blocksize might change.
+ * XXX problems here if some slice or partition is still
+ * open with the old size?
+ */
+ if ((softc->device_stats.flags & DEVSTAT_BS_UNAVAILABLE) != 0)
+ softc->device_stats.flags &= ~DEVSTAT_BS_UNAVAILABLE;
+ softc->device_stats.block_size = softc->params.blksize;
+
+ return (error);
+}
+
+static int
+cdsize(struct cam_periph *periph, u_int32_t *size)
{
- struct cam_periph *periph;
struct cd_softc *softc;
union ccb *ccb;
struct scsi_read_capacity_data *rcap_buf;
int error;
- periph = (struct cam_periph *)dev->si_drv1;
- if (periph == NULL)
- return (ENXIO);
-
CAM_DEBUG(periph->path, CAM_DEBUG_TRACE, ("entering cdsize\n"));
softc = (struct cd_softc *)periph->softc;
@@ -2999,7 +3055,7 @@
}
static int
-cdstartunit(struct cam_periph *periph)
+cdstartunit(struct cam_periph *periph, int load)
{
union ccb *ccb;
int error;
@@ -3013,7 +3069,7 @@
/* cbfcnp */ cddone,
/* tag_action */ MSG_SIMPLE_Q_TAG,
/* start */ TRUE,
- /* load_eject */ FALSE,
+ /* load_eject */ load,
/* immediate */ FALSE,
/* sense_len */ SSD_FULL_SIZE,
/* timeout */ 50000);
--qMm9M+Fa2AknHoGS--
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-standards" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020826221642.A39886>