From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:37879) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Sm7jg-0007k2-Bt for qemu-devel@nongnu.org; Tue, 03 Jul 2012 14:22:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Sm7jb-0000Ty-9a for qemu-devel@nongnu.org; Tue, 03 Jul 2012 14:22:03 -0400 Received: from e39.co.us.ibm.com ([32.97.110.160]:41217) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Sm7jb-0000Sp-2X for qemu-devel@nongnu.org; Tue, 03 Jul 2012 14:21:59 -0400 Received: from /spool/local by e39.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 3 Jul 2012 12:21:51 -0600 Received: from d03relay05.boulder.ibm.com (d03relay05.boulder.ibm.com [9.17.195.107]) by d03dlp03.boulder.ibm.com (Postfix) with ESMTP id D474919D805F for ; Tue, 3 Jul 2012 18:21:44 +0000 (WET) Received: from d03av02.boulder.ibm.com (d03av02.boulder.ibm.com [9.17.195.168]) by d03relay05.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q63ILLXY154728 for ; Tue, 3 Jul 2012 12:21:23 -0600 Received: from d03av02.boulder.ibm.com (loopback [127.0.0.1]) by d03av02.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q63ILK5h016531 for ; Tue, 3 Jul 2012 12:21:20 -0600 Message-ID: <4FF3381D.40101@linux.vnet.ibm.com> Date: Tue, 03 Jul 2012 14:21:17 -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> <4FF33004.5030909@linux.vnet.ibm.com> <4FF33349.10404@redhat.com> In-Reply-To: <4FF33349.10404@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/03/2012 02:00 PM, Eric Blake wrote: > On 07/03/2012 11:46 AM, Corey Bryant wrote: > >> >> Yes, I think adding a +1 to the refcount for the monitor makes sense. >> >> I'm a bit unsure how to increment the refcount when a monitor reconnects >> though. Maybe it is as simple as adding a +1 to each fd's refcount when >> the next QMP monitor connects. > > Or maybe delay a +1 until after a 'query-fds' - it is not until the > monitor has reconnected and learned what fds it should be aware of that > incrementing the refcount again makes sense. But that would mean making > 'query-fds' track whether this is the first call since the monitor > reconnected, as it shouldn't normally increase refcounts. This doesn't sound ideal. > > The other alternative is that the monitor never re-increments a > refcount. Once a monitor disconnects, that fd is lost to the monitor, > and a reconnected monitor must pass in a new fd to be re-associated with > the fdset. In other words, the monitor's use of an fd is a one-way > operation, starting life in use but ending at the first disconnect or > remove-fd. > > I would vote for this 2nd alternative. As long as we're not introducing an fd leak. And I don't think we are if we decrement the refcount on remove-fd or on QMP disconnect. >>> 1. client calls 'add-fd', qemu is now tracking fd=4 with refcount 1, in >>> use by monitor, as member of fdset1 >>> 2. client calls 'device-add' with /dev/fdset/1 as the backing filename, >>> so qemu_open() increments the refcount to 2 >>> 3. client crashes, so all tracked fds are visited; fd=4 had not yet been >>> passed to 'remove-fd', so qemu decrements refcount to 1, but leaves fd=4 >>> open because it is still in use by the block device >>> 4. client re-establishes QMP connection, and 'query-fds' lets client >>> learn about fd=4 still being open as part of fdset1, but also informs >>> client that fd is not in use by the monitor >> >> And in step 4 the QMP connection will increment the refcount +1 for all >> fds that persisted through the QMP disconnect. (?) > > I'm not sure we need the refcount increment on reconnect. 'query-fds' > should provide enough information for the new monitor to know what > fdsets are still in use by qemu, even though they are no longer > available to 'remove-fd' from the monitor, and if the monitor is worried > about keeping the fd set alive it can call 'add-fd' to again associate a > new fd with the set. The lifetime of a set is thus as long as any of > its associated fds have a non-zero refcount. > This sounds good to me. And qemu_open will need to make sure the monitor in_use flag is true before dup'ing an fd. -- Regards, Corey