From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59520) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UueOM-0002dx-QN for qemu-devel@nongnu.org; Thu, 04 Jul 2013 03:55:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UueOH-0005pG-Vv for qemu-devel@nongnu.org; Thu, 04 Jul 2013 03:55:50 -0400 Received: from mx1.redhat.com ([209.132.183.28]:41028) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UueOH-0005p5-NW for qemu-devel@nongnu.org; Thu, 04 Jul 2013 03:55:45 -0400 Date: Thu, 4 Jul 2013 09:55:42 +0200 From: Kevin Wolf Message-ID: <20130704075523.GB2992@dhcp-200-207.str.redhat.com> References: <1370438278-1703-1-git-send-email-kwolf@redhat.com> <1370438278-1703-4-git-send-email-kwolf@redhat.com> <1372881752.2883.206.camel@ul30vt.home> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <1372881752.2883.206.camel@ul30vt.home> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 3/4] ide: Set BSY bit during FLUSH List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson Cc: agraf@suse.de, qemu-devel@nongnu.org, stefanha@redhat.com, afaerber@suse.de Am 03.07.2013 um 22:02 hat Alex Williamson geschrieben: > On Wed, 2013-06-05 at 15:17 +0200, Kevin Wolf wrote: > > From: Andreas F=E4rber > >=20 > > The implementation of the ATA FLUSH command invokes a flush at the bl= ock > > layer, which may on raw files on POSIX entail a synchronous fdatasync= (). > > This may in some cases take so long that the SLES 11 SP1 guest driver > > reports I/O errors and filesystems get corrupted or remounted read-on= ly. > >=20 > > Avoid this by setting BUSY_STAT, so that the guest is made aware we a= re > > in the middle of an operation and no ATA commands are attempted to be > > processed concurrently. > >=20 > > Addresses BNC#637297. > >=20 > > Suggested-by: Gonglei (Arei) > > Signed-off-by: Andreas F=E4rber > > Signed-off-by: Kevin Wolf > > --- > > hw/ide/core.c | 1 + > > 1 file changed, 1 insertion(+) > >=20 > > diff --git a/hw/ide/core.c b/hw/ide/core.c > > index c7a8041..9926d92 100644 > > --- a/hw/ide/core.c > > +++ b/hw/ide/core.c > > @@ -814,6 +814,7 @@ void ide_flush_cache(IDEState *s) > > return; > > } > > =20 > > + s->status |=3D BUSY_STAT; > > bdrv_acct_start(s->bs, &s->acct, 0, BDRV_ACCT_FLUSH); > > bdrv_aio_flush(s->bs, ide_flush_cb, s); > > } >=20 >=20 > I can no longer boot win7 x64 on q35 with IDE using a qcow2 image. git > bisect determined this patch is the culprit. >=20 > -M q35 -nodefconfig -readconfig docs/q35-chipset.cfg -drive > file=3Dimage.qcow2,if=3Dnone,id=3Dmydisk -device > ide-drive,drive=3Dmydisk,bus=3Dide.0 This means you're using AHCI, right? handle_cmd() in ahci.c checks the flags and does indeed behave differently now: if (s->dev[port].port.ifs[0].status & (BUSY_STAT|DRQ_STAT)) { /* async command, complete later */ s->dev[port].busy_slot =3D slot; return -1; } /* done handling the command */ return 0; The caller of this code updates pr->cmd_issue to clear the bit for the respective command slot. This is missed now, and the later completion mentioned in the comment doesn't happen for flushes, the IDE core never calls back into the AHCI core for the completion. The correct fix might be to call ide_set_inactive() in the flush callback, though I haven't checked in detail yet whether there's anything specific to DMA read/write in ide_set_inactive(). Kevin