Fam, On 07/29/16 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 <ler...@redhat.com> > Reported-by: Laszlo Ersek <ler...@redhat.com> > Signed-off-by: Fam Zheng <f...@redhat.com> > --- > 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); > > if (s->dataplane) { >
The comment /* * This should cancel pending requests, but can't do nicely until there * are per-device request lists. */ dates back to commit 6e02c38dadfe4cf02b0da6135adfd8d9352b90e1 Author: aliguori <aliguori@c046a42c-6fe2-441c-8c8c-71466251a162> Date: Thu Dec 4 19:52:44 2008 +0000 Add virtio-blk support Virtio-blk is a paravirtual block device based on VirtIO. It can be used by specifying the if=virtio parameter to the -drive parameter. When using -enable-kvm, it can achieve very good performance compared to IDE or SCSI. Signed-off-by: Anthony Liguori <aligu...@us.ibm.com> git-svn-id: svn://svn.savannah.nongnu.org/qemu/trunk@5870 c046a42c-6fe2-441c-8c8c-71466251a162 I think the comment is obsolete now, the idea that it expresses was actually fixed in the following commit, in my opinion: commit 6e40b3bfc7e82823cf4df5f0bf668f56db41e53a Author: Alexander Yarygin <yary...@linux.vnet.ibm.com> Date: Wed Jun 17 13:37:20 2015 +0300 virtio-blk: Use blk_drain() to drain IO requests Each call of the virtio_blk_reset() function calls blk_drain_all(), which works for all existing BlockDriverStates, while draining only one is needed. This patch replaces blk_drain_all() by blk_drain() in virtio_blk_reset(). virtio_blk_data_plane_stop() should be called after draining because it restores vblk->complete_request. Cc: "Michael S. Tsirkin" <m...@redhat.com> Cc: Christian Borntraeger <borntrae...@de.ibm.com> Cc: Cornelia Huck <cornelia.h...@de.ibm.com> Cc: Kevin Wolf <kw...@redhat.com> Cc: Paolo Bonzini <pbonz...@redhat.com> Cc: Stefan Hajnoczi <stefa...@redhat.com> Signed-off-by: Alexander Yarygin <yary...@linux.vnet.ibm.com> Message-id: 1434537440-28236-3-git-send-email-yary...@linux.vnet.ibm.com Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com> So how about turning this patch into a two part series: the first patch should drop the comment, and reference 6e40b3bfc7e82823cf4df5f0bf668f56db41e53a, while the second patch should be this patch? For this patch: Reviewed-by: Laszlo Ersek <ler...@redhat.com> I definitely recommend that Stefan review this patch too. Thanks! Laszlo