[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [Qemu-devel] [PATCH v7 2/6] qapi: Introduce add-fd, remove-fd, query-fdsets





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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]