From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:34289) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SnC93-00066x-90 for qemu-devel@nongnu.org; Fri, 06 Jul 2012 13:16:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SnC90-0004Eq-L8 for qemu-devel@nongnu.org; Fri, 06 Jul 2012 13:16:40 -0400 Received: from e33.co.us.ibm.com ([32.97.110.151]:57070) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SnC90-0004E5-Aq for qemu-devel@nongnu.org; Fri, 06 Jul 2012 13:16:38 -0400 Received: from /spool/local by e33.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 6 Jul 2012 11:16:32 -0600 Received: from d03relay03.boulder.ibm.com (d03relay03.boulder.ibm.com [9.17.195.228]) by d03dlp01.boulder.ibm.com (Postfix) with ESMTP id 9CE7A1FF004C for ; Fri, 6 Jul 2012 17:15:52 +0000 (WET) Received: from d03av06.boulder.ibm.com (d03av06.boulder.ibm.com [9.17.195.245]) by d03relay03.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q66HFeiR128004 for ; Fri, 6 Jul 2012 11:15:44 -0600 Received: from d03av06.boulder.ibm.com (loopback [127.0.0.1]) by d03av06.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q66HGh7T009722 for ; Fri, 6 Jul 2012 11:16:43 -0600 Message-ID: <4FF71D39.8080208@linux.vnet.ibm.com> Date: Fri, 06 Jul 2012 13:15:37 -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> <4FF6ABD9.4040502@redhat.com> <4FF71CE3.90103@linux.vnet.ibm.com> In-Reply-To: <4FF71CE3.90103@linux.vnet.ibm.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: Kevin Wolf Cc: aliguori@us.ibm.com, stefanha@linux.vnet.ibm.com, libvir-list@redhat.com, qemu-devel@nongnu.org, Luiz Capitulino , pbonzini@redhat.com, Eric Blake Ugh... please disregard this. I hit send accidentally. On 07/06/2012 01:14 PM, Corey Bryant wrote: > > > On 07/06/2012 05:11 AM, Kevin Wolf wrote: >> Am 05.07.2012 19:00, schrieb Eric Blake: >>> 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 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. >>> >>> 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). >> >> I was in fact just going to suggest a removed flag as well, however >> combined with including the monitor connections in the refcount instead >> of an additional flag. This would also allow to have (the currently >> mostly hypothetical case of) multiple QMP monitors without interesting >> things happening. >> >> Maybe I'm missing some point that the inuse flag would allow and >> including monitors in the refcount doesn't. Is there one? >> >> Kevin >> > > I think we need the granularity of having an in-use flag per fd. Here's > an example of why: > > 1. client calls 'add-fd', qemu is now tracking fd=4 in fdset1 with > refcount of 1; fd=4's remove flag is initialized to off > 2. client calls 'device-add' with /dev/fdset/1 as the backing filename; > qemu_open() increments the refcount of fdset1 to 2 > 3. client crashes, so all fdsets are visited; fd=4 had not yet been > passed to 'remove-fd', so it's remove flag is off; refcount for fdset1 > is decremented to 1; fd=4 is left open because it is still in use by the > block device (refcount is 1) > 4. client re-establishes QMP connection, refcount for fdset1 is > incremented to 2; 'query-fds' lets client learn about fd=4 still being > open as part of fdset1 > 5. qemu_close is called for fd=4; refcount for fdset1 is decremented to > 1; fd=4 remains open as part of fdset1 > 6. QMP disconnects; refcount for fdset is decremented to zero; fd=4 is > closed and fdset1 is freed > > 1. client calls 'add-fd', qemu is now tracking fd=4 in fdset1 with > refcount of 1 (because of monitor reference); fd=4's remove flag is > initialized to off > 2. client calls 'device-add' with /dev/fdset/1 as the backing filename, > but the command fails for some other reason, so the refcount is still 1 > at the end of the command (although it may have been temporarily > incremented then decremented during the command) > 3. client calls 'remove-fd fdset=1 fd=4' to deal with the failure (or > QMP connection is closed), so qemu turns on the remove flag for fd=4; > refcount is 0 so all fds in fdset1 are closed > > > I think we need the granularity of having an in-use flag per fd. If we > track monitor in-use in the reference count, then we are tracking it for > the fdset and I think this could cause leaks. > > In the following example, we have a refcount for the fdset, an in-use > flag per fd, and a remove flag per fd. We're only > incrementing/decrementing refcount in qemu_open/qemu_close. > > > > > > In the following example, we have a refcount for the fdset, and a remove > flag per fd. We're incrementing refcount when the fdset is first > created, when QMP re-connects, and in qemu_open. We're decrementing > refcount when QMP disconnects, and in qemu_close. > > 1. client calls 'add-fd', qemu is now tracking fd=4 in fdset1 with > refcount of 1; fd=4's remove flag is initialized to off > 2. client crashes, so all tracked fdsets are visited; fd=4 has > not yet been passed to 'remove-fd', so its remove flag is off; in-use > flags are turned off and both fds are left open because the set is still > in use by the block device (refcount is 1) > > > > > > > > > 1. client calls 'add-fd', qemu is now tracking fd=4 in fdset1 with > refcount of 1; fd=4's remove flag is initialized to off > 2. client calls 'device-add' with /dev/fdset/1 as the backing filename; > qemu_open() increments the refcount of fdset1 to 2 > 3. client crashes, so all fdsets are visited; fd=4 had not yet been > passed to 'remove-fd', so it's remove flag is off; refcount for fdset1 > is decremented to 1; fd=4 is left open because it is still in use by the > block device (refcount is 1) > 4. client re-establishes QMP connection, refcount for fdset1 is > incremented to 2; 'query-fds' lets client learn about fd=4 still being > open as part of fdset1 > 5. qemu_close is called for fd=4; refcount for fdset1 is decremented to > 1; fd=4 remains open as part of fdset1 > 6. QMP disconnects; refcount for fdset is decremented to zero; fd=4 is > closed and fdset1 is freed > > 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 > > 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, > but the command fails for some other reason, so the refcount is still 0 > at the end of the command (although it may have been temporarily > incremented then decremented during the command) > 3. client calls 'remove-fd fdset=1 fd=4' to deal with the failure (or > QMP connection is closed), so qemu marks fd=4 as no longer in-use by the > monitor; refcount is 0 so all fds in fdset1 are closed > > 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 'add-fd', qemu is now tracking fd=5 in fdset1 with > refcount still 0; fd=5's in-use flag is turned on > 3. client crashes, so all tracked fds are visited; fd=4 and fd=5 had not > yet been passed to 'remove-fd', so their in-use flags are on; in-use > flags are turned off; refcount of fdset1 is 0 so qemu closes all fds in > fdset1 > > 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 'add-fd', qemu is now tracking fd=5 in fdset1 with > refcount still 0; fd=5's in-use flag is turned on > 3. client calls 'remove-fd fdset=1 fd=4', so qemu marks fd=4 as no > longer in-use by the monitor, and fd=4 is closed since the refcount is > 0; fd=5 remains open > > 1. client calls 'add-fd', qemu is now tracking fd=4 (O_RDWR) in fdset1 > with refcount of 0; fd=4's in-use flag is turned on > 2. client calls 'add-fd', qemu is now tracking fd=5 (O_RDONLY) in fdset1 > with refcount still 0; fd=5's in-use flag is turned on > 3. client calls 'device-add' with /dev/fdset/1 as the backing filename > and r/w, so qemu_open() dup()s fd 4 and increments the refcount to 1 > 4. client calls 'remove-fd fdset=1 fd=4', so qemu marks fd=4 as no > longer in-use by the monitor, but fd=4 remains open because refcount of > fdset1 is 1 > 5. client calls 'remove-fd fdset=1 fd=5', so qemu marks fd=5 as no > longer in-use by the monitor, and fd=5 remains open because refcount of > fdset1 is 1 > 6. qemu_close(fd=4) is called, refcount of fdset1 is decremented; both > fds are closed since refcount is 0 and their in-use flags are off > > 1. client calls 'add-fd', qemu is now tracking fd=4 (O_RDWR) in fdset1 > with refcount of 0; fd=4's in-use flag is turned on > 2. client calls 'add-fd', qemu is now tracking fd=5 (O_RDONLY) in fdset1 > with refcount still 0; fd=5's in-use flag is turned on > 3. client calls 'device-add' with /dev/fdset/1 as the backing filename > and r/w, so qemu_open() dup()s fd 4 and increments the refcount to 1 > 4. client crashes, so all tracked fds are visited; fd=4 and fd=5 have > not yet been passed to 'remove-fd', so their in-use flags are on; in-use > flags are turned off and both fds are left open because the set is still > in use by the block device (refcount is 1) > 5. qemu_close(fd=4) is called, refcount of fdset1 is decremented; both > fds are closed since refcount is 0 and their in-use flags are off > > -- Regards, Corey