Re: [Qemu-devel] [PATCH] virtio-blk: Release s->rq queue at system_reset

[Fam Zheng]

On Tue, 08/02 15:24, Fam Zheng wrote:
> On Tue, 08/02 08:46, Paolo Bonzini wrote:
> > 
> > 
> > On 29/07/2016 12:22, Fam Zheng wrote:
> > > At system_reset, there is no point in retrying the queued request,
> > > because the driver that issued the request won't be around any more.
> > > 
> > > Analyzed-by: Laszlo Ersek <address@hidden>
> > > Reported-by: Laszlo Ersek <address@hidden>
> > > Signed-off-by: Fam Zheng <address@hidden>
> > > ---
> > >  hw/block/virtio-blk.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> > > index 475a822..89eca65 100644
> > > --- a/hw/block/virtio-blk.c
> > > +++ b/hw/block/virtio-blk.c
> > > @@ -654,6 +654,7 @@ static void virtio_blk_reset(VirtIODevice *vdev)
> > >  {
> > >      VirtIOBlock *s = VIRTIO_BLK(vdev);
> > >      AioContext *ctx;
> > > +    VirtIOBlockReq *req;
> > >  
> > >      /*
> > >       * This should cancel pending requests, but can't do nicely until 
> > > there
> > > @@ -661,6 +662,11 @@ static void virtio_blk_reset(VirtIODevice *vdev)
> > >       */
> > >      ctx = blk_get_aio_context(s->blk);
> > >      aio_context_acquire(ctx);
> > > +    while (s->rq) {
> > > +        req = s->rq;
> > > +        s->rq = req->next;
> > > +        virtio_blk_free_request(req);
> > > +    }
> > >      blk_drain(s->blk);
> > 
> > blk_drain can consume requests too, so I think it should be the other
> > way round: first drain, then drop any failed request that's been left in
> > s->rq.
> 
> I don't think there is any difference in this, blk_drain cannot trigger
> virtio_blk_dma_restart_cb, because it is only hooked to vm state change.

Oh you mean that blk_drain can queue more requests to s->rq. That is a good
point, indeed.

Fam