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

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

On 07/09/2012 11:46 AM, Kevin Wolf wrote:
> Am 09.07.2012 17:05, schrieb Corey Bryant:
>> I'm not sure this is an issue with current design.  I know things have
>> changed a bit as the email threads evolved, so I'll paste the current
>> design that I am working from.  Please let me know if you still see any
>> issues.
>>
>> FD passing:
>> -----------
>> New monitor commands enable adding/removing an fd to/from a set.  New
>> monitor command query-fdsets enables querying of current monitor fdsets.
>>    The set of fds should all refer to the same file, with each fd having
>> different access flags (ie. O_RDWR, O_RDONLY).  qemu_open can then dup
>> the fd that has the matching access mode flags.
>>
>> Design points:
>> --------------
>> 1. add-fd
>> -> fd is passed via SCM rights and qemu adds fd to first unused fdset
>> (e.g. /dev/fdset/1)
>> -> add-fd monitor function initializes the monitor inuse flag for the
>> fdset to true
>> -> add-fd monitor function initializes the remove flag for the fd to false
>> -> add-fd returns fdset number and received fd number (e.g fd=3) to caller
>>
>> 2. drive_add file=/dev/fdset/1
>> -> qemu_open uses the first fd in fdset1 that has access flags matching
>> the qemu_open action flags and has remove flag set to false
>> -> qemu_open increments refcount for the fdset
>> -> Need to make sure that if a command like 'device-add' fails that
>> refcount is not incremented
>>
>> 3. add-fd fdset=1
>> -> fd is passed via SCM rights
>> -> add-fd monitor function adds the received fd to the specified fdset
>> (or fails if fdset doesn't exist)
>> -> add-fd monitor function initializes the remove flag for the fd to false
>> -> add-fd returns fdset number and received fd number (e.g fd=4) to caller
>>
>> 4. block-commit
>> -> qemu_open performs "reopen" by using the first fd from the fdset that
>> has access flags matching the qemu_open action flags and has remove flag
>> set to false
>> -> qemu_open increments refcount for the fdset
>> -> Need to make sure that if a command like 'block-commit' fails that
>> refcount is not incremented
>>
>> 5. remove-fd fdset=1 fd=4
>> -> remove-fd monitor function fails if fdset doesn't exist
>> -> remove-fd monitor function turns on remove flag for fd=4
>
> What was again the reason why we keep removed fds in the fdset at all?

Because if refcount is > 0 for the fd set, then the fd could be in use 
by a block device.  So we keep it around until refcount is decremented 
to zero, at which point it is safe to close.

>
> The removed flag would make sense for a fdset after a hypothetical
> close-fdset call because the fdset needs to be kept around until the
> last user closes it, but I think removed fds can be deleted immediately.

fds in an fd set really need to be kept around until zero block devices 
reference them.  At that point, if '(refcount == 0 && (!inuse || 
remove))' is true, then we'll officially close the fd.

>
> I think I might have confused remove-fd and close-fdset in earlier
> emails in this thread, so I hope this isn't inconsistent with what I
> said before.
>

Ok no problem.

>> 6. qemu_close (need to replace all close calls in block layer with
>> qemu_close)
>> -> qemu_close decrements refcount for fdset
>> -> qemu_close closes all fds that have (refcount == 0 && (!inuse || remove))
>> -> qemu_close frees the fdset if no fds remain in it
>>
>> 7. disconnecting the QMP monitor
>> -> monitor disconnect visits all fdsets on monitor and turns off monitor
>> in-use flag for fdset
>
> And close all fds with refcount == 0.
>

Yes, this makes sense.

It also makes sense to close removed fds with refcount == 0 in the 
remove-fd function.  Basically this will be the same thing we do in 
qemu_close.  We'll close any fds that evaulate the following as true:

(refcount == 0 && (!inuse || remove))

>> 8. connecting the QMP monitor
>> -> monitor connect visits all fdsets on monitor and turns on monitor
>> in-use flag for fdset
>>
>> 9. query-fdsets
>> -> returns all fdsets and fds that don't have remove flag on

-- 
Regards,
Corey