Re: PATCH (for 2.6.3-rc1) for cdrom driver dvd_read_struct

[Jens Axboe]

On Wed, Feb 11 2004, Jamie Lokier wrote:
> Jens Axboe wrote:
> > On Tue, Feb 10 2004, Stuart Hayes wrote:
> > > The attached patch will make the "dvd_read_struct" function in cdrom.c 
> > > check that the DVD drive can currently do the DVD read structure command 
> > > before sending the command to the drive.  It does this by checking the 
> > > "dvd read" feature using the "get configuration" command.
> > > 
> > > Currently, cdrom.c only checks that the drive is a DVD drive before 
> > > allowing the dvd read structure command to go to the drive--it does not 
> > > make sure that the DVD drive has a DVD in it.  Without this patch, if CD 
> > > medium is in a DVD drive, and the DVD_READ_STRUCT ioctl is used, the drive
> > > will spew an ugly "illegal request" error (magicdev does this).
> > 
> > I'm rather anxious about applying anything like this, in my experience
> > it's much much safer to simply let the command fail. I don't see
> > anything technically wrong with your approach, I'd just like it tested
> > on 100 different dvd drives :)
> 
> Meaning that you don't trust the DVD media test to return true on all
> drives when and only when there's DVD media currently in there?
> 
> If that's your logic, it follows that userspace programs shouldn't
> trust the DVD media test either - they should always call
> DVD_READ_STRUCT if they would like to treat DVDs differently to CDs.
> 
> So, can you add a flag to the request meaning "if this
> DVD_READ_STRUCT request fails with illegal request, don't log it"?

There's the generic cgc->quiet bit for this very purpose. However,
thinking about this particular issue some more,  the feature check
really ought to work for all cases. So I think it's worth a shot
applying the patch as-is. I don't see any better solutions.

-- 
Jens Axboe

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/