On 04/03/2015 07:17 AM, Ilias Tsitsimpis wrote:
For bidi commands if we double that to put the data-in buffer after
the data-out buffer, then walking the iovec for data_out_bytes will
give us the start of the data-in section. Not the most elegant, but we
need to balance against the rare usage (yes?) of bidi commands, and
the additional implementation work, because of the flaw I mentioned in
handling new tcmu_opcodes.
BIDI commands are not as rare as they look like. For example, all the
Object-Based Storage Devices Commands (OSD) are bidirectional. In fact,
the kernel has an implementation for an OSD initiator and this is what I
use to test my code.
I understand that handling new tcmu_opcodes requires additional
implementation work, but I believe we should head in that direction. I
am afraid that adding new *features* without fixing this now, will lead
to a less elegant approach, which will increase code complexity and make
debugging harder later on.
Ilias you make many good points. Given what you say I agree it would be
best to fix the TCMU interface. We need to 1) allow handling of BIDI ops
by adding a bidi_iov_cnt to struct tcmu_cmd_entry.req; and while we're
at it 2) define the mechanism by which userspace can indicate a
tcmu_opcode was not handled. This will enable future tcmu_opcodes to be
backwards compatible.
TCMU was only added in 3.18 and AFAIK tcmu-runner (written by me) is the
only user of it. To what degree do we need to worry about unknown users?
We can bump tcmu_mailbox.version to 2, which will prevent hypothetical
unknown users of v1 from accidentally using v2 (the spec says tcmu
handlers must check this value). Beyond that I don't see maintaining v1
along with v2 ABI -- for probably nobody -- as being worth the effort
and added complexity.
Nick, Christoph, anyone else: is this a reasonable approach to take, or
is the cat already out of the bag?
Thanks -- Regards -- Andy
--
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
