[summary] Re: burncd 'blank' not terminating ?
rizzo at icir.org
Tue Dec 26 18:21:26 PST 2006
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
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.
More information about the freebsd-stable