Re: [Libcdio-devel] Read Sub-channel changes

[Thomas Schmitt]

Hi,

when testing the sense code evaluator and its messages i got to a
problem of layering and the encapsulated knowledge about the performed
SCSI command (mmc_cdb_t).
I see the following alternatives:

- Move sense code evaluation and reporting to functions which know
  the mmc_cdb_t. This knowledge is currently quite low-level and very
  volatile.

- Bottleneck all calls to p_cdio->op.run_mmc_cmd() through MMC_RUN_CMD()
  which would then record the mmc_cdb_t before the command gets
  executed.

- Put responsibility for recording mmc_cdb_t on each implementation
  of p_cdio->op.run_mmc_cmd(). This is already done for sense data,
  because they have to be copied from driver specific memory to
  generic_img_private_t.

--------------------------------------------------------------------
Reasoning:

The sense code messages are incomplete if they are not accompanied
by info about the SCSI command which got them as reply.
But the functions which could best judge the impact of sense codes
are too high in the layer stack to know the SCSI command CDBs.

So additionally to the last received sense data, generic_img_private_t
should get a buffer to record the SCSI command which caused the buffered
sense data.

    typedef struct {
      ...
      mmc_cdb_t scsi_mmc_cmd;
      int scsi_mmc_cmd_len;
      ...
    } generic_img_private_t;

These new struct members could be filled by  MMC_RUN_CMD()  which knows
them as cdb and mmc_get_cmd_len(cdb.field[0]).

Regrettably not all SCSI command transactions are performed via
MMC_RUN_CMD(). There are direct uses of p_cdio->op.run_mmc_cmd()
and even function pointers handed down to other functions.

How many implementations of SCSI command transport do we have ?

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

The problem is partly hidden by run_mmc_cmd_linux(), because with severe
sense codes, the ioctl throws an error and run_mmc_cmd_linux() reports

   INFO: ioctl CDROM_SEND_PACKET for command READ_SUBCHANNEL (0x42) failed:
        Input/output error

before mmc_get_mcn_isrc_private() can call mmc_evaluate_last_sense()
which then says

   INFO: [5 24 00] : Illegal Request, Invalid field in cdb

I am quite sure that ioctl(SG_IO) would not throw a Unix error in
the same situation. We cannot rely on the SCSI command name to be yelled
by the SCSI transport layer when interesting sense data arrive.

ioctl(CDROM_SEND_PACKET) will not throw an error command name if a harmless
sense code is encountered. But we still might want to report it. Possibly
via cdio_error().

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

Have a nice day :)

Thomas