bin/123693: [patch] burncd(8): workaround for busy cd-writer while ejecting

[Jaakko Heinonen]

The following reply was made to PR bin/123693; it has been noted by GNATS.

From: Jaakko Heinonen <jh at saunalahti.fi>
To: Alexander Best <alexbestms at math.uni-muenster.de>,
	"Carlos A. M. dos Santos" <unixmania at gmail.com>
Cc: bug-followup at 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