Re: [PATCH v8 2/5] hw/block/nvme: pull aio error handling

[Minwoo Im <minwoo.im.dev@gmail.com>]

On 11/12 20:59, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
> 
> Add a new function, nvme_aio_err, to handle errors resulting from AIOs
> and use this from the callbacks.
> 
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---
>  hw/block/nvme.c | 61 +++++++++++++++++++++++++++++--------------------
>  1 file changed, 36 insertions(+), 25 deletions(-)
> 
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 51f35e8cf18b..a96a996ddc0d 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -878,6 +878,41 @@ static inline uint16_t nvme_check_bounds(NvmeNamespace *ns, uint64_t slba,
>      return NVME_SUCCESS;
>  }
>  
> +static void nvme_aio_err(NvmeRequest *req, int ret)
> +{
> +    uint16_t status = NVME_SUCCESS;
> +    Error *local_err = NULL;
> +
> +    switch (req->cmd.opcode) {
> +    case NVME_CMD_READ:
> +        status = NVME_UNRECOVERED_READ;
> +        break;
> +    case NVME_CMD_FLUSH:
> +    case NVME_CMD_WRITE:
> +    case NVME_CMD_WRITE_ZEROES:
> +        status = NVME_WRITE_FAULT;
> +        break;
> +    default:
> +        status = NVME_INTERNAL_DEV_ERROR;
> +        break;
> +    }
> +
> +    trace_pci_nvme_err_aio(nvme_cid(req), strerror(ret), status);
> +
> +    error_setg_errno(&local_err, -ret, "aio failed");
> +    error_report_err(local_err);
> +
> +    /*
> +     * Set the command status code to the first encountered error but allow a
> +     * subsequent Internal Device Error to trump it.
> +     */
> +    if (req->status && status != NVME_INTERNAL_DEV_ERROR) {
> +        return;
> +    }

Are we okay with the trace print up there in case that this if is taken?
I guess if this if is taken, the trace print may not that matter.