Re: [Qemu-block] [Qemu-devel] [PATCH 06/16] ahci: record ncq failures

[Stefan Hajnoczi]

On Fri, Jun 26, 2015 at 02:27:39PM -0400, John Snow wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
> 
> 
> 
> On 06/26/2015 11:35 AM, Stefan Hajnoczi wrote:
> > On Mon, Jun 22, 2015 at 08:21:05PM -0400, John Snow wrote:
> >> Handle NCQ failures for cases where we want to halt the VM on IO
> >> errors.
> >> 
> >> Signed-off-by: John Snow <address@hidden> --- hw/ide/ahci.c
> >> | 17 +++++++++++++++-- hw/ide/ahci.h     |  1 + hw/ide/internal.h
> >> |  1 + 3 files changed, 17 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 71b5085..a838317
> >> 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -959,13 +959,25
> >> @@ static void ncq_cb(void *opaque, int ret) return; }
> >> 
> >> +    ncq_tfs->halt = false;
> > 
> > Why does halt need to be cleared here?
> > 
> 
> Might make more sense to clear it just on the beginning of every
> command, in execute().
> 
> There's no strong reason here other than "If there's an error and it
> should be set, it'll get reset again pretty soon." It's just a default
> state.
> 
> I could move it from process to execute.

Clearing it in one place only is cleanest.  I was wondering if I missed
a code path where the two clears are really needed.

pgpATz7U8yyRG.pgp
Description: PGP signature