From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48050) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y72mg-00085s-Gp for qemu-devel@nongnu.org; Fri, 02 Jan 2015 09:00:59 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y72md-0007jQ-97 for qemu-devel@nongnu.org; Fri, 02 Jan 2015 09:00:58 -0500 Received: from mail-we0-x236.google.com ([2a00:1450:400c:c03::236]:37600) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y72md-0007jL-2Q for qemu-devel@nongnu.org; Fri, 02 Jan 2015 09:00:55 -0500 Received: by mail-we0-f182.google.com with SMTP id w62so4514981wes.41 for ; Fri, 02 Jan 2015 06:00:54 -0800 (PST) Date: Fri, 2 Jan 2015 14:00:52 +0000 From: Stefan Hajnoczi Message-ID: <20150102140052.GK10823@stefanha-thinkpad.redhat.com> References: <1419353600-30519-1-git-send-email-pbonzini@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="08ATZu8fEq0x2T3M" Content-Disposition: inline In-Reply-To: <1419353600-30519-1-git-send-email-pbonzini@redhat.com> Subject: Re: [Qemu-devel] [PATCH] net: remove all cleanup methods from NIC NetClientInfos List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org, stefanha@redhat.com --08ATZu8fEq0x2T3M Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Dec 23, 2014 at 05:53:19PM +0100, Paolo Bonzini wrote: > All NICs have a cleanup function that, in most cases, zeroes the pointer > to the NICState. In some cases, it frees data belonging to the NIC. >=20 > However, this function is never called except when exiting from QEMU. > It is not necessary to NULL pointers and free data here; the right place > to do that would be in the device's unrealize function, after calling > qemu_del_nic. Zeroing the NIC multiple times is also wrong for multiqueue > devices. >=20 > This cleanup function gets in the way of making the NetClientStates for > the NIC hold an object_ref reference to the object, so get rid of it. This patch does not drop NetClientInfo->cleanup() and clean up net.c. Do you have plans for a follow-up patch? > diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c > index 7eab7ad..7ce13d2 100644 > --- a/hw/net/dp8393x.c > +++ b/hw/net/dp8393x.c > @@ -859,22 +859,11 @@ static void nic_reset(void *opaque) > dp8393x_update_irq(s); > } > =20 > -static void nic_cleanup(NetClientState *nc) > -{ > - dp8393xState *s =3D qemu_get_nic_opaque(nc); > - > - timer_del(s->watchdog); > - timer_free(s->watchdog); > - > - g_free(s); > -} How about moving code that frees resources to unrealize? There are a couple of instances in this patch where we drop g_free() and it's a shame to lose the resource management life cycle completely. --08ATZu8fEq0x2T3M Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJUpqSUAAoJEJykq7OBq3PIFiwIALXHtJWz1/lgGjTx5pniFDIC j3ba2rqSvDucb4UvgH18/IwRHd5ZCpqZ3hDr7qCzaisVEtM66xvWpKt2qwry34BR LJrB1d9Y9GUYKp2KArnkuCZGFOitUNYbmFt42adyFvuNOjHjWShO9uyfg3E06i8d vwOK8VPgxdCNIrUzmnOjQqTlXH09Rx9mqk/QKHYsTxGUX9Kq+YEJNAHYLj4eXVnT M1BpRM0J/JLBmJFuEnaATIFVre7EQWydh+0b1qopWIbhZQsMk79IJqr1NzKsg6OL 0LGcI487hRKPvn8HRICkhb7Pbfw2q2+Z/AeWSUlDwd0PSAsDn7vGquGm2Mirwuw= =QKZB -----END PGP SIGNATURE----- --08ATZu8fEq0x2T3M--