From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57579) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yj8e7-0005fs-4z for qemu-devel@nongnu.org; Fri, 17 Apr 2015 11:57:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yj8e2-0006Ls-Go for qemu-devel@nongnu.org; Fri, 17 Apr 2015 11:57:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44637) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yj8e2-0006Lf-8R for qemu-devel@nongnu.org; Fri, 17 Apr 2015 11:57:30 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t3HFvTsu018569 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Fri, 17 Apr 2015 11:57:29 -0400 Message-ID: <55312D64.4080307@redhat.com> Date: Fri, 17 Apr 2015 17:57:24 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1429280557-8887-1-git-send-email-berrange@redhat.com> <1429280557-8887-26-git-send-email-berrange@redhat.com> <553123CA.2090408@redhat.com> <20150417154955.GJ23619@redhat.com> In-Reply-To: <20150417154955.GJ23619@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v1 RFC 25/34] io: add QIOTask class for async operations List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" Cc: qemu-devel@nongnu.org, Stefan Hajnoczi , Gerd Hoffmann On 17/04/2015 17:49, Daniel P. Berrange wrote: > > In this case I even think you're leaking the task. You do object_ref > > twice (once at creation time, once before qio_channel_add_watch_full) > > and only have one object_unref. I would just do without reference > > counting, and add a qio_task_free() function that calls > > qio_task_finalize() and frees the QIOTask. > > Are you referring to the qio_channel_tls_handshake() method in the > next patch ? If so it does actually have two object_unref calls > so shouldn't be leaking. In more complex scenarios I thnk the > ref counting ability will come in useful. Of course I could add > ref counting to a plain struct without using QOM, but it felt > better to just use QOM and be done with it, so people don't have > to remember which particular unref method they must use. I cannot find the second... O:-) > > BTW, do you have plans to use the GDestroyNotify argument to > > qio_task_new, or is it just for consistency? > > I'm not 100% sure if I'll need it or not yet. One of my todo items > is to double check the sequence of operations when a VNC/chardev > client disconnects while a background task is in progress. It is > possible I might need to hold a reference on the VNC/chardev state > in which case the GDestroyNotify arg will come in useful. So I just > added it for consistency initially. Yup, no problem there. Paolo