On Mon, 19 Dec 2005, Ben Collins wrote:
>
> > > case CDROMEJECT:
> > > - rq = blk_get_request(q, WRITE, __GFP_WAIT);
> > > - rq->flags |= REQ_BLOCK_PC;
> > > - rq->data = NULL;
> > > - rq->data_len = 0;
> > > - rq->timeout = BLK_DEFAULT_TIMEOUT;
> > > - memset(rq->cmd, 0, sizeof(rq->cmd));
> > > - rq->cmd[0] = GPCMD_START_STOP_UNIT;
> > > - rq->cmd[4] = 0x02 + (close != 0);
> > > - rq->cmd_len = 6;
> > > - err = blk_execute_rq(q, bd_disk, rq, 0);
> > > - blk_put_request(rq);
> > > + err = 0;
> > > +
> > > + err |= blk_send_allow_medium_removal(q, bd_disk);
> > > + err |= blk_send_start_stop(q, bd_disk, 0x01);
> > > + err |= blk_send_start_stop(q, bd_disk, 0x02);
> >
> > Do this in the eject tool, if it's required for some devices.
>
> It already is in eject tool, but as described, that requires root
> access. Not something I want to force a user to do in order to eject
> their CDROM/iPod/USBStick in gnome. What exactly is wrong with the
> commands? If they are harmless for devices that don't need it, and fix a
> huge number of problems (did you see the Cc list on the bug report?) for
> users with affected devices, then what's the harm?
I do agree that the suggested patch seems to be a real cleanup, regardless
of whether the original code bug has now been fixed or not.
Are there devices that really want the old sequence?
Also, do we really need to send fist a start_stop 1 and then a 2?
Wouldn't the _logical_ thing be to replace the old code with just a
cleaned-up-version of what the old code did, ie just do
err = blk_send_start_stop(q, bd_disk, 0x02);
for the eject case? That way we could do the patch as a pure cleanup, and
then a separate patch might change the singe "start_stop 2" with the more
complex sequence.
(IOW, I'd prefer to separate out the cleanup from the "make the eject
sequence more complete" part).
Linus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
