From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47997) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bUQWN-0004y4-9v for qemu-devel@nongnu.org; Mon, 01 Aug 2016 23:37:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bUQWL-0003uC-C3 for qemu-devel@nongnu.org; Mon, 01 Aug 2016 23:37:34 -0400 References: <1469570853-19770-1-git-send-email-jsnow@redhat.com> <1469570853-19770-2-git-send-email-jsnow@redhat.com> <7d6771b0-189f-dab9-d5a8-e336a46a4ea5@redhat.com> From: John Snow Message-ID: <252f57a7-52ac-cb52-e8fc-df998bba0cdb@redhat.com> Date: Mon, 1 Aug 2016 23:37:22 -0400 MIME-Version: 1.0 In-Reply-To: <7d6771b0-189f-dab9-d5a8-e336a46a4ea5@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH for-2.7 1/1] ide: fix halted IO segfault at reset List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu-block@nongnu.org Cc: qemu-devel@nongnu.org, lersek@redhat.com, armbru@redhat.com, mreitz@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 >> --- >> 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.