Patchwork [v5,2/6] qapi: Introduce add-fd, remove-fd, query-fdsets

login
register
mail settings
Submitter Corey Bryant
Date July 23, 2012, 1:08 p.m.
Message ID <1343048885-1701-3-git-send-email-coreyb@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/172691/
State New
Headers show

Comments

Corey Bryant - July 23, 2012, 1:08 p.m.
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

Note: These commands are not compatible with the existing getfd
and closefd QMP commands.

v5:
 -This patch is new in v5 and replaces the pass-fd QMP command
  from v4.
 -By grouping fds in fd sets, we ease managability with an fd
  set per file, addressing concerns raised in v4 about handling
  "reopens" and preventing fd leakage. (eblake@redhat.com,
  kwolf@redhat.com, dberrange@redhat.com)

Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
---
 monitor.c        |  144 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 qapi-schema.json |   97 ++++++++++++++++++++++++++++++++++++
 qmp-commands.hx  |  121 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 361 insertions(+), 1 deletion(-)
Eric Blake - July 25, 2012, 6:16 p.m.
On 07/23/2012 07:08 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.
> 
> +##
> +# @add-fd:
> +#
> +# Add a file descriptor, that was passed via SCM rights, to an fd set.
> +#
> +# @fdset_id: The ID of the fd set to add the file descriptor to.

Should this parameter be optional, in which case qemu generates the set
number as the next available?  The return information would include the
set number whether or not this parameter was present from the caller,
and allowing it to be optional makes it slightly easier for callers to
generate a new set without having to make the caller track which set ids
are in use.

> +##
> +# @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: The file descriptor that is to be removed.

Should 'fd' be optional, as a shorthand for saying that _all_ fds
belonging to the set be marked as removed at once?

> +++ b/qmp-commands.hx
> @@ -926,6 +926,127 @@ Example:
>  
>  EQMP
>  
> +     {
> +        .name       = "add-fd",
> +        .args_type  = "fdset_id:i",
> +        .params     = "add-fd fdset_id",
> +        .help       = "Add a file descriptor, that was passed via SCM rights, to an fd set",
> +        .mhandler.cmd_new = qmp_marshal_input_add_fd,
> +    },
> +
> +SQMP
> +add-fd
> +-------
> +
> +Add a file descriptor, that was passed via SCM rights, to an fd set.
> +
> +Arguments:
> +
> +- "fdset_id": The ID of the fd set to add the file descriptor to.  (json-int)

May need tweaking, based on the answers to the above questions.
Corey Bryant - July 26, 2012, 2:55 a.m.
On 07/25/2012 02:16 PM, Eric Blake wrote:
> On 07/23/2012 07:08 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.
>>
>> +##
>> +# @add-fd:
>> +#
>> +# Add a file descriptor, that was passed via SCM rights, to an fd set.
>> +#
>> +# @fdset_id: The ID of the fd set to add the file descriptor to.
>
> Should this parameter be optional, in which case qemu generates the set
> number as the next available?  The return information would include the
> set number whether or not this parameter was present from the caller,
> and allowing it to be optional makes it slightly easier for callers to
> generate a new set without having to make the caller track which set ids
> are in use.
>

Yes, I agree, I think @fdset_id should be optional.

>> +##
>> +# @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: The file descriptor that is to be removed.
>
> Should 'fd' be optional, as a shorthand for saying that _all_ fds
> belonging to the set be marked as removed at once?
>

Yes, this makes sense.

>> +++ b/qmp-commands.hx
>> @@ -926,6 +926,127 @@ Example:
>>
>>   EQMP
>>
>> +     {
>> +        .name       = "add-fd",
>> +        .args_type  = "fdset_id:i",
>> +        .params     = "add-fd fdset_id",
>> +        .help       = "Add a file descriptor, that was passed via SCM rights, to an fd set",
>> +        .mhandler.cmd_new = qmp_marshal_input_add_fd,
>> +    },
>> +
>> +SQMP
>> +add-fd
>> +-------
>> +
>> +Add a file descriptor, that was passed via SCM rights, to an fd set.
>> +
>> +Arguments:
>> +
>> +- "fdset_id": The ID of the fd set to add the file descriptor to.  (json-int)
>
> May need tweaking, based on the answers to the above questions.
>

Ok.

Patch

diff --git a/monitor.c b/monitor.c
index 09aa3cd..e27dbbe 100644
--- a/monitor.c
+++ b/monitor.c
@@ -140,6 +140,24 @@  struct mon_fd_t {
     QLIST_ENTRY(mon_fd_t) next;
 };
 
+/* file descriptor associated with a file descriptor set */
+typedef struct mon_fdset_fd_t mon_fdset_fd_t;
+struct mon_fdset_fd_t {
+    int fd;
+    bool removed;
+    QLIST_ENTRY(mon_fdset_fd_t) next;
+};
+
+/* file descriptor set containing fds passed via SCM_RIGHTS */
+typedef struct mon_fdset_t mon_fdset_t;
+struct mon_fdset_t {
+    int64_t id;
+    int refcount;
+    bool in_use;
+    QLIST_HEAD(, mon_fdset_fd_t) fds;
+    QLIST_ENTRY(mon_fdset_t) next;
+};
+
 typedef struct MonitorControl {
     QObject *id;
     JSONMessageParser parser;
@@ -176,7 +194,8 @@  struct Monitor {
     int print_calls_nr;
 #endif
     QError *error;
-    QLIST_HEAD(,mon_fd_t) fds;
+    QLIST_HEAD(, mon_fd_t) fds;
+    QLIST_HEAD(, mon_fdset_t) fdsets;
     QLIST_ENTRY(Monitor) entry;
 };
 
@@ -2396,6 +2415,129 @@  int monitor_get_fd(Monitor *mon, const char *fdname)
     return -1;
 }
 
+static void monitor_fdset_cleanup(mon_fdset_t *mon_fdset)
+{
+    mon_fdset_fd_t *mon_fdset_fd;
+    mon_fdset_fd_t *mon_fdset_fd_next;
+
+    if (mon_fdset->refcount != 0) {
+        return;
+    }
+
+    QLIST_FOREACH_SAFE(mon_fdset_fd, &mon_fdset->fds, next, mon_fdset_fd_next) {
+        if (!mon_fdset->in_use || mon_fdset_fd->removed) {
+            close(mon_fdset_fd->fd);
+            QLIST_REMOVE(mon_fdset_fd, next);
+            g_free(mon_fdset_fd);
+        }
+    }
+
+    if (QLIST_EMPTY(&mon_fdset->fds)) {
+        QLIST_REMOVE(mon_fdset, next);
+        g_free(mon_fdset);
+    }
+}
+
+AddfdInfo *qmp_add_fd(int64_t fdset_id, Error **errp)
+{
+    int fd;
+    Monitor *mon = cur_mon;
+    mon_fdset_t *mon_fdset;
+    mon_fdset_fd_t *mon_fdset_fd;
+    AddfdInfo *fdinfo;
+
+    fd = qemu_chr_fe_get_msgfd(mon->chr);
+    if (fd == -1) {
+        qerror_report(QERR_FD_NOT_SUPPLIED);
+        return NULL;
+    }
+
+    QLIST_FOREACH(mon_fdset, &mon->fdsets, next) {
+        if (mon_fdset->id == fdset_id) {
+            break;
+        }
+    }
+
+    if (!mon_fdset) {
+        mon_fdset = g_malloc0(sizeof(*mon_fdset));
+        mon_fdset->id = fdset_id;
+        mon_fdset->refcount = 0;
+        mon_fdset->in_use = true;
+        QLIST_INSERT_HEAD(&mon->fdsets, mon_fdset, next);
+    }
+
+    mon_fdset_fd = g_malloc0(sizeof(*mon_fdset_fd));
+    mon_fdset_fd->fd = fd;
+    mon_fdset_fd->removed = false;
+    QLIST_INSERT_HEAD(&mon_fdset->fds, mon_fdset_fd, next);
+
+    fdinfo = g_malloc0(sizeof(*fdinfo));
+    fdinfo->fdset_id = fdset_id;
+    fdinfo->fd = fd;
+
+    return fdinfo;
+}
+
+void qmp_remove_fd(int64_t fdset_id, int64_t fd, Error **errp)
+{
+    Monitor *mon = cur_mon;
+    mon_fdset_t *mon_fdset;
+    mon_fdset_fd_t *mon_fdset_fd;
+    char fd_str[20];
+
+    QLIST_FOREACH(mon_fdset, &mon->fdsets, next) {
+        if (mon_fdset->id != fdset_id) {
+            continue;
+        }
+        QLIST_FOREACH(mon_fdset_fd, &mon_fdset->fds, next) {
+            if (mon_fdset_fd->fd != fd) {
+                continue;
+            }
+            mon_fdset_fd->removed = true;
+            monitor_fdset_cleanup(mon_fdset);
+            return;
+        }
+    }
+    snprintf(fd_str, sizeof(fd_str), "%ld", fd);
+    qerror_report(QERR_FD_NOT_FOUND, fd_str);
+}
+
+FdsetInfoList *qmp_query_fdsets(Error **errp)
+{
+    Monitor *mon = cur_mon;
+    mon_fdset_t *mon_fdset;
+    mon_fdset_fd_t *mon_fdset_fd;
+    FdsetInfoList *fdset_list = NULL;
+
+    QLIST_FOREACH(mon_fdset, &mon->fdsets, next) {
+        FdsetInfoList *fdset_info = g_malloc0(sizeof(*fdset_info));
+        FdsetFdInfoList *fdsetfd_list = NULL;
+
+        fdset_info->value = g_malloc0(sizeof(*fdset_info->value));
+        fdset_info->value->fdset_id = mon_fdset->id;
+        fdset_info->value->refcount = mon_fdset->refcount;
+        fdset_info->value->in_use = mon_fdset->in_use;
+
+        QLIST_FOREACH(mon_fdset_fd, &mon_fdset->fds, next) {
+            FdsetFdInfoList *fdsetfd_info = g_malloc0(sizeof(*fdsetfd_info));
+
+            fdsetfd_info->value = g_malloc0(sizeof(*fdsetfd_info->value));
+            fdsetfd_info->value->fd = mon_fdset_fd->fd;
+            fdsetfd_info->value->removed = mon_fdset_fd->removed;
+
+            fdsetfd_info->next = fdsetfd_list;
+            fdsetfd_list = fdsetfd_info;
+        }
+
+        fdset_info->value->fds = fdsetfd_list;
+
+        fdset_info->next = fdset_list;
+        fdset_list = fdset_info;
+    }
+
+    return fdset_list;
+}
+
 /* mon_cmds and info_cmds would be sorted at runtime */
 static mon_cmd_t mon_cmds[] = {
 #include "hmp-commands.h"
diff --git a/qapi-schema.json b/qapi-schema.json
index a92adb1..c07ff07 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1905,3 +1905,100 @@ 
 # Since: 0.14.0
 ##
 { 'command': 'closefd', 'data': {'fdname': 'str'} }
+
+# @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
+##
+{ 'type': 'AddfdInfo', 'data': {'fdset_id': 'int', 'fd': 'int'} }
+
+##
+# @add-fd:
+#
+# Add a file descriptor, that was passed via SCM rights, to an fd set.
+#
+# @fdset_id: The ID of the fd set to add the file descriptor to.
+#
+# Returns: @AddfdInfo on success
+#          If file descriptor was not received, FdNotSupplied
+#
+# Since: 1.2.0
+##
+{ 'command': 'add-fd', 'data': {'fdset_id': 'int'},
+  '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: 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: 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.
+##
+{ 'command': 'remove-fd', 'data': {'fdset_id': 'int', 'fd': 'int'} }
+
+##
+# @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'} }
+
+##
+# @FdsetInfo:
+#
+# Information about an fd set.
+#
+# @fdset_id: The ID of the fd set.
+#
+# @refcount: A count of the number of outstanding dup() references to
+#            this fd set.
+#
+# @in_use: If true, this fd set is in use by a connected QMP monitor.
+#
+# @fds: A list of file descriptors that belong to this fd set.
+#
+# Since: 1.2.0
+##
+{ 'type': 'FdsetInfo',
+  'data': {'fdset_id': 'int', 'refcount': 'int', 'in_use': 'bool',
+           'fds': ['FdsetFdInfo']} }
+
+##
+# @query-fdsets:
+#
+# Return information describing all fd sets.
+#
+# Returns: A list of @FdsetInfo
+#
+# Since: 1.2.0
+#
+# Notes: File descriptors are not closed until @refcount is zero,
+#        and either @in_use is false or @removed is true.
+##
+{ 'command': 'query-fdsets', 'returns': ['FdsetInfo'] }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index e3cf3c5..d485df4 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -926,6 +926,127 @@  Example:
 
 EQMP
 
+     {
+        .name       = "add-fd",
+        .args_type  = "fdset_id:i",
+        .params     = "add-fd fdset_id",
+        .help       = "Add a file descriptor, that was passed via SCM rights, to an fd set",
+        .mhandler.cmd_new = qmp_marshal_input_add_fd,
+    },
+
+SQMP
+add-fd
+-------
+
+Add a file descriptor, that was passed via SCM rights, to an fd set.
+
+Arguments:
+
+- "fdset_id": The ID of the fd set to add the file descriptor to.  (json-int)
+
+Return a json-object with the following information:
+
+- "fdset_id": The ID of the fd set that the fd was added to. (json-int)
+- "fd": The file descriptor that was received via SCM rights and added to the
+        fd set. (json-int)
+
+Example:
+
+-> { "execute": "add-fd", "arguments": { "fdset_id": 1 } }
+<- { "return": { "fdset_id": 1, "fd": 3 } }
+
+EQMP
+
+     {
+        .name       = "remove-fd",
+        .args_type  = "fdset_id:i,fd:i",
+        .params     = "remove-fd fdset_id fd",
+        .help       = "Remove a file descriptor from an fd set",
+        .mhandler.cmd_new = qmp_marshal_input_remove_fd,
+    },
+
+SQMP
+remove-fd
+---------
+
+Remove a file descriptor from an fd set.
+
+Arguments:
+
+- "fdset_id": The ID of the fd set that the file descriptor belongs to.
+              (json-int)
+- "fd": The file descriptor that is to be removed. (json-int)
+
+Example:
+
+-> { "execute": "remove-fd", "arguments": { "fdset_id": 1, "fd": 3 } }
+<- { "return": {} }
+
+Notes:
+
+(1) File descriptors that are removed:
+    (a) will not be closed until the reference count corresponding to fdset_id
+        reaches zero.
+    (b) will not be available for use after successful completion of the
+        remove-fd command.
+
+EQMP
+
+    {
+        .name       = "query-fdsets",
+        .args_type  = "",
+        .help       = "Return information describing all fd sets",
+        .mhandler.cmd_new = qmp_marshal_input_query_fdsets,
+    },
+
+SQMP
+query-fdsets
+-------------
+
+Return information describing all fd sets.
+
+Arguments: None
+
+Example:
+
+-> { "execute": "query-fdsets" }
+<- { "return": [
+       {
+         "fdset_id": 1
+         "refcount": 0,
+         "in_use": true,
+         "fds": [
+           {
+             "fd": 21,
+             "removed": false
+           },
+           {
+             "fd": 23,
+             "removed": false
+           }
+         ],
+       },
+       {
+         "fdset_id": 2
+         "refcount": 0,
+         "in_use": true,
+         "fds": [
+           {
+             "fd": 22,
+             "removed": false
+           }
+         ],
+       }
+     ]
+   }
+
+Notes:
+
+(1) File descriptors are not closed until refcount is zero, and
+    either in_use is false or removed is true.
+
+EQMP
+
     {
         .name       = "block_passwd",
         .args_type  = "device:B,password:s",