On 08/09/2012 06:11 AM, Kevin Wolf wrote:
Am 07.08.2012 18:49, schrieb Eric Blake:
On 08/07/2012 09:58 AM, Corey Bryant wrote:
This patch adds support that enables passing of file descriptors to the QEMU monitor where they will be stored in specified file descriptor sets.
A file descriptor set can be used by a client like libvirt to store file descriptors for the same file. This allows the client to open a file with different access modes (O_RDWR, O_WRONLY, O_RDONLY) and add/remove the passed fds to/from an fd set as needed. This will allow QEMU to (in a later patch in this series) "open" and "reopen" the same file by dup()ing the fd in the fd set that corresponds to the file, where the fd has the matching access mode flag that QEMU requests.
The new QMP commands are: add-fd: Add a file descriptor to an fd set remove-fd: Remove a file descriptor from an fd set query-fdsets: Return information describing all fd sets
+ +# @AddfdInfo: +# +# Information about a file descriptor that was added to an fd set. +# +# @fdset_id: The ID of the fd set that @fd was added to. +# +# @fd: The file descriptor that was received via SCM rights and +# added to the fd set. +# +# Since: 1.2.0
We're not very consistent on '1.2' vs. '1.2.0' in since listings, but that's probably worth a global cleanup closer to hard freeze.
+## +{ 'type': 'AddfdInfo', 'data': {'fdset_id': 'int', 'fd': 'int'} }
This is a new command, so s/fdset_id/fdset-id/
+ +## +# @add-fd: +# +# Add a file descriptor, that was passed via SCM rights, to an fd set. +# +# @fdset_id: #optional The ID of the fd set to add the file descriptor to. +# +# Returns: @AddfdInfo on success +# If file descriptor was not received, FdNotSupplied +# If @fdset_id does not exist, FdSetNotFound +# +# Notes: The list of fd sets is shared by all monitor connections. +# +# If @fdset_id is not specified, a new fd set will be created. +# +# Since: 1.2.0 +## +{ 'command': 'add-fd', 'data': {'*fdset_id': 'int'},
Again, s/fdset_id/fdset-id/
+ 'returns': 'AddfdInfo' } + +## +# @remove-fd: +# +# Remove a file descriptor from an fd set. +# +# @fdset_id: The ID of the fd set that the file descriptor belongs to. +# +# @fd: #optional The file descriptor that is to be removed. +# +# Returns: Nothing on success +# If @fdset_id or @fd is not found, FdNotFound +# +# Since: 1.2.0 +# +# Notes: The list of fd sets is shared by all monitor connections. +# +# File descriptors that are removed: +# o will not be closed until the reference count corresponding +# to @fdset_id reaches zero. +# o will not be available for use after successful completion +# of the remove-fd command. +# +# If @fd is not specified, all file descriptors in @fdset_id +# will be removed. +## +{ 'command': 'remove-fd', 'data': {'fdset_id': 'int', '*fd': 'int'} }
And again.
+ +## +# @FdsetFdInfo: +# +# Information about a file descriptor that belongs to an fd set. +# +# @fd: The file descriptor value. +# +# @removed: If true, the remove-fd command has been issued for this fd. +# +# Since: 1.2.0 +## +{ 'type': 'FdsetFdInfo', 'data': {'fd': 'int', 'removed': 'bool'} }
Is it worth providing any additional information? For example, knowing whether the fd is O_RDONLY, O_WRONLY, or O_RDWR might be beneficial to management apps trying to discover what fds are already present after a reconnection, in order to decide whether to close them without having to resort to /proc/$qemupid/fdinfo/nnn lookups. It might even be worth marking such information optional, present only when 'removed':false.
Why do we even include removed=true file descriptors in query-fdsets? Shouldn't they appear to be, well, removed from a clients POV?
The problem with adding flags is the same as with errno numbers: How to do it in a platform independent way? The management application might run on a different OS than qemu, so a numeric 'flags' field could have an entirely different meaning there.
We could add bools for some flags and an enum for O_RDONLY/O_WRONLY/O_RDWR, but it's probably better to wait until we know which of them we really need.
Based on the other email thread that's going on in parallel to this, I think these issues are resolved. -- Regards, Corey