From owner-cvs-src@FreeBSD.ORG Fri Nov 16 16:57:06 2007 Return-Path: Delivered-To: cvs-src@FreeBSD.ORG Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 95AC016A41A; Fri, 16 Nov 2007 16:57:06 +0000 (UTC) (envelope-from sos@deepcore.dk) Received: from spider.deepcore.dk (cpe.atm2-0-70484.0x50a6c9a6.abnxx16.customer.tele.dk [80.166.201.166]) by mx1.freebsd.org (Postfix) with ESMTP id ED55713C447; Fri, 16 Nov 2007 16:57:05 +0000 (UTC) (envelope-from sos@deepcore.dk) Received: from ws.local (ws.deepcore.dk [194.192.25.137]) by spider.deepcore.dk (8.13.8/8.13.8) with ESMTP id lAGGuw7S048293; Fri, 16 Nov 2007 17:56:58 +0100 (CET) (envelope-from sos@deepcore.dk) Message-ID: <473DCBDA.1080104@deepcore.dk> Date: Fri, 16 Nov 2007 17:56:58 +0100 From: =?ISO-8859-1?Q?S=F8ren_Schmidt?= User-Agent: Thunderbird 2.0.0.9 (Macintosh/20071031) MIME-Version: 1.0 To: Nate Lawson References: <200710260859.l9Q8xPdP099307@repoman.freebsd.org> <473DC220.6030809@samsco.org> <473DC93D.5060205@deepcore.dk> <473DCAA7.3090408@root.org> In-Reply-To: <473DCAA7.3090408@root.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: quoted-printable Cc: cvs-src@FreeBSD.ORG, Scott Long , src-committers@FreeBSD.ORG, cvs-all@FreeBSD.ORG Subject: Re: cvs commit: src/sys/dev/ata atapi-cd.c atapi-cd.h X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 16 Nov 2007 16:57:06 -0000 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