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

[Kevin Wolf <kwolf@redhat.com>]

Am 03.07.2012 17:40, schrieb Corey Bryant:
> Thanks again for taking time to discuss this at today's QEMU community call.
> 
> Here's the proposal we discussed at the call.  Please let me know if I 
> missed anything or if there are any issues with this design.
> 
> Proposal Five:  New monitor commands enable adding/removing an fd 
> to/from a set.  The set of fds should all refer to the same file, but 
> may have different access flags (ie. O_RDWR, O_RDONLY).  qemu_open can 
> then dup the fd that has the matching access mode flags.
> PRO: Supports reopen
> PRO: All other commands work without impact by using qemu_open()
> PRO: No fd leakage (fds are associated with monitor connection and, if 
> not in use, closed when monitor disconnects)
> PRO: Security-wise this is ok since libvirt can manage the set of fd's 
> (ie. it can add/remove an O_RDWR fd to/from the set as needed).
> CON: Not atomic (e.g. doesn't add an fd with single drive_add command).
> USAGE:
> 1. add-fd /dev/fdset/1 FDSET={M} -> qemu adds fd to set named 
> "/dev/fdset/1" - command returns qemu fd (e.g fd=4) to caller.  libvirt 
> in-use flag turned on for fd.

I thought qemu would rather return the number of the fdset (which it
also assigns if none it passed, i.e. for fdset creation). Does libvirt
need the number of an individual fd?

If libvirt prefers to assign fdset numbers itself, I'm not against it,
it's just something that wasn't clear to me yet.

> 2. drive_add file=/dev/fdset/1 -> qemu_open uses the first fd from the
> set that has access flags matching the qemu_open action flags. 
> qemu_open increments refcount for this fd.
> 3. add-fd /dev/fdset/1 FDSET={M} -> qemu adds fd to set named 
> "/dev/fdset/1" - command returns qemu fd to caller (e.g fd=5).  libvirt 
> in-use flag turned on for fd.
> 3. block-commit -> qemu_open reopens "/dev/fdset/1" by using the first
> fd from the set that has access flags matching the qemu_open action 
> flags.  qemu_open increments refcount for this fd.
> 4. remove-fd /dev/fdset/1 5 -> caller requests fd==5 be removed from the 
> set.  turns libvirt in-use flag off marking the fd ready to be closed 
> when qemu is done with it.

If we decided to not return the individual fd numbers to libvirt, file
descriptors would be uniquely identified by an fdset/flags pair here.

> 5. qemu_close decrements refcount for fd, and closes fd when refcount is 
> zero and libvirt in use flag is off.

The monitor could just hold another reference, then we save the
additional flag. But that's a qemu implementation detail.

> More functional details:
> -If libvirt crashes it can call "query-fd /dev/fdset/1" to determine 
> which fds are open in the set.

We also need a query-fdsets command that lists all fdsets that exist. If
we add information about single fds to the return value of it, we
probably don't need a separate query-fd that operates on a single fdset.

> -If monitor connection closes, qemu will close fds that have a refcount 
> of zero.  Do we also need a qemu in-use flag in case refcount is zero 
> and fd is still in use?

In use by whom? If it's still in use in qemu (as in "in-use flag would
be set") and we have a refcount of zero, then that's a bug.

> -This support requires introduction of qemu_close function that will be 
> called everywhere in block layer that close is currently called.
> 
> Notes:
> -Patch series 1 will include support for all of the above.  This will be 
> my initial focus.
> -Patch series 2 will include command line support that enables 
> association of command line fd with a monitor set.  This will be my 
> secondary focus, most likely after patch series 1 is applied.

Thanks, this is a good and as far as I can tell complete summary of what
we discussed.

Kevin