cvs commit: src/sys/dev/ata atapi-cd.c atapi-cd.h

Søren Schmidt sos at deepcore.dk
Fri Nov 16 08:45:51 PST 2007


Scott Long wrote:
> SXren Schmidt wrote:
>> sos         2007-10-26 08:59:24 UTC
>>
>>   FreeBSD src repository
>>
>>   Modified files:
>>     sys/dev/ata          atapi-cd.c atapi-cd.h   Log:
>>   Update the way we get the mode pages on probe.
>>     Revision  Changes    Path
>>   1.194     +22 -25    src/sys/dev/ata/atapi-cd.c
>>   1.47      +1 -0      src/sys/dev/ata/atapi-cd.h
>
> Just curious, what was the motivation for changing from doing a
> TUR to doing a MODE_SENSE in g_access?  Your new code now relies
> on what I'd consider is a fairly obscure feature of the SFF-8020i
> spec, and one that contradicts the MMC and SPC specs that are now
> used as the normative references for packet commands.  There's at
> least once case, the virtual CDROM emulator in Parallels, that
> appears not to support this feature, and I'd bet pretty heavily
> that there are a number of real devices that won't support it
> either.
>
> Without reverting to the old TUR code, an easy way forward is to
> not fail the g_access command for the MST_FMT_NONE case.  This is
> generally incorrect anyways as it just means that the device can't
> specifically identify the media though it knows that the media is
> inserted and valid.  Since this constant evaluates to 0x00, ignoring
> it will also allow devices that don't support it to still work.  The
> only side effect is that devices that don't support it will also not
> be able to report MST_NO_DISC.  These devices will get handled later
> as a side effect of reporting a bogus media size.
>
> Ultimately, I do think that the code needs to go back to using a TUR
> and interpreting sense, asc, and ascq codes correctly.  The code prior
> to 10/26 looks like it does this, so again I'm curious as to what ,
> motivated the change.
The code is only intended to work around the verbose error chatter from 
GEOM as it will read from a nonexisting media (if the drive is empty) 
during boot, and clutter the console needlessly. There is currently no 
way to prevent this, but I've talked to phk about it lately so this can 
be left out alltogether.
Anyhow the only failure I've seen so far is Parallels when using a CD 
image as a virtual CDROM drive, it does a poor HW emu in that case if 
you ask me :)

Anyhow you are free to do what you want with the code until I get a real 
fix via GEOM done, but mind you the old code will not work on SATA ATAPI 
drives so a simple rollback of the commit is not really a fix...


-Søren



More information about the cvs-src mailing list