Christoph Hellwig, on 10/10/2010 11:32 AM wrote: > What you did in this patch is a good step toward getting rid of the > scsi logic in the backend, but it's quite enough yet. For one thing > the backends really shouldn't know anything about scsi commands, so > the call to transport_emulate_control_cdb should happen before even > calling into the backend. Second your ->emulate_foo callbacks still > are far too SCSI-specific, e.g. WRITE SAME witht unmap bit and UNMAP > emulation really should just go into a single ->discard callback > for the backend. And instead of calling into the backend again for > readcap emulation just set block_size and size attributs on a per- > device object and let common code handle it. Similarly for inquity > just set the device type and other variables and handle it in common > code. That avoid the special se_subsystem_api_cdb vector and simplifies > the code a lot conceptually a lot. Together with calling > transport_emulate_control_cdb directly from core code before going > into ->do_task that does all the required work to make the backends > independent of the scsi protocol, so we could also use it e.g. > for ATA or virtio targets. I strongly disagree with this approach to allow non-SCSI transports to use facilities provided by a generic (SCSI) target subsystem. This approach is bad, because you enforce the SCSI target subsystem to implement in its interface "the least common denominator" of all services non-SCSI transports can requests, which leads to: 1. The SCSI target subsystem code overcomplication, so making it much harder to audit and more buggy. 2. You creates hidden inter-transport dependencies inside of the target subsystem, so by fixing services for one transport you can break another. 3. By introducing additional processing to hide SCSI internals you hurt performance, especially for the mainline SCSI case. Moreover, my experience tells me that fully hiding SCSI internals of the target subsystem is hardly feasible at all. There is another, much better approach to allow non-SCSI transports to use facilities provided by the SCSI target subsystem: leave the generic target subsystem fully SCSI-centric and create for each non-SCSI transport a helper library which would allow to translate its requests to the corresponding SCSI requests and SCSI sense to their error codes. All non-SCSI transports I know, including ATA, are, basically, subsets of SCSI facilities, so translation of their requests to the corresponding SCSI commands would be a straightforward task. For instance, discard would be translated to SCSI UNMAP. This approach would bring: 1. Clear, easy auditable target subsystem's code. 2. Clear separation of SCSI and non-SCSI processing among different SCSI and non-SCSI target drivers. 3. The best performance in all cases. Vlad -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
