Date: Tue, 26 Dec 2006 18:21:22 -0800 From: Luigi Rizzo <rizzo@icir.org> To: =?iso-8859-1?Q?S=F8ren_Schmidt?= <sos@deepcore.dk> Cc: stable@freebsd.org, sos@freebsd.org Subject: Re: [summary] Re: burncd 'blank' not terminating ? Message-ID: <20061226182122.B56038@xorpc.icir.org> In-Reply-To: <4591CD5B.4090005@deepcore.dk>; from sos@deepcore.dk on Wed, Dec 27, 2006 at 02:33:15AM %2B0100 References: <20061221092717.A6431@xorpc.icir.org> <20061222073857.GA10704@tmn.ru> <20061225165735.M22401@atlantis.atlantis.dp.ua> <20061225084704.A23448@xorpc.icir.org> <4590066B.8050604@samsco.org> <20061226064831.B48751@xorpc.icir.org> <4591CD5B.4090005@deepcore.dk>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Dec 27, 2006 at 02:33:15AM +0100, Søren Schmidt wrote: > Luigi Rizzo wrote: > > summary: there was some discussion on how to > > fix the problem, in 6.x, with "burncd -f /dev/acd0 -v blank" getting > > stuck with this message > > > > blanking CD, please wait.. > > > > This used to work on 4.x. > > > > 6.x changes in two places: > > > > * the ioctl handler, acd_get_progress() in /sys/dev/ata/atapi-cd.c > > does a much stricter error checking, expecting the ATA_SENSE_VALID > > bit set in response to the ATAPI_READ_CAPACITY call. > > None of my DVD drives do that: > > > > acd0: DVDR <HL-DT-ST DVDRAM GSA-4163B/A101> at ata1-master UDMA33 > > acd0: DVDR <HL-DT-ST DVDRAM GSA-H10N/JL10> at ata0-slave UDMA33 > > > > even though they do report a valid progress status if you disable > > the check for ATA_SENSE_VALID, and in fact they do work under 4.x > > > > * usr.sbin/burncd/burncd.c waits for a transition of CDRIOCGETPROGRESS > > from 90..99 to 0, which fails to be detected because the ioctl() > > always returns 0 when ATA_SENSE_VALID is not set. > > > > In private mail, Soren mentioned that the spec (whatever it is called) > > says that the ATA_SENSE_VALID 'shall be set' when returning a valid value, > > so he is slightly dubious on removing the check in atapi-cd.c > > > > Also, it seems that the proper way to check for completion is to issue > > the TEST UNIT READY command, which is accessible through the CDIOCRESET > > ioct. > > > > I suggest the following two fixes: > > 1. change burncd.c as below, so that if CDRIOCGETPROGRESS does not return > > anything good, it calls CDIOCRESET to determine when the command > > is complete. > > This can be improved by calling CDIOCRESET unconditionally as a > > termination test > > > FWIW just checking with TEST UNIT READY in general for progress will > fail for lots of devices really ? i thought it was the standard way to test for completion (i don't have access to a recent atapi-X spec other than this SFF-8020i r2.6 - circa 1996). But i see you have put a workaround in act_fixate(), presumably for a similar behaviour. > progress reporting so the sum of doing both is of benefit. > > 2. change atapi-cd.c to return something even if ATA_SENSE_VALID is > > unset. Apparently there is a lot of non-complying hardware around > > so restricting to what the spec says is like shooting ourselves > > in the foot. > > > As I mentioned lots of devices does return garbage there so its more a > question on which pool of devices you want to have working and which you > wont, so its not a solution, rather a decision on which devices to > support properly, I chose those that follow specs. a few comments: 1. we don't need to limit ourselves to support only one type of devices; and given the large set of non-compliant devices (basically all the ones i have!) at the very least we should have a user-settable sysctl (to ignore the ATA_SENSE_VALID bit) or a burncd option (to trust TEST UNIT READY). I'd rather not use quirks because the list might become huge. 2. these same non-compliant devices used to work on 4.x, so people will really perceive this as a regression, which is not good from a PR perspective; 3. again i have an old copy of the spec (SFF-8020i), but the description of the ATA_SENSE_VALID is ambiguous - it says (Table 137-Request Sense Standard Data) "A Valid bit of zero indicates that the information field is not as defined in this specification. A Valid bit of one indicates the information field contains valid information as defined in this Specification. ATAPI CD-ROM drives shall implement the Valid bit" So, it doesn't say the bit _must_ be '1', and i can imagine implementors that slightly deviate from the standard formatting (e.g. adding fields etc. ) setting the bit to 0. Yet, we could at least pass the info up to userland (including the VALID bit) and let the application decide what to do with it. > > Again, if burncd.c uses CDIOCRESET to determine the completion > > of the 'blank' operation, we don't care if the return value from > > CDRIOCGETPROGRESS is incorrect, because we don't rely on it. > > > Right, but that will leave out reporting of progress which has been > asked for lots of times since some of this takes time. the way i have it now, i do both CDRIOCGETPROGRESS and CDIOCRESET, using either one as a termination signal. cheers luigi
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20061226182122.B56038>