[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 3/4] ide: Set BSY bit during FLUSH
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH v2 3/4] ide: Set BSY bit during FLUSH |
Date: |
Wed, 10 Jul 2013 09:27:16 +0300 |
On Thu, Jul 04, 2013 at 09:55:42AM +0200, Kevin Wolf wrote:
> 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ärber <address@hidden>
> > >
> > > The implementation of the ATA FLUSH command invokes a flush at the block
> > > 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-only.
> > >
> > > Avoid this by setting BUSY_STAT, so that the guest is made aware we are
> > > in the middle of an operation and no ATA commands are attempted to be
> > > processed concurrently.
> > >
> > > Addresses BNC#637297.
> > >
> > > Suggested-by: Gonglei (Arei) <address@hidden>
> > > Signed-off-by: Andreas Färber <address@hidden>
> > > Signed-off-by: Kevin Wolf <address@hidden>
> > > ---
> > > hw/ide/core.c | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > 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;
> > > }
> > >
> > > + s->status |= BUSY_STAT;
> > > bdrv_acct_start(s->bs, &s->acct, 0, BDRV_ACCT_FLUSH);
> > > bdrv_aio_flush(s->bs, ide_flush_cb, s);
> > > }
> >
> >
> > I can no longer boot win7 x64 on q35 with IDE using a qcow2 image. git
> > bisect determined this patch is the culprit.
> >
> > -M q35 -nodefconfig -readconfig docs/q35-chipset.cfg -drive
> > file=image.qcow2,if=none,id=mydisk -device
> > ide-drive,drive=mydisk,bus=ide.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 = 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
Any resolution yet? This blocks testing for me.