From owner-freebsd-bugs@FreeBSD.ORG Tue Jan 20 20:10:02 2009 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A12011065670 for ; Tue, 20 Jan 2009 20:10:02 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 8EDC78FC31 for ; Tue, 20 Jan 2009 20:10:02 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.14.3/8.14.3) with ESMTP id n0KKA26K032908 for ; Tue, 20 Jan 2009 20:10:02 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.3/8.14.3/Submit) id n0KKA2FU032907; Tue, 20 Jan 2009 20:10:02 GMT (envelope-from gnats) Date: Tue, 20 Jan 2009 20:10:02 GMT Message-Id: <200901202010.n0KKA2FU032907@freefall.freebsd.org> To: freebsd-bugs@FreeBSD.org From: Jaakko Heinonen Cc: Subject: Re: bin/123693: [patch] burncd(8): workaround for busy cd-writer while ejecting X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Jaakko Heinonen List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 20 Jan 2009 20:10:03 -0000 The following reply was made to PR bin/123693; it has been noted by GNATS. From: Jaakko Heinonen To: Alexander Best , "Carlos A. M. dos Santos" Cc: bug-followup@FreeBSD.org Subject: Re: bin/123693: [patch] burncd(8): workaround for busy cd-writer while ejecting Date: Tue, 20 Jan 2009 22:00:39 +0200 Hi, A similar problem exists if you combine blank and data commands: # burncd -s max blank data image.iso blanking CD - 100 % done burncd: ioctl(CDIOCSTART): Device busy Thus in my opinion a more generic solution is desired. Possibly the correct solution would be to issue ATAPI_TEST_UNIT_READY command until the drive reports a ready state. This could be done in kernel or in user space. If done in user space there's the CDIOCRESET ioctl which issues the ATAPI_TEST_UNIT_READY command. Here is a patch which implements wait_for_ready() function in burncd using the CDIOCRESET ioctl: --- patch begins here --- Index: usr.sbin/burncd/burncd.c =================================================================== --- usr.sbin/burncd/burncd.c (revision 187153) +++ usr.sbin/burncd/burncd.c (working copy) @@ -65,6 +65,7 @@ void add_track(char *, int, int, int); void do_DAO(int fd, int, int); void do_TAO(int fd, int, int, int); void do_format(int, int, char *); +void wait_for_ready(int); int write_file(int fd, struct track_info *); int roundup_blocks(struct track_info *); void cue_ent(struct cdr_cue_entry *, int, int, int, int, int, int, int); @@ -219,6 +220,7 @@ main(int argc, char **argv) break; last = pct; } + wait_for_ready(fd); if (!quiet) printf("\n"); continue; @@ -323,6 +325,8 @@ main(int argc, char **argv) err(EX_IOERR, "ioctl(CDRIOCSETBLOCKSIZE)"); } + wait_for_ready(fd); + if (eject) if (ioctl(fd, CDIOCEJECT) < 0) err(EX_IOERR, "ioctl(CDIOCEJECT)"); @@ -600,6 +604,27 @@ do_format(int the_fd, int force, char *t fprintf(stderr, "\n"); } +void +wait_for_ready(int fd) +{ + int timeout = 10 * 1000; + + while (timeout > 0) { + /* + * CDIOCRESET issues ATAPI_TEST_UNIT_READY command. + */ + if (ioctl(fd, CDIOCRESET) == 0) + return; + else if (errno != EBUSY) + err(EX_IOERR, "ioctl(CDIOCRESET)"); + + usleep(500 * 1000); + timeout -= 500; + } + + errx(EX_IOERR, "timed out while waiting for the drive to become ready"); +} + int write_file(int fd, struct track_info *track_info) { --- patch ends here --- There is apparently a bogus privilege check in atapi-cd.c for CDIOCRESET ioctl. It prevents CDIOCRESET to work for non-root users. (Remember that ioctls are restricted with device permissions.) So this change is needed for CDIOCRESET to work for non-root users: --- patch begins here --- Index: sys/dev/ata/atapi-cd.c =================================================================== --- sys/dev/ata/atapi-cd.c (revision 187157) +++ sys/dev/ata/atapi-cd.c (working copy) @@ -255,13 +255,7 @@ acd_geom_ioctl(struct g_provider *pp, u_ cdp->flags |= F_LOCKED; break; - /* - * XXXRW: Why does this require privilege? - */ case CDIOCRESET: - error = priv_check(td, PRIV_DRIVER); - if (error) - break; error = acd_test_ready(dev); break; --- patch ends here --- Note that if you intend to test these patches on stable/7 (or older) you need to backport r187105 (a fix to ata-queue.c) from head. Otherwise the CDIOCRESET ioctl doesn't correctly report the busy state. (See http://www.freebsd.org/cgi/query-pr.cgi?pr=95979 for more information.) On 2009-01-10, Alexander Best wrote: > i adjusted your patch a bit. now the loop will break if the return value is > EIO e.g. > + while ((error = ioctl(fd, CDIOCEJECT)) < 0 && retries-- > 0) { > + if (error != EBUSY) > + err(EX_IOERR, "ioctl(CDIOCEJECT)"); Alexander: you can't test a value returned from ioctl(2) with EBUSY. You should use errno instead of error. -- Jaakko