From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:34101) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SmpzM-0007Cs-NK for qemu-devel@nongnu.org; Thu, 05 Jul 2012 13:37:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SmpzK-0006Ey-M0 for qemu-devel@nongnu.org; Thu, 05 Jul 2012 13:37:12 -0400 Received: from e9.ny.us.ibm.com ([32.97.182.139]:50883) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SmpzK-0006EA-I5 for qemu-devel@nongnu.org; Thu, 05 Jul 2012 13:37:10 -0400 Received: from /spool/local by e9.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 5 Jul 2012 13:37:04 -0400 Received: from d01relay07.pok.ibm.com (d01relay07.pok.ibm.com [9.56.227.147]) by d01dlp01.pok.ibm.com (Postfix) with ESMTP id 8C77138C803A for ; Thu, 5 Jul 2012 13:37:01 -0400 (EDT) Received: from d03av01.boulder.ibm.com (d03av01.boulder.ibm.com [9.17.195.167]) by d01relay07.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q65Haxhe52363296 for ; Thu, 5 Jul 2012 13:37:00 -0400 Received: from d03av01.boulder.ibm.com (loopback [127.0.0.1]) by d03av01.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q65HasHt003631 for ; Thu, 5 Jul 2012 11:36:54 -0600 Message-ID: <4FF5D0AF.7040307@linux.vnet.ibm.com> Date: Thu, 05 Jul 2012 13:36:47 -0400 From: Corey Bryant MIME-Version: 1.0 References: <1340390174-7493-1-git-send-email-coreyb@linux.vnet.ibm.com> <20120626091004.GA14451@redhat.com> <4FE9A0F0.2050809@redhat.com> <20120626175045.2c7011b3@doriath.home> <4FEA37A9.10707@linux.vnet.ibm.com> <4FEA3D9C.8080205@redhat.com> <4FF21A67.8010100@linux.vnet.ibm.com> <4FF31265.1000308@linux.vnet.ibm.com> <4FF316C9.5020100@redhat.com> <4FF31CFD.7030508@linux.vnet.ibm.com> <4FF325C8.4060401@redhat.com> <4FF3F806.7020307@redhat.com> <4FF5A331.9030108@linux.vnet.ibm.com> <4FF5A9D9.7080607@redhat.com> <4FF5C24E.9010008@linux.vnet.ibm.com> <4FF5C811.6040605@redhat.com> In-Reply-To: <4FF5C811.6040605@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4 0/7] file descriptor passing using pass-fd List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Kevin Wolf , aliguori@us.ibm.com, stefanha@linux.vnet.ibm.com, libvir-list@redhat.com, qemu-devel@nongnu.org, Luiz Capitulino , pbonzini@redhat.com On 07/05/2012 01:00 PM, Eric Blake wrote: > On 07/05/2012 10:35 AM, Corey Bryant wrote: > >> 1. client calls 'add-fd', qemu is now tracking fd=4 in fdset1 with >> refcount of 0; fd=4's in-use flag is turned on >> 2. client calls 'device-add' with /dev/fdset/1 as the backing filename, >> so qemu_open() increments the refcount of fdset1 to 1 >> 3. client crashes, so all tracked fds are visited; fd=4 had not yet been >> passed to 'remove-fd', so it's in-use flag is on; in-use flag is turned >> off and fd=4 is left open because it is still in use by the block device >> (refcount is 1) >> 4. client re-establishes QMP connection, so all tracked fds are visited, >> and in-use flags are turned back on; 'query-fds' lets client learn about >> fd=4 still being open as part of fdset1 > > This says that an in-use fd comes back into use after a crash... > >> >> 1. client calls 'add-fd', qemu is now tracking fd=4 in fdset1 with >> refcount of 0; fd=4's in-use flag is turned on >> 2. client calls 'device-add' with /dev/fdset/1 as the backing filename, >> so qemu_open() increments the refcount of fdset1 to 1 >> 3. client calls 'remove-fd fdset=1 fd=4', so qemu marks fd=4 as no >> longer in-use by the monitor, and is left open because it is in use by >> the block device (refcount is 1) >> 4. client crashes, so all tracked fds are visited; fd=4 is not in-use >> but refcount is 1 so it is not closed > 5. client re-establishes QMP connection, so all tracked fds are visited. > What happens to the fd=4 in-use flag? > > ...but what are the semantics here? > > If we _always_ turn the in-use flag back on, then that says that even > though libvirt successfully asked to close the fd, the reconnection > means that libvirt once again has to ask to close things. > Ah, yeah I missed that. > If we _never_ turn the in-use flag back on, then we've broken the first > case above where we want an in-use fd to come back into use after a crash. > > Maybe that argues for two flags per fd: an in-use flag (there is > currently a monitor connection that can manipulate the fd, either > because it passed in the fd or because it reconnected) and a removed > flag (a monitor called remove-fd, and no longer wants to know about the > fd, even if it crashes and reconnects). An fd is then closed when > 'refcount == 0 && (!inuse || removed)'. On monitor disconnect, the > inuse flag is cleared, and on reconnect, it is set; but that does not > impact the removed flag. And the 'query-fds' command would omit any fd > with the 'removed' flag, even when the fd is still kept open internally > thanks to the refcount being nonzero. > I agree. Having the 2 flags makes sense and solves the issues you mentioned above. -- Regards, Corey > But I'm definitely agreeing that tying the refcount to the set rather > than to individual fds within the set makes sense; you still avoid the > fd leak in that all fds in the set are closed when both the monitor has > disavowed the set and when qemu_close() has finished using any of the fds. >