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
