On Thu, 6 Feb 2014, Andreas Färber wrote:
> Am 06.02.2014 16:55, schrieb Olaf Hering:
> > 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 <o...@aepfle.de>
> > ---
> > hw/block/xen_disk.c | 13 ++++++++++++-
> > 1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> > index 098f6c6..7f0f14a 100644
> > --- a/hw/block/xen_disk.c
> > +++ b/hw/block/xen_disk.c
> > @@ -483,7 +483,18 @@ 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;
> > + }
>
> This will need a /* fall through */ comment for static analysis tools.
>
Aside from this, I think that the patch is OK.
Can you please resend it with the comment?
> > + case BLKIF_OP_READ:
> > + bdrv_acct_done(ioreq->blkdev->bs, &ioreq->acct);
> > + break;
> > + default:
> > + break;
> > + }
> > qemu_bh_schedule(ioreq->blkdev->bh);
> > }
> >
> >
>
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>
