On Mon, Jul 16, 2012 at 6:56 PM, Nicholas A. Bellinger
<n...@linux-iscsi.org> wrote:
>> Do you have a plan for how to handle this? Do we really want to plumb
>> through another callback to tell the fabric driver to free the command
>> in this case?
> I need to think more about this ahead of changing it back again for-3.6
> now that other fabrics have the assumption of target_submit_cmd() would
> not fail.
> There is a clear case with qla2xxx for just changing it back (we might
> end up doing that just yet) but I wanted to get the other important bits
> into for-next into place first..
Sleeping on things, I now feel pretty strongly that having target_submit_cmd
return an error value for "immediate" errors where the command does not
make it into the target core is the right approach.
Freeing commands is already one of the most confusing and complex parts
of the target code, with "check_stop_free," "cmd_wait_comp" and
"SCF_ACK_KREF." Adding another code flow back to the fabric driver
with yet another set of semantics around freeing a command seems like
it's making things even harder to maintain.
On the other hand, every caller of target_submit_cmd() in the tree already
has an error path where it handles problems it detects right before calling
target_submit_cmd(), and so it's trivial to share that for problems detected
inside target_submit_cmd(). If you look at patch 4/7, pretty much the
only changes are like going from
target_submit_cmd(...);
to
if (target_submit_cmd(...))
goto err;
and in fact the ->handle_cmd() wrapper that qla_target uses to call
target_submit_cmd via tcm_qla2xxx already has a return value that
qla_target handled properly!
I guess another approach would be to split target_submit_cmd() into
the target_get_sess_cmd() part and the rest of it, and have fabric
drivers handle errors queueing commands but not have to worry about
the submit cmd part failing. But I'm not sure that's any better.
Andy, any feelings about this one way or another? Christoph?
Thanks,
Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
