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

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

On 07/05/2012 01:00 PM, Eric Blake wrote:
> 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 crashes, so all tracked fds are visited; fd=4 had not yet been
>> passed to 'remove-fd', so it's in-use flag is on;  in-use flag is turned
>> off and fd=4 is left open because it is still in use by the block device
>> (refcount is 1)
>> 4. client re-establishes QMP connection, so all tracked fds are visited,
>> and in-use flags are turned back on; 'query-fds' lets client learn about
>> fd=4 still being open as part of fdset1
>
> This says that an in-use fd comes back into use after a crash...
>
>>
>> 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.
>

Ah, yeah I missed that.

> 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).  An fd is then closed when
> 'refcount == 0 && (!inuse || removed)'.  On monitor disconnect, the
> inuse flag is cleared, and on reconnect, it is set; but that does not
> impact the removed flag.  And the 'query-fds' command would omit any fd
> with the 'removed' flag, even when the fd is still kept open internally
> thanks to the refcount being nonzero.
>

I agree.  Having the 2 flags makes sense and solves the issues you 
mentioned above.


-- 
Regards,
Corey


> But I'm definitely agreeing that tying the refcount to the set rather
> than to individual fds within the set makes sense; you still avoid the
> fd leak in that all fds in the set are closed when both the monitor has
> disavowed the set and when qemu_close() has finished using any of the fds.
>