diff mbox

[v9,2/7] qapi: Introduce add-fd, remove-fd, query-fdsets

Message ID 1344690878-1555-3-git-send-email-coreyb@linux.vnet.ibm.com
State New
Headers show

Commit Message

Corey Bryant Aug. 11, 2012, 1:14 p.m. UTC
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.

Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
---
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)

v6
 -Make @fd optional for remove-fd (eblake@redhat.com)
 -Make @fdset-id optional for add-fd (eblake@redhat.com)

v7:
 -Share fd sets among all monitor connections (kwolf@redhat.com)
 -Added mon_refcount to keep track of monitor connection count.

v8:
 -Add opaque string to add-fd/query-fdsets.
  (stefanha@linux.vnet.ibm.com)
 -Use camel case for structures. (stefanha@linux.vnet.ibm.com)
 -Don't return in-use and refcount from query-fdsets.
  (stefanha@linux.vnet.ibm.com)
 -Don't return removed fd's from query-fdsets.
  (stefanha@linux.vnet.ibm.com)
 -Use fdset-id rather than fdset_id. (eblake@redhat.com)
 -Fix fd leak in qmp_add_fd(). (stefanha@linux.vnet.ibm.com)
 -Update QMP errors. (stefanha@linux.vnet.ibm.com, eblake@redhat.com)

v9:
 -Use fdset-id rather than fdset_id. (eblake@redhat.com)
 -Update example for query-fdsets. (eblake@redhat.com)
 -Close fd immediately on remove-fd.
  (kwolf@redhat.com, eblake@redhat.com)
 -Drop fdset refcount, and check dup_fds instead (in a later patch).
  (eblake@redhat.com)
 -Move mon_refcount code to a later patch. (kwolf@redhat.com)

 monitor.c        |  185 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qapi-schema.json |   98 +++++++++++++++++++++++++++++
 qmp-commands.hx  |  122 +++++++++++++++++++++++++++++++++++
 3 files changed, 405 insertions(+)

Comments

Eric Blake Aug. 11, 2012, 2:16 p.m. UTC | #1
On 08/11/2012 07:14 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.
> 

> 
> v9:
>  -Use fdset-id rather than fdset_id. (eblake@redhat.com)
>  -Update example for query-fdsets. (eblake@redhat.com)
>  -Close fd immediately on remove-fd.
>   (kwolf@redhat.com, eblake@redhat.com)
>  -Drop fdset refcount, and check dup_fds instead (in a later patch).
>   (eblake@redhat.com)
>  -Move mon_refcount code to a later patch. (kwolf@redhat.com)
> 

> +AddfdInfo *qmp_add_fd(bool has_fdset_id, int64_t fdset_id, bool has_opaque,
> +                      const char *opaque, Error **errp)
> +{
> +    int fd;
> +    Monitor *mon = cur_mon;
> +    MonFdset *mon_fdset;
> +    MonFdsetFd *mon_fdset_fd;
> +    AddfdInfo *fdinfo;
> +
> +    fd = qemu_chr_fe_get_msgfd(mon->chr);
> +    if (fd == -1) {
> +        error_set(errp, QERR_FD_NOT_SUPPLIED);
> +        goto error;
> +    }
> +
> +    if (has_fdset_id) {
> +        QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {
> +            if (mon_fdset->id == fdset_id) {
> +                break;
> +            }
> +        }
> +        if (mon_fdset == NULL) {
> +            error_set(errp, QERR_INVALID_PARAMETER_VALUE, "fdset-id",
> +                      "an existing fdset-id or no fdset-id");

The 'no fdset-id' portion of this error message doesn't make sense - it
can only be reached if has_fdset_id was true.

> +
> +void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp)
> +{
> +    MonFdset *mon_fdset;
> +    MonFdsetFd *mon_fdset_fd;
> +    char fd_str[60];
> +
> +    QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {
...

> +    }
> +
> +error:
> +    snprintf(fd_str, sizeof(fd_str),
> +             "fdset-id:%" PRId64 ", fd:%" PRId64, fdset_id, fd);

Oops - fd is uninitialized if has_fd is false and the outer loop failed
to find fdset_id.  You need two separate error messages here, based on
whether fd was provided.

> +-> { "execute": "query-fdsets" }
> +<- { "return": [
> +       {
> +         "fds": [
> +           {
> +             "fd": 30,
> +             "opaque": "rdonly:/path/to/file"
> +           },
> +           {
> +             "fd": 24,
> +             "opaque": "rdwr:/path/to/file"
> +           }
> +         ],
> +         "fdset-id": 1
> +       },
> +       {
> +         "fds": [
> +           {
> +             "fd": 28
> +           },
> +           {
> +             "fd": 29
> +           }
> +         ],
> +         "fdset-id": 0
> +       },

No trailing comma here.
Corey Bryant Aug. 13, 2012, 1:44 p.m. UTC | #2
I'll send a new version shortly with these updates.
Corey Bryant Aug. 13, 2012, 1:44 p.m. UTC | #3
I'll send a new version shortly with these updates.
diff mbox

Patch

diff --git a/monitor.c b/monitor.c
index 49dccfe..c1851f0 100644
--- a/monitor.c
+++ b/monitor.c
@@ -140,6 +140,23 @@  struct mon_fd_t {
     QLIST_ENTRY(mon_fd_t) next;
 };
 
+/* file descriptor associated with a file descriptor set */
+typedef struct MonFdsetFd MonFdsetFd;
+struct MonFdsetFd {
+    int fd;
+    bool removed;
+    char *opaque;
+    QLIST_ENTRY(MonFdsetFd) next;
+};
+
+/* file descriptor set containing fds passed via SCM_RIGHTS */
+typedef struct MonFdset MonFdset;
+struct MonFdset {
+    int64_t id;
+    QLIST_HEAD(, MonFdsetFd) fds;
+    QLIST_ENTRY(MonFdset) next;
+};
+
 typedef struct MonitorControl {
     QObject *id;
     JSONMessageParser parser;
@@ -211,6 +228,7 @@  static inline int mon_print_count_get(const Monitor *mon) { return 0; }
 #define QMP_ACCEPT_UNKNOWNS 1
 
 static QLIST_HEAD(mon_list, Monitor) mon_list;
+static QLIST_HEAD(mon_fdsets, MonFdset) mon_fdsets;
 
 static mon_cmd_t mon_cmds[];
 static mon_cmd_t info_cmds[];
@@ -2389,6 +2407,173 @@  int monitor_get_fd(Monitor *mon, const char *fdname)
     return -1;
 }
 
+static void monitor_fdset_cleanup(MonFdset *mon_fdset)
+{
+    MonFdsetFd *mon_fdset_fd;
+    MonFdsetFd *mon_fdset_fd_next;
+
+    QLIST_FOREACH_SAFE(mon_fdset_fd, &mon_fdset->fds, next, mon_fdset_fd_next) {
+        if (mon_fdset_fd->removed) {
+            close(mon_fdset_fd->fd);
+            g_free(mon_fdset_fd->opaque);
+            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(bool has_fdset_id, int64_t fdset_id, bool has_opaque,
+                      const char *opaque, Error **errp)
+{
+    int fd;
+    Monitor *mon = cur_mon;
+    MonFdset *mon_fdset;
+    MonFdsetFd *mon_fdset_fd;
+    AddfdInfo *fdinfo;
+
+    fd = qemu_chr_fe_get_msgfd(mon->chr);
+    if (fd == -1) {
+        error_set(errp, QERR_FD_NOT_SUPPLIED);
+        goto error;
+    }
+
+    if (has_fdset_id) {
+        QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {
+            if (mon_fdset->id == fdset_id) {
+                break;
+            }
+        }
+        if (mon_fdset == NULL) {
+            error_set(errp, QERR_INVALID_PARAMETER_VALUE, "fdset-id",
+                      "an existing fdset-id or no fdset-id");
+            goto error;
+        }
+    } else {
+        int64_t fdset_id_prev = -1;
+        MonFdset *mon_fdset_cur = QLIST_FIRST(&mon_fdsets);
+
+        /* Use first available fdset ID */
+        QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {
+            mon_fdset_cur = mon_fdset;
+            if (fdset_id_prev == mon_fdset_cur->id - 1) {
+                fdset_id_prev = mon_fdset_cur->id;
+                continue;
+            }
+            break;
+        }
+
+        mon_fdset = g_malloc0(sizeof(*mon_fdset));
+        mon_fdset->id = fdset_id_prev + 1;
+
+        /* The fdset list is ordered by fdset ID */
+        if (mon_fdset->id == 0) {
+            QLIST_INSERT_HEAD(&mon_fdsets, mon_fdset, next);
+        } else if (mon_fdset->id < mon_fdset_cur->id) {
+            QLIST_INSERT_BEFORE(mon_fdset_cur, mon_fdset, next);
+        } else {
+            QLIST_INSERT_AFTER(mon_fdset_cur, mon_fdset, next);
+        }
+    }
+
+    mon_fdset_fd = g_malloc0(sizeof(*mon_fdset_fd));
+    mon_fdset_fd->fd = fd;
+    mon_fdset_fd->removed = false;
+    if (has_opaque) {
+        mon_fdset_fd->opaque = g_strdup(opaque);
+    }
+    QLIST_INSERT_HEAD(&mon_fdset->fds, mon_fdset_fd, next);
+
+    fdinfo = g_malloc0(sizeof(*fdinfo));
+    fdinfo->fdset_id = mon_fdset->id;
+    fdinfo->fd = mon_fdset_fd->fd;
+
+    return fdinfo;
+
+error:
+    if (fd != -1) {
+        close(fd);
+    }
+    return NULL;
+}
+
+void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp)
+{
+    MonFdset *mon_fdset;
+    MonFdsetFd *mon_fdset_fd;
+    char fd_str[60];
+
+    QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {
+        if (mon_fdset->id != fdset_id) {
+            continue;
+        }
+        QLIST_FOREACH(mon_fdset_fd, &mon_fdset->fds, next) {
+            if (has_fd) {
+                if (mon_fdset_fd->fd != fd) {
+                    continue;
+                }
+                mon_fdset_fd->removed = true;
+                break;
+            } else {
+                mon_fdset_fd->removed = true;
+            }
+        }
+        if (has_fd && !mon_fdset_fd) {
+            goto error;
+        }
+        monitor_fdset_cleanup(mon_fdset);
+        return;
+    }
+
+error:
+    snprintf(fd_str, sizeof(fd_str),
+             "fdset-id:%" PRId64 ", fd:%" PRId64, fdset_id, fd);
+    error_set(errp, QERR_FD_NOT_FOUND, fd_str);
+}
+
+FdsetInfoList *qmp_query_fdsets(Error **errp)
+{
+    MonFdset *mon_fdset;
+    MonFdsetFd *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;
+
+        QLIST_FOREACH(mon_fdset_fd, &mon_fdset->fds, next) {
+            FdsetFdInfoList *fdsetfd_info;
+
+            fdsetfd_info = g_malloc0(sizeof(*fdsetfd_info));
+            fdsetfd_info->value = g_malloc0(sizeof(*fdsetfd_info->value));
+            fdsetfd_info->value->fd = mon_fdset_fd->fd;
+            if (mon_fdset_fd->opaque) {
+                fdsetfd_info->value->has_opaque = true;
+                fdsetfd_info->value->opaque = g_strdup(mon_fdset_fd->opaque);
+            } else {
+                fdsetfd_info->value->has_opaque = false;
+            }
+
+            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 bd9c450..53cf083 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2199,3 +2199,101 @@ 
 # 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: #optional The ID of the fd set to add the file descriptor to.
+#
+# @opaque: #optional A free-form string that can be used to describe the fd.
+#
+# Returns: @AddfdInfo on success
+#          If file descriptor was not received, FdNotSupplied
+#          If @fdset-id does not exist, InvalidParameterValue
+#
+# 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', '*opaque': 'str'},
+  '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.
+#
+#        If @fd is not specified, all file descriptors in @fdset-id
+#        will be removed.
+##
+{ '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.
+#
+# @opaque: #optional A free-form string that can be used to describe the fd.
+#
+# Since: 1.2.0
+##
+{ 'type': 'FdsetFdInfo',
+  'data': {'fd': 'int', '*opaque': 'str'} }
+
+##
+# @FdsetInfo:
+#
+# Information about an fd set.
+#
+# @fdset-id: The ID of the fd set.
+#
+# @fds: A list of file descriptors that belong to this fd set.
+#
+# Since: 1.2.0
+##
+{ 'type': 'FdsetInfo',
+  'data': {'fdset-id': 'int', 'fds': ['FdsetFdInfo']} }
+
+##
+# @query-fdsets:
+#
+# Return information describing all fd sets.
+#
+# Returns: A list of @FdsetInfo
+#
+# Since: 1.2.0
+#
+# Note: The list of fd sets is shared by all monitor connections.
+#
+##
+{ 'command': 'query-fdsets', 'returns': ['FdsetInfo'] }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index ac46638..f44926c 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -926,6 +926,128 @@  Example:
 
 EQMP
 
+     {
+        .name       = "add-fd",
+        .args_type  = "fdset-id:i?,opaque:s?",
+        .params     = "add-fd fdset-id opaque",
+        .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, optional)
+- "opaque": A free-form string that can be used to describe the fd.
+            (json-string, optional)
+
+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 } }
+
+Notes:
+
+(1) The list of fd sets is shared by all monitor connections.
+(2) If "fdset-id" is not specified, a new fd set will be created.
+
+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, optional)
+
+Example:
+
+-> { "execute": "remove-fd", "arguments": { "fdset-id": 1, "fd": 3 } }
+<- { "return": {} }
+
+Notes:
+
+(1) The list of fd sets is shared by all monitor connections.
+(2) If "fd" is not specified, all file descriptors in "fdset-id" will be
+    removed.
+
+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": [
+       {
+         "fds": [
+           {
+             "fd": 30,
+             "opaque": "rdonly:/path/to/file"
+           },
+           {
+             "fd": 24,
+             "opaque": "rdwr:/path/to/file"
+           }
+         ],
+         "fdset-id": 1
+       },
+       {
+         "fds": [
+           {
+             "fd": 28
+           },
+           {
+             "fd": 29
+           }
+         ],
+         "fdset-id": 0
+       },
+     ]
+   }
+
+Note: The list of fd sets is shared by all monitor connections.
+
+EQMP
+
     {
         .name       = "block_passwd",
         .args_type  = "device:B,password:s",