Messages in this thread | | | Date | Thu, 4 Dec 2008 14:08:52 +0100 | From | Jens Axboe <> | Subject | Re: [RFC] cdrom weirdness |
| |
On Thu, Dec 04 2008, Al Viro wrote: > 1) CDROM_LOCKDOOR sets a global variable (keeplocked) that affects all > cdroms. Intentional?
It's always been so, predates me. Pretty ugly, I don't think anyone would mind if that was changed to be per-device when the ioctl is ussed :-)
> 2) cdrom_dvd_rw_close_write() call can be delayed indefinitely by keeping > an ioctl-only (opened with O_NDELAY) descriptor.
Yep, not sure what you want me to say here...
> 3) open cdrom for data, have the door locked, keep fd opened. > open it again for write, have the open fail and cleanup in cdrom_open() > will happily unlock the door for you. I'd change that to "lock if we > had no lockers, unlock on failure exit if we did lock", but there's > an interesting comment: > /* Something failed. Try to unlock the drive, because some drivers > (notably ide-cd) lock the drive after every command. > ... > What the hell is that about? It's not "some drivers", AFAICT - it's > been done explicitly in open_for_data(). Or is there something > really driver-specific in it? > > 4) while we are at it, if you clear lockdoor via sysctl while something has > cdrom opened - no unlock on close for you. > > 5) autoeject happens on the last close *IF* the last file happens to be > opened for data. IOW, if some crap has opened it ioctl-only and kept > that opened after everyone else has closed - no autoeject for you.
Most of the above are long known issues with not counting write/non-write/ioctls opens, since it was tricky/impossible to do because of fcntl().
> 6) /* > * flush cache on last write release > */ > if (CDROM_CAN(CDC_RAM) && !cdi->use_count && cdi->for_data) > cdrom_close_write(cdi); > is interesting, seeing that nothing has ever touched ->for_data, for > values of "ever" including "since the code in question had been merged > into the tree"...
Hmm weird, you are right. The member was added in 2.6.2, but never used except here. I guess this just needs to use opened_for_data.
-- Jens Axboe
| |