Re: iSCSI target: Handling in-flight requests during ctld shutdown

On 12/31/21 3:07 PM, Alexander Motin wrote:
> On 31.12.2021 17:17, John Baldwin wrote:
>> So my remaining question I guess is if I add a new
>> 'cs->cs_terminating_tasks'
>> or the like, how does cfiscsi_datamove_out ensure that no response is sent?
> 
> By the time cs_terminating_tasks is set, there is no connection any
> more, so nothing will be sent any way.  The problem with cs_terminating
> is that it is set too early, when connection may still be alive.

Ok.

> On top of that, probably not important, but CTL_TASK_I_T_NEXUS_RESET
> call by cfiscsi_session_terminate_tasks() will make the code below to
> also block the status reporting for the most commands.

Well, this was more about if there are potential commands (perhaps in the
future) that get aborted in cfiscsi_datamove_out that weren't cancelled
by CTL_TASK_I_T_NEXUS_RESET due to not being on the OOA queue.  However,
I think we agree that currently those shouldn't exist.  Perhaps I can just
assert that CTL_FLAG_ABORT is set.
  
>> The only thing I've seen so far is this code in cfiscsi_scsi_command_done:
>>
>>      /*
>>       * Do not return status for aborted commands.
>>       * There are exceptions, but none supported by CTL yet.
>>       */
>>      if (((io->io_hdr.flags & CTL_FLAG_ABORT) &&
>>           (io->io_hdr.flags & CTL_FLAG_ABORT_STATUS) == 0) ||
>>          (io->io_hdr.flags & CTL_FLAG_STATUS_SENT)) {
>>          ctl_free_io(io);
>>          icl_pdu_free(request);
>>          return;
>>      }
>>
>> Would you prefer checking cs_terminating_tasks in this function as well to
>> avoid sending the peudo-aborted responses instead of forcing CTL_FLAG_ABORT
>> on?
> 
> CTL_FLAG_ABORT flags there are correct.  The tasks are really aborted.
> But if I remember it right, CTL_TASK_I_T_NEXUS_RESET should not set
> CTL_FLAG_ABORT_STATUS, so abort statuses should not be sent.  Abort
> statuses are only sent when initiator does not expect aborts, for
> example, when some other initiator requested LUN or target reset.

I was more asking about the potential future use case of there being a command
we data abort that doesn't already have CTL_FLAG_ABORT set, but just asserting
it is already set it perhaps the simplest approach instead.

-- 
John Baldwin