Date: Fri, 16 Nov 2007 17:56:58 +0100 From: =?ISO-8859-1?Q?S=F8ren_Schmidt?= <sos@deepcore.dk> To: Nate Lawson <nate@root.org> Cc: cvs-src@FreeBSD.ORG, Scott Long <scottl@samsco.org>, src-committers@FreeBSD.ORG, cvs-all@FreeBSD.ORG Subject: Re: cvs commit: src/sys/dev/ata atapi-cd.c atapi-cd.h Message-ID: <473DCBDA.1080104@deepcore.dk> In-Reply-To: <473DCAA7.3090408@root.org> References: <200710260859.l9Q8xPdP099307@repoman.freebsd.org> <473DC220.6030809@samsco.org> <473DC93D.5060205@deepcore.dk> <473DCAA7.3090408@root.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Nate Lawson wrote: > S=F8ren Schmidt wrote: > =20 >> Scott Long wrote: >> =20 >>> SXren Schmidt wrote: >>> =20 >>>> 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 >>>> =20 >>> 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 prio= r >>> to 10/26 looks like it does this, so again I'm curious as to what , >>> motivated the change. >>> =20 >> The code is only intended to work around the verbose error chatter fro= m >> 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 ca= n >> 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 re= al >> fix via GEOM done, but mind you the old code will not work on SATA ATA= PI >> drives so a simple rollback of the commit is not really a fix... >> =20 > > While there may be a way to solve this automatically for ATA, this is > also an issue for floppy drives. GEOM tries to read the floppy on boot= > to see if there's a label, even if no media is present. I think GEOM > should have a way of asking drives if they can tell if they have media > present and not probe them if they can't. A separate env flag could be= > set to say "always probe removable media" if users want the previous > behavior. For most of us, this just causes boot delay. > =20 What I've sortof talked phk into is that if I return the "right" error=20 code in the "start" function, it will back off and not retry nor chatter.= We still need the "access" function to return OK if HW present or we=20 cannot send ioctl calls to an empty drive. -S=F8ren
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?473DCBDA.1080104>