On Sat, 2010-08-14 at 15:36 -0700, Matthew Dharm wrote: > On Sat, Aug 14, 2010 at 05:11:24PM -0500, James Bottomley wrote: > > On Sat, 2010-08-14 at 14:06 -0700, Greg KH wrote: > > > On Sat, Aug 14, 2010 at 03:22:33PM -0400, James Bottomley wrote: > > > > On Sat, 2010-08-14 at 12:13 -0700, Greg KH wrote: > > > > > On Thu, Aug 12, 2010 at 05:15:05PM -0700, Matthew Dharm wrote: > > > > > > On Thu, Aug 12, 2010 at 05:02:26PM -0700, Greg KH wrote: > > > > > > > Ok, that convinces me. And these patches are ok with you, right? > > > > > > > > > > > > I have only one comment... > > > > > > > > > > > > The unusual_devs.h file keeps a blank line between entries. I think this > > > > > > series of patches adds an entry without adding a blank (spacer) line. > > > > > > > > > > > > It's a minor comment, and something I think you should just fix rather than > > > > > > ask for a regeneration of the patch. > > > > > > > > > > Good point, I'll fix this up by hand. > > > > > > > > > > James, any further objection to this? > > > > > > > > Just what I've already said: it doesn't fix the whole problem. > > > > > > > > Since there's already filtering in the stor thread, just use that to > > > > return ILLEGAL REQUEST, which is what the devices would return if their > > > > firmware didn't crash. > > > > > > I thought Matt said there was no filtering there, that's the issue. > > > > It already filters INQUIRY ... this is just another couple of commands. > > INQUIRY is the *only* command filtered. It is filtered only for devices > that return garbage, since it is a MANDATORY command from a point of view > of making a device discoverable. In other words, it would be unreasonable > to ask SCSI or userspace not to generate it. > > It would hardly call this an "infrastructure" for filtering commands. it's an if ... else ... else ... discriminator; it would take two more cases for the commands > That said, James has not yet addressed my point about allowing userspace > tools to attempt the command, if they believe they can generate it. Well, that's because it didn't really make sense: the same would apply to INQUIRY which you already filter. Plus if there is a way to send the commands that doesn't crash the device, we should likely be using it. Although I don't think so: READ DISC INFORMATION has no parameters at all and the only parameter for READ CAPACITY(16) is the partial medium indicator which is pretty much never used (So I'd be really surprised if any USB devices understood it). > Nor > has he addressed the history of this practice, whereby changing SCSI was > the only way to prevent revisting the same fixes over and over again. We didn't really do that. Historically, we removed a lot of cruft from USB storage by changing the way SCSI did the initialisation sequence. This actually involved eliminating an awful lot of flags within USB. The residue got either pushed to SCSI or kept (like the INQUIRY filter) in usb storage. The reason for pushing some stuff to SCSI is that they were mandatory commands which were unsupported (i.e. the device is out of spec) ... SCSI initialisation doesn't react well when you fail a mandatory command, so the devices have an "I'm stupid" flag. It's easier to condition SCSI not to use the mandatory commands for the USB case than to try to construct a correct response (because we'll likely never get the "correct" response entirely right). The two commands in question are optional in the standards. This means that every SCSI device is allowed to respond "I don't know what you're talking about" and we handle this condition correctly in SCSI. There's no point in SCSI having a flag which says "I don't support this optional command" because we have to expect any device to say "I don't support this command" with the correct return code ... so you might as well just return the correct code and not add the flag. James -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
