Re: [Qemu-devel] [PATCH for-2.7 1/1] ide: fix halted IO segfault at reset

[John Snow <jsnow@redhat.com>]

On 08/01/2016 04:52 AM, Paolo Bonzini wrote:
>
>
> On 27/07/2016 00:07, John Snow wrote:
>> If one attempts to perform a system_reset after a failed IO request
>> that causes the VM to enter a paused state, QEMU will segfault trying
>> to free up the pending IO requests.
>>
>> These requests have already been completed and freed, though, so all
>> we need to do is free them before we enter the paused state.
>>
>> Existing AHCI tests verify that halted requests are still resumed
>> successfully after a STOP event.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>  hw/ide/core.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/hw/ide/core.c b/hw/ide/core.c
>> index 081c9eb..d117b7c 100644
>> --- a/hw/ide/core.c
>> +++ b/hw/ide/core.c
>> @@ -823,6 +823,7 @@ static void ide_dma_cb(void *opaque, int ret)
>>      }
>>      if (ret < 0) {
>>          if (ide_handle_rw_error(s, -ret, ide_dma_cmd_to_retry(s->dma_cmd))) {
>> +            s->bus->dma->aiocb = NULL;
>>              return;
>>          }
>>      }
>>
>
> The patch is (was, since it's committed :)) okay, but I think there is
> another bug in the REPORT case, where ide_rw_error and
> ide_atapi_io_error are not calling ide_set_inactive and thus are leaving
> s->bus->dma->aiocb non-NULL.
>
> Paolo
>

Actually, won't we hit ide_dma_error on REPORT which calls 
ide_set_inactive? I think this might be OK, but I have to audit a little 
more carefully -- I will do so tomorrow.

I think the ide_rw_error case is likely OK, but I always manage to 
forget exactly how the ATAPI DMA looks.