From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:40814)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1VorJK-0001ao-1j
	for qemu-devel@nongnu.org; Fri, 06 Dec 2013 04:03:06 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1VorJB-0005ZP-Jp
	for qemu-devel@nongnu.org; Fri, 06 Dec 2013 04:02:57 -0500
Received: from mail-ea0-x234.google.com ([2a00:1450:4013:c01::234]:53799)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1VorJB-0005ZC-DS
	for qemu-devel@nongnu.org; Fri, 06 Dec 2013 04:02:49 -0500
Received: by mail-ea0-f180.google.com with SMTP id f15so155178eak.25
	for <qemu-devel@nongnu.org>; Fri, 06 Dec 2013 01:02:48 -0800 (PST)
Date: Fri, 6 Dec 2013 10:02:40 +0100
From: Stefan Hajnoczi <stefanha@gmail.com>
Message-ID: <20131206090240.GA2616@stefanha-thinkpad.redhat.com>
References: <1381417639-22547-1-git-send-email-pbonzini@redhat.com>
	<1381417639-22547-4-git-send-email-pbonzini@redhat.com>
	<20131204140649.GA27759@stefanha-thinkpad.redhat.com>
	<529F690E.9020802@redhat.com>
	<20131205092406.GA9629@stefanha-thinkpad.redhat.com>
	<52A056B4.3000509@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <52A056B4.3000509@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 3/4] dataplane: change vring API to use
	VirtQueueElement
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-devel@nongnu.org, Stefan Hajnoczi <stefanha@redhat.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