Re: [Qemu-devel] [PATCH 2/3] scsi/virtio-scsi: Cleanup of I/Os that never started

[Eric Farman <farman@linux.vnet.ibm.com>]

On 01/14/2014 09:49 AM, Paolo Bonzini wrote:
> Il 14/01/2014 15:24, Eric Farman ha scritto:
>> There is still a small window that occurs when a cancel I/O affects
>> an asynchronous I/O operation that hasn't started.  In other words,
>> when the residual data length equals the expected data length.
>>
>> Today, the routine virtio_scsi_command_complete fails because the
>> VirtIOSCSIReq pointer (from the hba_private field in SCSIRequest)
>> was cleared earlier when virtio_scsi_complete_req was called by
>> the virtio_scsi_request_cancelled routine.  As a result, the
>> virtio_scsi_command_complete routine needs to simply return when
>> it is processing a SCSIRequest block that was marked canceled.
>>
>> Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com>
>> ---
>>   hw/scsi/virtio-scsi.c |    4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
>> index a8fde04..49a9576 100644
>> --- a/hw/scsi/virtio-scsi.c
>> +++ b/hw/scsi/virtio-scsi.c
>> @@ -306,6 +306,10 @@ static void virtio_scsi_command_complete(SCSIRequest *r, uint32_t status,
>>       VirtIOSCSIReq *req = r->hba_private;
>>       uint32_t sense_len;
>>   
>> +    if (r->io_canceled) {
>> +        return;
>> +    }
>> +
>>       req->resp.cmd->response = VIRTIO_SCSI_S_OK;
>>       req->resp.cmd->status = status;
>>       if (req->resp.cmd->status == GOOD) {
>>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
>
> Can you please look more closely into whether patch 1 is really
> necessary, include the small fix I posted into that review, and post v2
> of the series?

Heh, that's fantastic.  I pulled patch 1 out, ran the tests that hit 
into these problems, didn't see anything.  Pulled out patch 2, reran the 
tests, hit them almost immediately.  Did this a few times, with the same 
results.  So I'll fix up another series, and post shortly.

Thanks for the feedback and the review!

- Eric
>
> Thanks!
>
> Paolo
>