On Mon, May 11, 2015 at 09:32:35PM -0700, Andy Grover wrote: > I don't have a testcase but I've been looking at this code a LOT... > > The execute_rw function uses data_direction to determine read vs. write. > TCMU doesn't use that or the passed-in sgl, it just passes the CDB and the > data buffers to userspace for handling. If the emulated sequencing is in > place for CAW, the first call to tcmu_execute_rw will result in userspace > being given the cmd, and userspace would then presumably complete both > parts of the command and return success, instead of just doing the read > part. Oh, I really should have looked deeper. Yes, with the current TCMU model it doesn't work - it still passes the CDB on, which is very contrary to how the model for a backend using sbc_parse_cdb works. If you look at file, iblock and rd they totally ignore the CDB - that's the job sbc_parse_cdb does, and the get a few sets of "perfect" commanѕ out through the sbc_ops: read/write/flush, write_same and not quite perfect unmap/write_same_unmap (which really should be one). tcmu on the other hand uses sbc ops and then tries to look at the command again. I think this is really the wrong way - either you pass on the whole CDB like the old full passthrough mode and get full control in userspace. Or you just pass through the above very limited idealized commands and isolate userspace from the nasty details of SCSI CDBs and their complexities. Note thast in this case it would be even better to just implement generic block device in userspace support insteada of limiting the use case by being a target backend. So I think in the current tcmu code TCMU_PASS_ALL actually is what works better - except for the wrong set of configfs attributes we actually get that right. The PASS_IO case on the other seem very broken due to not adhering to either the low-level CDB or the high-level sbc_ops model. -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
