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

[Thomas Schmitt]

Hi,

Rocky Bernstein wrote:
> Leave mmc_read_subchannel() as is.
> Again it does a well-defined MMC function AND JUST THAT.

But in the stack of functions to retrieve MCN and ISRC it is the
highest one which knows the SCSI command.

Handling of the sense data reply is part of SCSI command transaction.
libcdio relies on the fact that ioctl(CDROM_SEND_PACKET) contains
an own interpreter for sense data which may throw a UNIX error.
ioctl(SG_IO) or the transport mechanisms of non-Linux systems do not
indicate drive protests by bad return values and errno.

Nevertheless it is beneficial to do the handling where the best
knowledge about the intention of the SCSI command is present.
I would put it into mmc_get_mcn_isrc_private() which would not have
to interpret the SCSI command to know what it is supposed to do.

Therefore my two other alternative proposals to not only record
the sense data reply of the last SCSI command but also the command
itself (i.e. mmc_cdb_t).

This idea raises the question what instance shall record mmc_cdb_t.
My alternatives are a bottleneck macro or putting the plight onto
the various implementations of p_cdio->op.run_mmc_cmd().
Those implementations already record the returned sense data.


> This new thing that has the parsed sense data and better error reporting,
> this is a mmc_hl thing. Especially if it issues more than one MMC command.

I don't think so. The sense data handling is bound to single SCSI
commands. So mmc_hl is not the layer where this should be done.

The current situation is that Linux ioctl(CDROM_SEND_PACKET) does
the job of sense data interpretation. Surely at least two levels of
abstraction too deep. Neither the ioctl nor p_cdio->op.run_mmc_cmd()
really know what an SCSI command shall do. So they cannot really
judge the impact of problems on the intended side effect.


> I feel like I've repeated this a couple of times:

I cannot fit your wish to interpret sense data in mmc_hl with what
i perceive as the model of MMC command execution in libcdio.

Of course you are free to define an architecture to represent MMC.
But this architecture has to match the way how SCSI transactions
are used to perform the tasks of libcdio.


> I am not sure what you mean by:
> > This prevents my plan to let error indicating sense data act like
> > missing MCVAL/TCVAL bit. These bits are known only in
> > mmc_get_mcn_isrc_private().

mmc_get_mcn_isrc_private() knows that it shall retrieve MCN or ISRC.
Both have the Valid bit at the same position in the reply data.

The function  mmc_read_subchannel()  implements MMC command
READ SUB-CHANNEL, which does not only MCN or ISRC but also can retrieve
other info, which has no such Valid bit.

So if i want to treat refusal by sense data the same as refusal by
unset Valid bit, then i have to do this in  mmc_get_mcn_isrc_private().
In mmc_read_subchannel() i can only indicate a transport failure.


> >   ++ WARN: READ_SUBCHANNEL (CDB: 42 00 40 03 00 00 04 00 04 00 )
> >   ++ WARN: [5 24 00] : Illegal Request, Invalid field in cdb

> Looks great!

The first of these lines causes the need for knowing the mmc_cdb_t.

The tight association of both lines causes the need to evaluate and
report sense data after each single SCSI command and not after each
mmc_hl gesture.

Leaving out any of these informations (or disassociating them) would
hamper our ability to diagnose problem reports.


> Did you contact address@hidden ?

I have now asked for a snail mail address where to send my personal info
for getting the form. The initial curiosity of FSF in
  
http://git.savannah.gnu.org/cgit/gnulib.git/tree/doc/Copyright/request-assign.future
exceeds my willingness to tell by e-mail.


Have a nice day :)

Thomas