[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [Qemu-devel] [PATCH v6 3/6] monitor: Clean up fd sets on monitor disconnect

On 08/07/2012 01:32 PM, Stefan Hajnoczi wrote:
On Fri, Aug 03, 2012 at 01:28:06PM -0400, Corey Bryant wrote:
Each fd set has a boolean that keeps track of whether or not the
fd set is in use by a monitor connection.  When a monitor
disconnects, all fds that are members of an fd set with refcount
of zero are closed.  This prevents any fd leakage associated with
a client disconnect prior to using a passed fd.

  -This patch is new in v5.
  -This support addresses concerns from v4 regarding fd leakage
   if the client disconnects unexpectedly. (eblake redhat com,
   kwolf redhat com, dberrange redhat com)

  -No changes

Signed-off-by: Corey Bryant <coreyb linux vnet ibm com>
  monitor.c |   15 +++++++++++++++
  1 file changed, 15 insertions(+)

The lifecycle of fdsets and fds isn't clear to me.  It seems like just a
refcount in fdset should handle this without extra fields like in_use.

The lifecycle of fdsets and fds starts with add-fd.

I'll explain the lifecycle end of fdsets and fds below. To follow along with the code, this cleanup occurs in monitor_fdset_cleanup().

Fds are closed and removed from an fdset when there are no more open dup() fds (refcount == 0) for the fd set, and there are either no monitor connections (!in-use) or the fd has been removed with remove-fd. In other words fds get cleaned up when:

(refcount == 0 && (!in-use || removed))

Let me explain each variable:

(1) refcount is incremented when qemu_open() dup()s an fd from an fd set and is decremented when qemu_close() closes a dup()d fd. We don't want to close any fds in an fd set if refcount > 0, because this file could be reopened with different access mode flags, which would require dup() of another fd from the fdset.

(2) in-use is used to prevent fd leakage if a monitor disconnects and abandons fds. If libvirt adds fds and then disconnects without issuing a command that references the fds, then refcount will be zero, and in-use will be false, and the fds will be closed and removed from the fd set. When the monitor connection is restored, the query-fdsets command can be used to see what fd sets and fds are available.

(3) If the remove-fd command is issued, the fd is marked for removal. It won't be closed until there are no more outstanding dup() references on the fd set, for similar reasons to why we don't close the fd in (1).

fdsets simply get cleaned up once all fds from the set have been closed and removed.

Hopefully this clears things up a bit more.

Please also take a look at the v7 series that I sent out today. Fd sets are now stored globally, rather than one per Monitor object. This simplifies things a bit.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]