[Qemu-devel] [PATCH v4 3/7] qapi: Add pass-fd QMP command

[Corey Bryant <coreyb@linux.vnet.ibm.com>]

This patch adds the pass-fd QMP command using the QAPI framework.
Like the getfd command, it is used to pass a file descriptor via
SCM_RIGHTS and associate it with a name.  However, the pass-fd
command also returns the received file descriptor, which is a
difference in behavior from the getfd command, which returns
nothing.  pass-fd also supports a boolean "force" argument that
can be used to specify that an existing file descriptor is
associated with the specified name, and that it should become a
copy of the newly passed file descriptor.

The closefd command can be used to close a file descriptor that was
passed with the pass-fd command.

Note that when using getfd or pass-fd, there are some commands
(e.g. migrate with fd:name) that implicitly close the named fd.
When this is not the case, closefd must be used to avoid fd leaks.

Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
---
v2:
 -Introduce new QMP command to pass/return fd (lcapitulino@redhat.com)
 -Use passfd as command name (berrange@redhat.com)

v3:
 -Use pass-fd as command name (lcapitulino@redhat.com)
 -Fail pass-fd if fdname already exists (lcapitulino@redhat.com)
 -Add notes to QMP command describing behavior in more detail
  (lcapitulino@redhat.com, eblake@redhat.com)
 -Add note about fd leakage (eblake@redhat.com)

v4:
 -Don't return same error class twice (lcapitulino@redhat.com)
 -Share code for qmp_gefd and qmp_pass_fd (lcapitulino@redhat.com)
 -Update qmp_closefd to call monitor_get_fd
 -Add support for "force" argument to pass-fd (eblake@redhat.com)

 dump.c           |    3 +-
 migration-fd.c   |    2 +-
 monitor.c        |   96 +++++++++++++++++++++++++++++++++++-------------------
 monitor.h        |    2 +-
 net.c            |    6 ++--
 qapi-schema.json |   36 ++++++++++++++++++++
 qmp-commands.hx  |   42 +++++++++++++++++++++++-
 7 files changed, 146 insertions(+), 41 deletions(-)

diff --git a/dump.c b/dump.c
index 4412d7a..2fd8ced 100644
--- a/dump.c
+++ b/dump.c
@@ -836,9 +836,8 @@ void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
 
 #if !defined(WIN32)
     if (strstart(file, "fd:", &p)) {
-        fd = monitor_get_fd(cur_mon, p);
+        fd = monitor_get_fd(cur_mon, p, errp);
         if (fd == -1) {
-            error_set(errp, QERR_FD_NOT_FOUND, p);
             return;
         }
     }
diff --git a/migration-fd.c b/migration-fd.c
index 50138ed..7335167 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -75,7 +75,7 @@ static int fd_close(MigrationState *s)
 
 int fd_start_outgoing_migration(MigrationState *s, const char *fdname)
 {
-    s->fd = monitor_get_fd(cur_mon, fdname);
+    s->fd = monitor_get_fd(cur_mon, fdname, NULL);
     if (s->fd == -1) {
         DPRINTF("fd_migration: invalid file descriptor identifier\n");
         goto err_after_get_fd;
diff --git a/monitor.c b/monitor.c
index 1a7f7e7..3433c06 100644
--- a/monitor.c
+++ b/monitor.c
@@ -814,7 +814,7 @@ static int add_graphics_client(Monitor *mon, const QDict *qdict, QObject **ret_d
     CharDriverState *s;
 
     if (strcmp(protocol, "spice") == 0) {
-        int fd = monitor_get_fd(mon, fdname);
+        int fd = monitor_get_fd(mon, fdname, NULL);
         int skipauth = qdict_get_try_bool(qdict, "skipauth", 0);
         int tls = qdict_get_try_bool(qdict, "tls", 0);
         if (!using_spice) {
@@ -828,18 +828,18 @@ static int add_graphics_client(Monitor *mon, const QDict *qdict, QObject **ret_d
         return 0;
 #ifdef CONFIG_VNC
     } else if (strcmp(protocol, "vnc") == 0) {
-	int fd = monitor_get_fd(mon, fdname);
+        int fd = monitor_get_fd(mon, fdname, NULL);
         int skipauth = qdict_get_try_bool(qdict, "skipauth", 0);
-	vnc_display_add_client(NULL, fd, skipauth);
-	return 0;
+        vnc_display_add_client(NULL, fd, skipauth);
+        return 0;
 #endif
     } else if ((s = qemu_chr_find(protocol)) != NULL) {
-	int fd = monitor_get_fd(mon, fdname);
-	if (qemu_chr_add_client(s, fd) < 0) {
-	    qerror_report(QERR_ADD_CLIENT_FAILED);
-	    return -1;
-	}
-	return 0;
+        int fd = monitor_get_fd(mon, fdname, NULL);
+        if (qemu_chr_add_client(s, fd) < 0) {
+            qerror_report(QERR_ADD_CLIENT_FAILED);
+            return -1;
+        }
+        return 0;
     }
 
     qerror_report(QERR_INVALID_PARAMETER, "protocol");
@@ -2182,57 +2182,69 @@ static void do_inject_mce(Monitor *mon, const QDict *qdict)
 }
 #endif
 
-void qmp_getfd(const char *fdname, Error **errp)
+static int monitor_put_fd(Monitor *mon, const char *fdname, bool replace,
+                          bool copy, Error **errp)
 {
     mon_fd_t *monfd;
     int fd;
 
-    fd = qemu_chr_fe_get_msgfd(cur_mon->chr);
+    fd = qemu_chr_fe_get_msgfd(mon->chr);
     if (fd == -1) {
         error_set(errp, QERR_FD_NOT_SUPPLIED);
-        return;
+        return -1;
     }
 
     if (qemu_isdigit(fdname[0])) {
         error_set(errp, QERR_INVALID_PARAMETER_VALUE, "fdname",
                   "a name not starting with a digit");
-        return;
+        return -1;
     }
 
-    QLIST_FOREACH(monfd, &cur_mon->fds, next) {
+    QLIST_FOREACH(monfd, &mon->fds, next) {
         if (strcmp(monfd->name, fdname) != 0) {
             continue;
         }
 
-        close(monfd->fd);
-        monfd->fd = fd;
-        return;
+        if (replace) {
+            /* the existing fd is replaced with the new fd */
+            close(monfd->fd);
+            monfd->fd = fd;
+            return fd;
+        }
+
+        if (copy) {
+            /* the existing fd becomes a copy of the new fd */
+            if (dup2(fd, monfd->fd) == -1) {
+                error_set(errp, QERR_UNDEFINED_ERROR);
+                return -1;
+            }
+            close(fd);
+            return monfd->fd;
+        }
+
+        error_set(errp, QERR_INVALID_PARAMETER, "fdname");
+        return -1;
+    }
+
+    if (copy) {
+        error_set(errp, QERR_INVALID_PARAMETER, "fdname");
+        return -1;
     }
 
     monfd = g_malloc0(sizeof(mon_fd_t));
     monfd->name = g_strdup(fdname);
     monfd->fd = fd;
 
-    QLIST_INSERT_HEAD(&cur_mon->fds, monfd, next);
+    QLIST_INSERT_HEAD(&mon->fds, monfd, next);
+    return fd;
 }
 
 void qmp_closefd(const char *fdname, Error **errp)
 {
-    mon_fd_t *monfd;
-
-    QLIST_FOREACH(monfd, &cur_mon->fds, next) {
-        if (strcmp(monfd->name, fdname) != 0) {
-            continue;
-        }
-
-        QLIST_REMOVE(monfd, next);
-        close(monfd->fd);
-        g_free(monfd->name);
-        g_free(monfd);
-        return;
+    int fd = monitor_get_fd(cur_mon, fdname, errp);
+    if (fd != -1) {
+        close(fd);
     }
-
-    error_set(errp, QERR_FD_NOT_FOUND, fdname);
 }
 
 static void do_loadvm(Monitor *mon, const QDict *qdict)
@@ -2247,7 +2259,7 @@ static void do_loadvm(Monitor *mon, const QDict *qdict)
     }
 }
 
-int monitor_get_fd(Monitor *mon, const char *fdname)
+int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp)
 {
     mon_fd_t *monfd;
 
@@ -2268,9 +2280,25 @@ int monitor_get_fd(Monitor *mon, const char *fdname)
         return fd;
     }
 
+    error_set(errp, QERR_FD_NOT_FOUND, fdname);
     return -1;
 }
 
+int64_t qmp_pass_fd(const char *fdname, bool has_force, bool force,
+                    Error **errp)
+{
+    bool replace = false;
+    bool copy = has_force ? force : false;
+    return monitor_put_fd(cur_mon, fdname, replace, copy, errp);
+}
+
+void qmp_getfd(const char *fdname, Error **errp)
+{
+    bool replace = true;
+    bool copy = false;
+    monitor_put_fd(cur_mon, fdname, replace, copy, errp);
+}
+
 /* mon_cmds and info_cmds would be sorted at runtime */
 static mon_cmd_t mon_cmds[] = {
 #include "hmp-commands.h"
diff --git a/monitor.h b/monitor.h
index cd1d878..8fa515d 100644
--- a/monitor.h
+++ b/monitor.h
@@ -63,7 +63,7 @@ int monitor_read_block_device_key(Monitor *mon, const char *device,
                                   BlockDriverCompletionFunc *completion_cb,
                                   void *opaque);
 
-int monitor_get_fd(Monitor *mon, const char *fdname);
+int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp);
 
 void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
     GCC_FMT_ATTR(2, 0);
diff --git a/net.c b/net.c
index 4aa416c..28860b8 100644
--- a/net.c
+++ b/net.c
@@ -730,12 +730,14 @@ int qemu_find_nic_model(NICInfo *nd, const char * const *models,
 int net_handle_fd_param(Monitor *mon, const char *param)
 {
     int fd;
+    Error *local_err = NULL;
 
     if (!qemu_isdigit(param[0]) && mon) {
 
-        fd = monitor_get_fd(mon, param);
+        fd = monitor_get_fd(mon, param, &local_err);
         if (fd == -1) {
-            error_report("No file descriptor named %s found", param);
+            qerror_report_err(local_err);
+            error_free(local_err);
             return -1;
         }
     } else {
diff --git a/qapi-schema.json b/qapi-schema.json
index 26a6b84..2ac1261 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1864,6 +1864,41 @@
 { 'command': 'netdev_del', 'data': {'id': 'str'} }
 
 ##
+# @pass-fd:
+#
+# Pass a file descriptor via SCM rights and assign it a name
+#
+# @fdname: file descriptor name
+#
+# @force: #optional true specifies that an existing file descriptor is
+#         associated with @fdname, and that it should become a copy of the
+#         newly passed file descriptor.
+#
+# Returns: The QEMU file descriptor that is assigned to @fdname
+#          If file descriptor was not received, FdNotSupplied
+#          If @fdname is not valid, InvalidParameterValue
+#          If @fdname already exists, and @force is not true, InvalidParameter
+#          If @fdname does not already exist, and @force is true,
+#          InvalidParameter
+#          If @force fails to copy the passed file descriptor,
+#          UndefinedError
+#
+# Since: 1.2.0
+#
+# Notes: If @fdname already exists, and @force is not true, the
+#        command will fail.
+#
+#        If @fdname already exists, and @force is true, the value of
+#        the existing file descriptor is returned when the command is
+#        successful.
+#
+#        The 'closefd' command can be used to explicitly close the
+#        file descriptor when it is no longer needed.
+##
+{ 'command': 'pass-fd', 'data': {'fdname': 'str', '*force': 'bool'},
+  'returns': 'int' }
+
+##
 # @getfd:
 #
 # Receive a file descriptor via SCM rights and assign it a name
@@ -1879,6 +1914,7 @@
 # Notes: If @fdname already exists, the file descriptor assigned to
 #        it will be closed and replaced by the received file
 #        descriptor.
+#
 #        The 'closefd' command can be used to explicitly close the
 #        file descriptor when it is no longer needed.
 ##
diff --git a/qmp-commands.hx b/qmp-commands.hx
index e3cf3c5..7e3c07e 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -869,9 +869,49 @@ Example:
 EQMP
 
     {
+        .name       = "pass-fd",
+        .args_type  = "fdname:s,force:b?",
+        .params     = "pass-fd fdname force",
+        .help       = "pass a file descriptor via SCM rights and assign it a name",
+        .mhandler.cmd_new = qmp_marshal_input_pass_fd,
+    },
+
+SQMP
+pass-fd
+-------
+
+Pass a file descriptor via SCM rights and assign it a name.
+
+Arguments:
+
+- "fdname": file descriptor name (json-string)
+- "force": true specifies that an existing file descriptor is associated
+           with "fdname", and that it should become a copy of the newly
+           passed file descriptor. (json-bool, optional)
+
+Return a json-int with the QEMU file descriptor that is assigned to "fdname".
+
+Example:
+
+-> { "execute": "pass-fd", "arguments": { "fdname": "fd1" } }
+<- { "return": 42 }
+
+Notes:
+
+(1) If the name specified by the "fdname" argument already exists, and
+    "force" is not true, the command will fail.
+(2) If the name specified by the "fdname" argument already exists, and
+    "force" is true, the value of the existing file descriptor is
+    returned when the command is successful.
+(3) The 'closefd' command can be used to explicitly close the file
+    descriptor when it is no longer needed.
+
+EQMP
+
+    {
         .name       = "getfd",
         .args_type  = "fdname:s",
-        .params     = "getfd name",
+        .params     = "getfd fdname",
         .help       = "receive a file descriptor via SCM rights and assign it a name",
         .mhandler.cmd_new = qmp_marshal_input_getfd,
     },
-- 
1.7.10.2