Re: [Qemu-devel] [PATCH v4 0/7] file descriptor passing using pass-fd

[Corey Bryant]

On 07/09/2012 10:04 AM, Kevin Wolf wrote:
> Am 06.07.2012 19:40, schrieb Corey Bryant:
> > 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
> >
> > Ok let me try this again. I was going through some of the examples and I
> > think we need a separate in-use flag.  Otherwise, when refcount gets to
> > 1, we don't know if it is because of a monitor reference or a block
> > device reference.
>
> Does it matter?

> > I think it would cause fds to sit on the monitor
> > until refcount gets to zero (monitor disconnects). Here's an example
> > without the in-use flag:
> >
> > 1. client calls 'add-fd', qemu is now tracking fd=4 in fdset1 with
> > refcount of 1 (incremented 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;
> > 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. client calls 'remove-fd fdset=1 fd=4'; qemu turns on remove flag for
> > fd=4; but fd=4 remains open because refcount of fdset1 is 2
>
> It also decreases the reference count because the monitor doesn't use it
> any more.

I don't think that will work because refcount is for the entire fdset. 
So we can't decrement the refcount for every fd that is removed from the 
fdset.

I think it is much simpler if we only increment refcount for an fdset on 
qemu_open, and only decrement refcount on qemu_close.

> > 6. qemu_close is called for fd=4; refcount for fdset1 is decremented to
> > 1; fd=4 remains open because monitor still references fdset1 (refcount
> > of fdset1 is 1)
>
> So here the refcount becomes 0 and the fdset is closed.

> > 7. Sometime later.. QMP disconnects; refcount for fdset is decremented
> > to zero; fd=4 is closed
>
> The only question that is a bit unclear to me is whether a remove-fd on
> one monitor drops the refcount only for this monitor or for all
> monitors. However, both options can be implemented without additional
> flags or counters.

Before we go back and forth on this thread, would you mind taking a look 
at the last email I sent to Luiz?  It includes all the design points 
that I'm currently working from.  I think it's a good level set and we 
can work off that thread if there are still any issues.

--
Regards,
Corey