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

[Corey Bryant <coreyb@linux.vnet.ibm.com>]

On 07/06/2012 01:40 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
>>
>
> 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.  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
> 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)
> 7. Sometime later.. QMP disconnects; refcount for fdset is decremented
> to zero; fd=4 is closed
>
> In the following case, we have an in-use and remove flag per fd and we
> only increment/decrement refcount on qemu_open/qemu_close:
>
> 1. client calls 'add-fd', qemu is now tracking fd=4 in fdset1 with
> refcount of 0; fd=4's remove flag is initialized to off and in-use flag
> is initialized to on
> 2. client calls 'device-add' with /dev/fdset/1 as the backing filename;
> qemu_open() increments the refcount of fdset1 to 1
> 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; fd=4's in-use flag is
> turned off; fd=4 is left open because it is still in-use by the block
> device (refcount is still 1)
> 4. client re-establishes QMP connection, refcount for fdset1 is still 1;
> fd=4's in-use flag is turned on; '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 1
> 6. qemu_close is called for fd=4; refcount for fdset1 is decremented to
> 0; fd=4 is closed because (refcount == 0 && (!inuse || removed)) is true
> 7. Sometime later.. QMP disconnects
>

I have one modification to this.  It looks the inuse flag could be at 
the fdset level, rather than an inuse flag per fd.  Although.. you did 
mention multiple monitors, so perhaps inuse should be a counter?

-- 
Regards,
Corey