libcdio-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Libcdio-devel] Re: Recent libcdio breakage fixed I think. Please test.


From: Thomas Schmitt
Subject: [Libcdio-devel] Re: Recent libcdio breakage fixed I think. Please test.
Date: Tue, 09 Feb 2010 09:55:53 +0100

Hi,

there are pitfalls in
  lib/driver/mmc/mmc*.c

E.g. in function
  mmc_get_blocksize( ... )
  {
    uint8_t buf[255] = { 0, };
    ...
    i_status = mmc_mode_sense_10(p_cdio, buf, sizeof(buf),
                                 CDIO_MMC_R_W_ERROR_PAGE);

Note this important gesture in test/driver/mmc.c
with a command that has variable reply length
(by design or by MMC history):

  test_mode_sense(0x5, 0, 10) ... return= 0 , sense(0)
  test_mode_sense(0x5, 0, 60) ... return= 0 , sense(0)

First one has to inquire the reply length by a
command with minimal Allocation Length (here 10)
and only then one may request up to the replied
length (here 60).
This is not about MMC but about system bus
drivers. On Linux, some USB versions take offense
if one requests more bytes than available.

One should do this with any command that begins
its reply by telling the reply size. Those are
suspicious to be variable in length.
The minimal Allocation Length is the position
of the last byte of the reply length size field.
(See MMC command reply formats.)

I wondered for about half a year why growisofs
does this cumbersome double call. Then the first
bug report came in.
Some months later, cdrecord had to join our
club. :))

------------------------------------------------

Now for the tests:

  $ cd test/driver
  $ make mmc

complains about missing
  mmc_sense_key2str
  mmc_test_unit_ready

Adding them to lib/driver/libcdio.sym helps.

Empty drive:

  $ ./mmc
     INFO: ioctl CDROM_SEND_PACKET failed: Input/output error
     INFO: ioctl CDROM_SEND_PACKET failed: Input/output error
  Got status -1 back from get_disc_erasable(/dev/cdrom)
  $ echo $?
  0

Maybe one should suppress INFO messages in
non-verbous mode.
The same with the non-fatal message about
get_disc_erasable().

  $ ./mmc babble
      INFO: ioctl CDROM_SEND_PACKET failed: Input/output error
   Disc is not erasable.
      INFO: ioctl CDROM_SEND_PACKET failed: Input/output error
   Got status -1 back from get_disc_erasable(/dev/cdrom)
   test_disctype: profile is The Logical Unit does not conform to any Profile 
(0xFFFF)
   Drive '/dev/cdrom' has cdio_get_arg("scsi-tuple") = '4,4,0,0,0'
   Drive '/dev/cdrom' has cdio_get_arg("scsi-tuple") = '4,4,0,0,0'
   test_test_unit_ready ... return= -1 , sense(18):  KEY=Not Ready (2), ASC= 3A 
, ASCQ= 00
   test_mode_sense(0x3E, 0, 10) ... return= -1 , sense(18):  KEY=Illegal 
Request (5), ASC= 24 , ASCQ= 00
   test_test_unit_ready ... return= -1 , sense(18):  KEY=Not Ready (2), ASC= 3A 
, ASCQ= 00
   test_mode_sense(0x3E, 0, 10) ... return= -1 , sense(18):  KEY=Illegal 
Request (5), ASC= 24 , ASCQ= 00
   test_mode_sense(0x5, 0, 10) ... return= 0 , sense(0)
   test_mode_sense(0x5, 0, 60) ... return= 0 , sense(0)
   test_rwr_mode_page: Write type = 1 (TAO)
   test_mode_select to drive: 60 bytes
   00 00 00 00 00 00 00 00 05 32 62 04 08 00 00 00 00 00 00 00 
   00 00 00 96 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
   test_mode_select(0x5, 50, 60) ... return= 0 , sense(0)
   test_mode_sense(0x5, 0, 60) ... return= 0 , sense(0)
   test_mode_select to drive: 60 bytes
   00 3A 71 00 00 00 00 00 05 32 61 C4 08 00 00 00 00 00 00 00 
   00 00 00 96 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
   test_mode_select(0x5, 50, 60) ... return= 0 , sense(0)
   test_mode_sense(0x5, 0, 60) ... return= 0 , sense(0)
   test_rwr_mode_page: Mode page 2Ah restored to previous state
   test_test_unit_ready ... return= -1 , sense(18):  KEY=Not Ready (2), ASC= 3A 
, ASCQ= 00
   test_unit_ready: Note: No loaded media detected.

(Frank: Don't riddle here.
 Mode page 0x3E is intentionally illegal to
 provoke an error indicating sense reply.
 So this Illegal Request is a positive test
 result.)

I wonder about some double message lines.

Now with CD-RW loaded:

  $ ./mmc
  echo $?
  0

Fine.

  $ ./mmc v
  Disc is erasable.
  test_disctype: profile is CD-RW Re-writable Compact Disc capable (0xA)
  Drive '/dev/cdrom' has cdio_get_arg("scsi-tuple") = '4,4,0,0,0'
  Drive '/dev/cdrom' has cdio_get_arg("scsi-tuple") = '4,4,0,0,0'
  test_test_unit_ready ... return= 0 , sense(0)
  test_mode_sense(0x3E, 0, 10) ... return= -1 , sense(18):  KEY=Illegal Request 
(5), ASC= 24 , ASCQ= 00
  test_test_unit_ready ... return= 0 , sense(0)
  test_mode_sense(0x3E, 0, 10) ... return= -1 , sense(18):  KEY=Illegal Request 
(5), ASC= 24 , ASCQ= 00
  ...

But why this double view ?

Aha ... you moved
  cdio_get_arg(p_cdio, "scsi-tuple");
and its messages down to test_write() (former
tmmc_test()) but did not delete the old lines
in main().

And this gesture is twice in test_write():

  /* Test availability of sense reply in case of unready drive.
     E.g. if the tray is already ejected.
  */
  ret = test_test_unit_ready(p_cdio, &sense_avail, sense, !!verbose);
  if (ret != 0 && sense_avail < 18) {
    fprintf(stderr,
            "Error: Drive not ready. Only %d sense bytes. Expected >= 18.\n",
            sense_avail);
    {ret = 2; goto ex;}
  }

  /* Cause sense reply failure by requesting inappropriate mode page 3Eh */
  ret = test_mode_sense(p_cdio, &sense_avail, sense,
                       0x3e, 0, alloc_len, buf, &i_size, !!verbose);
  if (ret != 0 && sense_avail < 18) {
    fprintf(stderr,
            "Error: An illegal command yields only %d sense bytes. Expected >= 
18.\n",
            sense_avail);
    {ret = 2; goto ex;}
  } 

The comment text for 0x3e is misleading.
Mine was:
  /* Provoke sense reply by requesting inappropriate mode page 3Eh */
So it should rather be like:
  /* Cause sense reply with error key by requesting inappropriate mode page 3Eh 
*/

I see a compiler warning
  mmc.c:724: warning: unused variable 'scsi_tuple'
The line number is not original since i already
fiddled in the file for test purposes.

Ok. Now a verbous run on CD-RW looks like this:

  Disc is erasable.
  test_disctype: profile is CD-RW Re-writable Compact Disc capable (0xA)
  Drive '/dev/cdrom' has cdio_get_arg("scsi-tuple") = '4,4,0,0,0'
  test_test_unit_ready ... return= 0 , sense(0)
  test_mode_sense(0x3E, 0, 10) ... return= -1 , sense(18):  KEY=Illegal Request 
(5), ASC= 24 , ASCQ= 00
  test_mode_sense(0x5, 0, 10) ... return= 0 , sense(0)
  test_mode_sense(0x5, 0, 60) ... return= 0 , sense(0)
  test_rwr_mode_page: Write type = 1 (TAO)
  test_mode_select to drive: 60 bytes
  00 00 00 00 00 00 00 00 05 32 62 04 08 00 00 00 00 00 00 00 
  00 00 00 96 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
  test_mode_select(0x5, 50, 60) ... return= 0 , sense(0)
  test_mode_sense(0x5, 0, 60) ... return= 0 , sense(0)
  test_mode_select to drive: 60 bytes
  00 3A 24 00 00 00 00 00 05 32 61 C4 08 00 00 00 00 00 00 00 
  00 00 00 96 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
  test_mode_select(0x5, 50, 60) ... return= 0 , sense(0)
  test_mode_sense(0x5, 0, 60) ... return= 0 , sense(0)
  test_rwr_mode_page: Mode page 2Ah restored to previous state
  test_test_unit_ready ... return= 0 , sense(0)

Rocky, i'll send you a copy of the current state
in a private mail.


Have a nice day :)

Thomas





reply via email to

[Prev in Thread] Current Thread [Next in Thread]