Re: subchannel bug or change??

Erik Andersen (andersee@debian.org)
Thu, 8 Jan 1998 01:29:24 -0700


On Wed, Jan 07, 1998 at 09:31:53AM -0700, scoffin@netcom.com wrote:
> Here is a simple program that seems to fail on kernel 2.1.78,
> but worked fine in kernels 2.1.67 and earlier (sorry, I do not
> know when it started to fail after 2.1.67).
>
> I have a PPro machine with no SMP, with a SCSI CDROM. Attached
> at the end of this message is my .config for 2.1.78. Am I doing
> something wrong, has the IOCTL interface changed, or is there a
> bug in 2.1.78 ???
>
> Thanks for your efforts in support of LINUX :-)
>
> =S.Coffin
> scoffin@netcom.com
>
>
> /***************************************************************/
> #include <stdio.h>
> #include <fcntl.h>
> #include <linux/cdrom.h>
> #include <linux/errno.h>
>
> static char cd_device[] = "/dev/cdrom";
> static int cdrom_fd = -1;
> extern int errno;
>
> main() {
> struct cdrom_subchnl subchnl;
> int xxx;
>
> if( (cdrom_fd = open( cd_device, O_RDONLY|O_NONBLOCK) ) == (-1) ) {
> if( errno == ENOMEDIUM || errno == EMEDIUMTYPE )
> fprintf( stderr, "\n No music CDROM in drive!\n\n" );
> else
> perror( "open cdrom: " );
> return( (-1) );
> }
>
>
> if( (xxx=ioctl( cdrom_fd, CDROMSUBCHNL, &subchnl )) == (-1) ) {
> fprintf( stderr, "ioctl(cdromsubchnl) [1]: " );
> perror( "cdromsubchnl:" );
> return( (-1) );
> }
> else fprintf( stderr, "XXX=%d\n", xxx );
> }
>
[---------------.config snipped-----------------]

Hi,

I am copying this to linux-kernel because I think this will be of
general interest. I hope you don't mind...

What you are seeing is a change that occured due to my major update
to the file drivers/cdrom/cdrom.c (the Uniform cdrom driver), in
particular, a change to the open_for_data function.

In my update to cdrom.c (and to all the cdrom drivers) per kernel
2.1.70, I make CDO_CHECK_TYPE a default config option (previously it
was _not_ a default because it requires that you use cd player programs
that respect the O_NONBLOCK option for opening the cdrom-device (See
Documentation/cdrom/cdrom-standard.tex as to why this is a good thing).
It was my intention to make this _policy_, because it is a good thing. I
went to the effort of helping the libc (both libc5 and glibc) folks add
proper support for EMEDIUMTYPE and ENOMEDIUM, and got the error messages
into kernel 2.0.33 as well. I knew it would break things, but since the
change to cd-player programs is so trivial, and because patches for
a number of cd players (CDplayer-2.0, cd-console-1.1, workbone-2.3,
cdtool-1.0, cdp-0.33, cdplayer-0.4, cdthing-1.4, lyn0.8a, playcd-1.0)
can be found at: ftp://ftp.gwdg.de/pub/linux/cdrom/drivers/cm206/ I was
willing to break a few things, because the result is a good thing. As
part of this change, I changed:

- if (cdi->options & CDO_CHECK_TYPE &&
- ds != CDS_DATA_1)
- return -EMEDIUMTYPE;
+ if (cdi->options & CDO_CHECK_TYPE && tracks.data==0) {
+ cdinfo(CD_OPEN, "bummer. wrong media type...\n");
+ ret=-EMEDIUMTYPE;
+ goto clean_up_and_return;

Which is functionally an equivalent piece of code.

This was then changed in 2.1.71 by Gerd Knorr
<kraxel@goldbach.isdn.cs.tu-berlin.de> because "it was just the fastest
way to fix the problem." and "becauce it breaks many existing CD players
and people complained about this." He changed it like this:

+#if 0
+ /* this breaks CD-Players which don't use O_NONBLOCK, workman
+ * for example, probably others too */
if (cdi->options & CDO_CHECK_TYPE && tracks.data==0) {
cdinfo(CD_OPEN, "bummer. wrong media type...\n");
ret=-EMEDIUMTYPE;
goto clean_up_and_return;
}
+#endif

After David A. van Leeuwen <david@ElseWare.cistron.nl> (the former
maintainer of cdrom.c) and I explained to Gerd that the purpose of
the CDROM_SET_OPTIONS ioctl is to allow people to disable things like
CDO_CHECK_TYPE, so there is no need to #ifdef out this code, it is only
necessary to have the user set their options to turn off CDO_CHECK_TYPE.
Gerd then said that if I back out my little policy change (i.e. no more
CDO_CHECK_TYPE by default) then he had no problem with removing the
#if 0. I was on vacation at the time, so David submitted a patch that
changed the default options of the Uniform cdrom driver as follows:

- cdi->options = CDO_AUTO_CLOSE | CDO_USE_FFLAGS | CDO_LOCK |
+CDO_CHECK_TYPE;
+ cdi->options = CDO_AUTO_CLOSE | CDO_USE_FFLAGS | CDO_LOCK;
+ /* default compatibility mode */

and also removed the #if 0 as follows:

-#if 0
- /* this breaks CD-Players which don't use O_NONBLOCK, workman
- * for example, probably others too */
+ /* CD-Players which don't use O_NONBLOCK, workman
+ * for example, need bit CDO_CHECK_TYPE cleared! */
if (cdi->options & CDO_CHECK_TYPE && tracks.data==0) {
cdinfo(CD_OPEN, "bummer. wrong media type...\n");
ret=-EMEDIUMTYPE;
goto clean_up_and_return;
}
-#endif

and he also added in a CDS_MIXED return for the CDROM_DISC_STATUS ioctl
just for good measure.

Anyway, all of this is to say that I expect that previous to kernels
2.1.67, I imagine you must have either compiled cdrom.c with #define
CHECKTYPE, or you called ioctl( fd, CDROM_SET_OPTIONS, CDO_CHECK_TYPE )
from your application to get the desired behavior. Unless I can convince
the world that this policy change is a good thing (it is), then for now,
you should write your application with something like what follows:

#include <stdio.h>
#include <fcntl.h>
#include <linux/cdrom.h>
#include <linux/errno.h>

static char cd_device[] = "/dev/cdrom";
static int cdrom_fd = -1;
extern int errno;

main() {
struct cdrom_subchnl subchnl;
int xxx;

+ /* Turn on medium-type checking before we open the drive */
+ ioctl( cdrom_fd, CDROM_SET_OPTIONS, CDO_CHECK_TYPE );
if( (cdrom_fd = open( cd_device, O_RDONLY|O_NONBLOCK) ) == (-1) ) {
if( errno == ENOMEDIUM || errno == EMEDIUMTYPE )
fprintf( stderr, "\n No music CDROM in drive!\n\n" );
else
perror( "open cdrom: " );
return( (-1) );
}
...
+ /* Turn off medium-type checking on exit so old cd players don't barf */
+ ioctl( cdrom_fd, CDROM_CLEAR_OPTIONS, CDO_CHECK_TYPE );
}

Doing things this way, the interface doesn't change for old cd players, but
via the CDROM_SET_OPTIONS ioctl, you can still get the desired behavior.
Sorry for writing a book here... I will probably take this and put it
into cdrom-standard.tex sometime.

I sincerely hope this helps,

-Erik

The Linux cdrom and ide-cd guy.

--
Erik B. Andersen   Web:    http://www.inconnect.com/~andersen/ 
                   email:  andersee@debian.org
--This message was written using 73% post-consumer electrons--