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

[Stefan Hajnoczi]

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?

>      if (ret < 0) {
> -        ncq_err(ncq_tfs);
> +        bool is_read = ncq_tfs->cmd == READ_FPDMA_QUEUED;
> +        BlockErrorAction action = blk_get_error_action(ide_state->blk,
> +                                                       is_read, -ret);
> +        if (action == BLOCK_ERROR_ACTION_STOP) {
> +            ncq_tfs->halt = true;
> +            ide_state->bus->error_status = IDE_RETRY_HBA;
> +        } else if (action == BLOCK_ERROR_ACTION_REPORT) {
> +            ncq_err(ncq_tfs);
> +        }
> +        blk_error_action(ide_state->blk, action, is_read, -ret);
>      } else {
>          ide_state->status = READY_STAT | SEEK_STAT;
>      }
>  
> -    ncq_finish(ncq_tfs);
> +    if (!ncq_tfs->halt) {
> +        ncq_finish(ncq_tfs);
> +    }
>  }
>  
>  static int is_ncq(uint8_t ata_cmd)
> @@ -1042,6 +1054,7 @@ static void process_ncq_command(AHCIState *s, int port, 
> uint8_t *cmd_fis,
>      }
>  
>      ncq_tfs->used = 1;
> +    ncq_tfs->halt = false;
>      ncq_tfs->drive = ad;
>      ncq_tfs->slot = slot;
>      ncq_tfs->cmd = ncq_fis->command;
> diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
> index 33607d7..47a3122 100644
> --- a/hw/ide/ahci.h
> +++ b/hw/ide/ahci.h
> @@ -262,6 +262,7 @@ typedef struct NCQTransferState {
>      uint8_t cmd;
>      int slot;
>      int used;
> +    bool halt;
>  } NCQTransferState;
>  
>  struct AHCIDevice {
> diff --git a/hw/ide/internal.h b/hw/ide/internal.h
> index 7a4a86d..5abee19 100644
> --- a/hw/ide/internal.h
> +++ b/hw/ide/internal.h
> @@ -499,6 +499,7 @@ struct IDEDevice {
>  #define IDE_RETRY_READ  0x20
>  #define IDE_RETRY_FLUSH 0x40
>  #define IDE_RETRY_TRIM 0x80
> +#define IDE_RETRY_HBA  0x100

Feel free to squash this patch together with the next one.  It is hard
to review in isolation since IDE_RETRY_HBA and ->halt aren't used yet.

pgp4RhTtWxpdl.pgp
Description: PGP signature