From owner-freebsd-stable@FreeBSD.ORG Tue Dec 26 14:48:32 2006 Return-Path: X-Original-To: stable@freebsd.org Delivered-To: freebsd-stable@FreeBSD.ORG Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id B061D16A403; Tue, 26 Dec 2006 14:48:32 +0000 (UTC) (envelope-from rizzo@icir.org) Received: from xorpc.icir.org (xorpc.icir.org [192.150.187.68]) by mx1.freebsd.org (Postfix) with ESMTP id 9BD6113C463; Tue, 26 Dec 2006 14:48:32 +0000 (UTC) (envelope-from rizzo@icir.org) Received: from xorpc.icir.org (localhost [127.0.0.1]) by xorpc.icir.org (8.12.11/8.13.6) with ESMTP id kBQEmV3K049905; Tue, 26 Dec 2006 06:48:31 -0800 (PST) (envelope-from rizzo@xorpc.icir.org) Received: (from rizzo@localhost) by xorpc.icir.org (8.12.11/8.12.3/Submit) id kBQEmVMT049904; Tue, 26 Dec 2006 06:48:31 -0800 (PST) (envelope-from rizzo) Date: Tue, 26 Dec 2006 06:48:31 -0800 From: Luigi Rizzo To: sos@freebsd.org, stable@freebsd.org Message-ID: <20061226064831.B48751@xorpc.icir.org> 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> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5.1i In-Reply-To: <4590066B.8050604@samsco.org>; from scottl@samsco.org on Mon, Dec 25, 2006 at 12:12:11PM -0500 Cc: Subject: [summary] Re: burncd 'blank' not terminating ? X-BeenThere: freebsd-stable@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Production branch of FreeBSD source code List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 Dec 2006 14:48:32 -0000 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 at ata1-master UDMA33 acd0: DVDR 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 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. 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. Patches below (to be improved to make CDIOCRESET unconditional). Does this satisfy all ? cheers luigi Index: burncd.c =================================================================== RCS file: /home/ncvs/src/usr.sbin/burncd/burncd.c,v retrieving revision 1.45 diff -u -r1.45 burncd.c --- burncd.c 13 May 2005 20:06:44 -0000 1.45 +++ burncd.c 26 Dec 2006 12:32:29 -0000 @@ -188,6 +188,7 @@ if ((!strcasecmp(argv[arg], "erase") || !strcasecmp(argv[arg], "blank")) && !test_write) { int blank, pct, last = 0; + int sec = 0; if (!strcasecmp(argv[arg], "erase")) blank = CDR_B_ALL; @@ -200,15 +201,20 @@ if (ioctl(fd, CDRIOCBLANK, &blank) < 0) err(EX_IOERR, "ioctl(CDRIOCBLANK)"); while (1) { + int done = -1; sleep(1); + sec++; + pct = 0; if (ioctl(fd, CDRIOCGETPROGRESS, &pct) == -1) err(EX_IOERR,"ioctl(CDRIOGETPROGRESS)"); - if (pct > 0 && !quiet) + if (pct == 0) + done = ioctl(fd, CDIOCRESET, NULL); + if (!quiet) fprintf(stderr, - "%sing CD - %d %% done \r", + "%sing CD - %3dsec %d %% done %d \r", blank == CDR_B_ALL ? - "eras" : "blank", pct); - if (pct == 100 || (pct == 0 && last > 90)) + "eras" : "blank", sec, pct, done); + if (pct == 100 || (pct == 0 && last > 90) || done == 0) break; last = pct; } Index: atapi-cd.c =================================================================== RCS file: /home/ncvs/src/sys/dev/ata/atapi-cd.c,v retrieving revision 1.179.2.7 diff -u -r1.179.2.7 atapi-cd.c --- atapi-cd.c 2 Sep 2006 17:01:32 -0000 1.179.2.7 +++ atapi-cd.c 24 Dec 2006 10:38:01 -0000 @@ -1234,7 +1234,7 @@ request->flags = ATA_R_ATAPI | ATA_R_READ; request->timeout = 30; ata_queue_request(request); - if (!request->error && request->u.atapi.sense.error & ATA_SENSE_VALID) + if (!request->error && request->u.atapi.sense.error) *finished = ((request->u.atapi.sense.specific2 | (request->u.atapi.sense.specific1 << 8)) * 100) / 65535; else