Re: [Qemu-devel] [PATCH 3/4] dataplane: change vring API to use VirtQueueElement

[Stefan Hajnoczi <stefanha@gmail.com>]

On Thu, Dec 05, 2013 at 11:34:28AM +0100, Paolo Bonzini wrote:
> Il 05/12/2013 10:24, Stefan Hajnoczi ha scritto:
> >> > 
> >> > That's what already happens actually.  vring_push has
> >> > 
> >> > 
> >> > +    g_slice_free(VirtQueueElement, elem);
> >> > +
> >> >      /* Don't touch vring if a fatal error occurred */
> >> >      if (vring->broken) {
> >> >          return;
> >> > 
> >> > in this patch and
> >> > 
> >> > +    for (i = 0; i < elem->out_num; i++) {
> >> > +        vring_unmap(elem->out_sg[i].iov_base, false);
> >> > +    }
> >> > +
> >> > +    for (i = 0; i < elem->in_num; i++) {
> >> > +        vring_unmap(elem->in_sg[i].iov_base, true);
> >> > +    }
> >> > 
> >> >      g_slice_free(VirtQueueElement, elem);
> >> > 
> >> > in the next one.
> >> > 
> >> > Though I admit vring_push isn't such a great name and API.  I can add
> >> > instead a vring_free_element function.  Do you think vring_push should
> >> > call it, or should the caller do that?
> > I think vring_push() should free the VirtQueueElement.
> > 
> > We just need to expose vring_free_element() so that handle_notify() can
> > call it without pushing bogus buffers back to the guest.
> 
> It's not pushing back bogus buffer, see the "if (vring->broken)" above.
>  But if you prefer handle_notify() to call vring_free_element(), I can
> of course do that.

Ah, I missed that :).  It would be clearer to call vring_free_element()
explicitly.

Stefan