On Tue, 02/16 16:53, Paolo Bonzini wrote: > The current implementation of bdrv_qed_drain can cause a double > completion of a request. > > The problem is that bdrv_qed_drain calls qed_plug_allocating_write_reqs > unconditionally, but this is not correct if an allocating write > is queued. In this case, qed_unplug_allocating_write_reqs will > restart the allocating write and possibly cause it to complete. > The aiocb however is still in use for the L2/L1 table writes, > and will then be completed again as soon as the table writes > are stable. > > The fix is to only call qed_plug_allocating_write_reqs and > bdrv_aio_flush (which is the same as the timer callback---the patch > makes this explicit) only if the timer was scheduled in the first > place. This fixes qemu-iotests test 011. > > Cc: qemu-sta...@nongnu.org > Cc: qemu-block@nongnu.org > Cc: Stefan Hajnoczi <stefa...@redhat.com> > Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> > --- > block/qed.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/block/qed.c b/block/qed.c > index 404be1e..ebba220 100644 > --- a/block/qed.c > +++ b/block/qed.c > @@ -380,12 +380,13 @@ static void bdrv_qed_drain(BlockDriverState *bs) > { > BDRVQEDState *s = bs->opaque; > > - /* Cancel timer and start doing I/O that were meant to happen as if it > - * fired, that way we get bdrv_drain() taking care of the ongoing > requests > - * correctly. */ > - qed_cancel_need_check_timer(s); > - qed_plug_allocating_write_reqs(s); > - bdrv_aio_flush(s->bs, qed_clear_need_check, s); > + /* Fire the timer immediately in order to start doing I/O as soon as the > + * header is flushed. > + */ > + if (s->need_check_timer && timer_pending(s->need_check_timer)) {
We can assert(s->need_check_timer); > + qed_cancel_need_check_timer(s); > + qed_need_check_timer_cb(s); > + } What if an allocating write is queued (the else branch case)? Its completion will be in bdrv_drain and it could arm the need_check_timer which is wrong. We need to drain the allocating_write_reqs queue before checking the timer. Fam