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.
>
> v5:
> -This patch is new in v5.
> -This support addresses concerns from v4 regarding fd leakage
> if the client disconnects unexpectedly. (eblake(a)redhat.com,
> kwolf(a)redhat.com, dberrange(a)redhat.com)
>
> v6:
> -No changes
>
> Signed-off-by: Corey Bryant <coreyb(a)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.
--
Regards,
Corey