diff mbox

[2/3] virtio-blk: add missing virtio_detach_element() call

Message ID 1474291685-24226-3-git-send-email-stefanha@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Stefan Hajnoczi Sept. 19, 2016, 1:28 p.m. UTC
Make sure to unmap the scatter-gather list and decrement vq->inuse
before freeing requests in virtio_blk_reset().

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/block/virtio-blk.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Ladi Prosek Sept. 27, 2016, 7:49 a.m. UTC | #1
On Mon, Sep 19, 2016 at 3:28 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> Make sure to unmap the scatter-gather list and decrement vq->inuse
> before freeing requests in virtio_blk_reset().
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  hw/block/virtio-blk.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 3a6112f..c7ca4d6 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -665,6 +665,7 @@ static void virtio_blk_reset(VirtIODevice *vdev)
>      while (s->rq) {
>          req = s->rq;
>          s->rq = req->next;
> +        virtqueue_detach_element(req->vq, &req->elem, 0);
>          virtio_blk_free_request(req);

Random observation. virtio_blk_free_request should be static and
removed from the header file. Or maybe removed altogether because
g_free takes NULL pointers just fine.

>      }
>
> --
> 2.7.4
>

Reviewed-by: Ladi Prosek <lprosek@redhat.com>

Thanks!
Greg Kurz Sept. 27, 2016, 8:07 a.m. UTC | #2
On Tue, 27 Sep 2016 09:49:17 +0200
Ladi Prosek <lprosek@redhat.com> wrote:

> On Mon, Sep 19, 2016 at 3:28 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > Make sure to unmap the scatter-gather list and decrement vq->inuse
> > before freeing requests in virtio_blk_reset().
> >
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> >  hw/block/virtio-blk.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> > index 3a6112f..c7ca4d6 100644
> > --- a/hw/block/virtio-blk.c
> > +++ b/hw/block/virtio-blk.c
> > @@ -665,6 +665,7 @@ static void virtio_blk_reset(VirtIODevice *vdev)
> >      while (s->rq) {
> >          req = s->rq;
> >          s->rq = req->next;
> > +        virtqueue_detach_element(req->vq, &req->elem, 0);
> >          virtio_blk_free_request(req);  
> 
> Random observation. virtio_blk_free_request should be static and
> removed from the header file. 

I've sent a followup patch for this:

<147487884587.6679.6170297932839464278.stgit@bahia>

> Or maybe removed altogether because g_free takes NULL pointers just fine.
> 

virtio_blk_free_request() does not seem useful indeed... :)

Cheers.

--
Greg

> >      }
> >
> > --
> > 2.7.4
> >  
> 
> Reviewed-by: Ladi Prosek <lprosek@redhat.com>
> 
> Thanks!
>
diff mbox

Patch

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 3a6112f..c7ca4d6 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -665,6 +665,7 @@  static void virtio_blk_reset(VirtIODevice *vdev)
     while (s->rq) {
         req = s->rq;
         s->rq = req->next;
+        virtqueue_detach_element(req->vq, &req->elem, 0);
         virtio_blk_free_request(req);
     }