Re: [Qemu-devel] [PATCH v2] xen_disk: fix io accounting

[Kevin Wolf <kwolf@redhat.com>]

Am 07.02.2014 um 15:47 hat Olaf Hering geschrieben:
> bdrv_acct_done was called unconditional. But in case the ioreq has no
> segments there is no matching bdrv_acct_start call. This could lead to
> bogus accounting values.
> 
> Found by code inspection.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> ---
> v2:
>   add comment /* fall through */
> 
>  hw/block/xen_disk.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> index 098f6c6..fb4ca4a 100644
> --- a/hw/block/xen_disk.c
> +++ b/hw/block/xen_disk.c
> @@ -483,7 +483,19 @@ static void qemu_aio_complete(void *opaque, int ret)
>      ioreq->status = ioreq->aio_errors ? BLKIF_RSP_ERROR : BLKIF_RSP_OKAY;
>      ioreq_unmap(ioreq);
>      ioreq_finish(ioreq);
> -    bdrv_acct_done(ioreq->blkdev->bs, &ioreq->acct);
> +    switch (ioreq->req.operation) {
> +    case BLKIF_OP_WRITE:
> +    case BLKIF_OP_FLUSH_DISKCACHE:
> +        if (!ioreq->req.nr_segments) {
> +            break;
> +        }
> +	/* fall through */
> +    case BLKIF_OP_READ:
> +        bdrv_acct_done(ioreq->blkdev->bs, &ioreq->acct);
> +        break;
> +    default:
> +        break;
> +    }
>      qemu_bh_schedule(ioreq->blkdev->bh);
>  }

I think the default case could actually be abort() because it should
never happen. But either way, you can add:

Acked-by: Kevin Wolf <kwolf@redhat.com>