[Spice-devel] [PATCH vdagent 11/11] vdagent: Use GMainLoop

Jakub Janků janku.jakub.jj at gmail.com
Tue Sep 26 20:59:47 UTC 2017


Replace our existing loop with GMainLoop.
Use GIOChannels with g_io_add_watch() to manage IO
from udscs and x11 connections in the main loop.

Add function spice_vdagent_write_msg()
which internally uses udscs_write(), but on top of that
calls g_io_add_watch(, G_IO_OUT, ,). Once the fd becomes
ready to write, conn_channel_io_cb() is called and next
message is sent using udscs_do_write().
When using udscs_write() instead of this new function,
messages might not be delivered.

Signals are also handled in the main loop,
using g_unix_signal_add().
SIGQUIT is currently not supported by GLib.

This enables further GTK+ integration in the future.

Original author: Victor Toso
---
 src/vdagent/file-xfers.c |  29 +++----
 src/vdagent/vdagent.c    | 194 ++++++++++++++++++++++++++++-------------------
 src/vdagent/vdagent.h    |  13 ++++
 src/vdagent/x11-randr.c  |   5 +-
 src/vdagent/x11.c        |  29 +++----
 5 files changed, 164 insertions(+), 106 deletions(-)

diff --git a/src/vdagent/file-xfers.c b/src/vdagent/file-xfers.c
index 4d76003..266af5f 100644
--- a/src/vdagent/file-xfers.c
+++ b/src/vdagent/file-xfers.c
@@ -39,6 +39,7 @@
 
 #include "vdagentd-proto.h"
 #include "file-xfers.h"
+#include "vdagent.h"
 
 struct vdagent_file_xfers {
     GHashTable *xfers;
@@ -221,12 +222,12 @@ void vdagent_file_xfers_start(struct vdagent_file_xfers *xfers,
         g_free(free_space_str);
         g_free(file_size_str);
 
-        udscs_write(xfers->vdagentd,
-                    VDAGENTD_FILE_XFER_STATUS,
-                    msg->id,
-                    VD_AGENT_FILE_XFER_STATUS_NOT_ENOUGH_SPACE,
-                    (uint8_t *)&free_space,
-                    sizeof(free_space));
+        spice_vdagent_write_msg(xfers->vdagentd,
+                                VDAGENTD_FILE_XFER_STATUS,
+                                msg->id,
+                                VD_AGENT_FILE_XFER_STATUS_NOT_ENOUGH_SPACE,
+                                (uint8_t *)&free_space,
+                                sizeof(free_space));
         vdagent_file_xfer_task_free(task);
         g_free(file_path);
         g_free(dir);
@@ -274,15 +275,15 @@ void vdagent_file_xfers_start(struct vdagent_file_xfers *xfers,
         syslog(LOG_DEBUG, "file-xfer: Adding task %u %s %"PRIu64" bytes",
                task->id, path, task->file_size);
 
-    udscs_write(xfers->vdagentd, VDAGENTD_FILE_XFER_STATUS,
-                msg->id, VD_AGENT_FILE_XFER_STATUS_CAN_SEND_DATA, NULL, 0);
+    spice_vdagent_write_msg(xfers->vdagentd, VDAGENTD_FILE_XFER_STATUS,
+                            msg->id, VD_AGENT_FILE_XFER_STATUS_CAN_SEND_DATA, NULL, 0);
     g_free(file_path);
     g_free(dir);
     return ;
 
 error:
-    udscs_write(xfers->vdagentd, VDAGENTD_FILE_XFER_STATUS,
-                msg->id, VD_AGENT_FILE_XFER_STATUS_ERROR, NULL, 0);
+    spice_vdagent_write_msg(xfers->vdagentd, VDAGENTD_FILE_XFER_STATUS,
+                            msg->id, VD_AGENT_FILE_XFER_STATUS_ERROR, NULL, 0);
     if (task)
         vdagent_file_xfer_task_free(task);
     g_free(file_path);
@@ -353,8 +354,8 @@ void vdagent_file_xfers_data(struct vdagent_file_xfers *xfers,
     }
 
     if (status != -1) {
-        udscs_write(xfers->vdagentd, VDAGENTD_FILE_XFER_STATUS,
-                    msg->id, status, NULL, 0);
+        spice_vdagent_write_msg(xfers->vdagentd, VDAGENTD_FILE_XFER_STATUS,
+                                msg->id, status, NULL, 0);
         g_hash_table_remove(xfers->xfers, GUINT_TO_POINTER(msg->id));
     }
 }
@@ -363,6 +364,6 @@ void vdagent_file_xfers_error_disabled(struct udscs_connection *vdagentd, uint32
 {
     g_return_if_fail(vdagentd != NULL);
 
-    udscs_write(vdagentd, VDAGENTD_FILE_XFER_STATUS,
-                msg_id, VD_AGENT_FILE_XFER_STATUS_DISABLED, NULL, 0);
+    spice_vdagent_write_msg(vdagentd, VDAGENTD_FILE_XFER_STATUS,
+                            msg_id, VD_AGENT_FILE_XFER_STATUS_DISABLED, NULL, 0);
 }
diff --git a/src/vdagent/vdagent.c b/src/vdagent/vdagent.c
index cdbda8b..6a5204a 100644
--- a/src/vdagent/vdagent.c
+++ b/src/vdagent/vdagent.c
@@ -34,8 +34,8 @@
 #include <sys/select.h>
 #include <sys/stat.h>
 #include <spice/vd_agent.h>
-#include <glib.h>
 #include <poll.h>
+#include <glib-unix.h>
 
 #include "vdagentd-proto.h"
 #include "vdagentd-proto-strings.h"
@@ -44,7 +44,6 @@
 
 G_DEFINE_TYPE (SpiceVDAgent, spice_vdagent, G_TYPE_OBJECT);
 
-static int quit = 0;
 static int version_mismatch = 0;
 
 /* Command line options */
@@ -164,7 +163,7 @@ static void daemon_read_complete(struct udscs_connection **connp,
         if (strcmp((char *)data, VERSION) != 0) {
             syslog(LOG_INFO, "vdagentd version mismatch: got %s expected %s",
                    data, VERSION);
-            udscs_destroy_connection(connp);
+            g_main_loop_quit(agent->loop);
             version_mismatch = 1;
         }
         break;
@@ -222,27 +221,6 @@ static void daemon_read_complete(struct udscs_connection **connp,
     }
 }
 
-static struct udscs_connection *client_setup_sync(SpiceVDAgent *agent)
-{
-    struct udscs_connection *conn = NULL;
-
-    while (!quit) {
-        conn = udscs_connect(vdagentd_socket, daemon_read_complete, agent,
-                             NULL, vdagentd_messages, VDAGENTD_NO_MESSAGES,
-                             debug);
-        if (conn || !do_daemonize || quit) {
-            break;
-        }
-        sleep(1);
-    }
-    return conn;
-}
-
-static void quit_handler(int sig)
-{
-    quit = 1;
-}
-
 /* When we daemonize, it is useful to have the main process
    wait to make sure the X connection worked.  We wait up
    to 10 seconds to get an 'all clear' from the child
@@ -298,13 +276,86 @@ static int file_test(const char *path)
     return stat(path, &buffer);
 }
 
+static gboolean x11_channel_io_cb(GIOChannel *source,
+                                  GIOCondition condition,
+                                  gpointer data)
+{
+    SpiceVDAgent *agent = data;
+    vdagent_x11_do_read(agent->x11);
+
+    return G_SOURCE_CONTINUE;
+}
+
+static gboolean conn_channel_io_cb(GIOChannel *source,
+                                   GIOCondition condition,
+                                   gpointer data)
+{
+    SpiceVDAgent *agent = data;
+
+    if (condition & G_IO_IN) {
+        udscs_do_read(&agent->conn);
+        if (agent->conn == NULL)
+            goto err;
+        return G_SOURCE_CONTINUE;
+    }
+    if (condition & G_IO_OUT) {
+        udscs_do_write(&agent->conn);
+        if (agent->conn == NULL)
+            goto err;
+        if (udscs_is_write_pending(agent->conn))
+            return G_SOURCE_CONTINUE;
+        agent->conn_out_source_id = 0;
+        return G_SOURCE_REMOVE;
+    }
+
+err:
+    g_main_loop_quit(agent->loop);
+    return G_SOURCE_REMOVE;
+}
+
+void spice_vdagent_write_msg(struct udscs_connection *conn, uint32_t type, uint32_t arg1,
+                             uint32_t arg2, const uint8_t *data, uint32_t size)
+{
+    if (udscs_write(conn, type, arg1, arg2, data, size) == -1)
+        return;
+
+    SpiceVDAgent *agent = udscs_get_user_data(conn);
+
+    if (agent->conn_out_source_id)
+        return;
+    agent->conn_out_source_id =
+        g_io_add_watch(agent->conn_channel,
+                       G_IO_OUT | G_IO_ERR | G_IO_NVAL,
+                       conn_channel_io_cb,
+                       agent);
+}
+
+gboolean session_agent_signal_handler(gpointer user_data)
+{
+    SpiceVDAgent *agent = user_data;
+    do_daemonize = FALSE;
+    g_main_loop_quit(agent->loop);
+    return G_SOURCE_REMOVE;
+}
+
 static void spice_vdagent_init(SpiceVDAgent *self)
 {
+    self->loop = g_main_loop_new(NULL, FALSE);
+
+    g_unix_signal_add(SIGINT, session_agent_signal_handler, self);
+    g_unix_signal_add(SIGHUP, session_agent_signal_handler, self);
+    g_unix_signal_add(SIGTERM, session_agent_signal_handler, self);
 }
 
 static void spice_vdagent_finalize(GObject *gobject)
 {
     SpiceVDAgent *self = SPICE_VDAGENT(gobject);
+
+    g_clear_pointer(&self->x11_channel, g_io_channel_unref);
+    g_clear_pointer(&self->conn_channel, g_io_channel_unref);
+
+    g_clear_pointer(&self->loop, g_main_loop_unref);
+
     vdagent_finalize_file_xfer(self);
     vdagent_x11_destroy(self->x11, self->conn == NULL);
 
@@ -323,30 +374,60 @@ static void spice_vdagent_class_init(SpiceVDAgentClass  *klass)
     gobject_class->finalize = spice_vdagent_finalize;
 }
 
-static gboolean spice_vdagent_init_sync(SpiceVDAgent *agent)
+static gboolean spice_vdagent_init_async_cb(gpointer user_data)
 {
-    g_return_val_if_fail(SPICE_IS_VDAGENT(agent), FALSE);
+    g_return_val_if_fail(SPICE_IS_VDAGENT(user_data), G_SOURCE_REMOVE);
+    SpiceVDAgent *agent = user_data;
+    g_return_val_if_fail(agent->conn == NULL, G_SOURCE_REMOVE);
 
-    agent->conn = client_setup_sync(agent);
+    agent->conn = udscs_connect(vdagentd_socket, daemon_read_complete, agent,
+                                NULL, vdagentd_messages, VDAGENTD_NO_MESSAGES,
+                                debug);
     if (agent->conn == NULL)
-        return FALSE;
+        return G_SOURCE_CONTINUE;
+    udscs_set_user_data(agent->conn, agent);
+
+    agent->conn_channel = g_io_channel_unix_new(udscs_client_get_fd(agent->conn));
+    if (agent->conn_channel == NULL)
+        goto err_init;
 
     agent->x11 = vdagent_x11_create(agent->conn, debug, x11_sync);
     if (agent->x11 == NULL)
-        return FALSE;
+        goto err_init;
+    agent->x11_channel = g_io_channel_unix_new(vdagent_x11_get_fd(agent->x11));
+    if (agent->x11_channel == NULL)
+        goto err_init;
+
+    g_io_add_watch(agent->conn_channel,
+                   G_IO_IN | G_IO_ERR | G_IO_NVAL,
+                   conn_channel_io_cb,
+                   agent);
+    g_io_add_watch(agent->x11_channel,
+                   G_IO_IN,
+                   x11_channel_io_cb,
+                   agent);
 
     if (!vdagent_init_file_xfer(agent))
         syslog(LOG_WARNING, "File transfer is disabled");
 
-    return TRUE;
+    if (agent->parent_socket) {
+        if (write(agent->parent_socket, "OK", 2) != 2)
+            syslog(LOG_WARNING, "Parent already gone.");
+        close(agent->parent_socket);
+        agent->parent_socket = 0;
+    }
+
+    return G_SOURCE_REMOVE;
+
+err_init:
+    g_main_loop_quit(agent->loop);
+    do_daemonize = FALSE;
+    return G_SOURCE_REMOVE;
 }
 
 int main(int argc, char *argv[])
 {
-    fd_set readfds, writefds;
-    int n, nfds, x11_fd;
     int parent_socket = 0;
-    struct sigaction act;
     GOptionContext *context;
     GError *error = NULL;
     SpiceVDAgent *agent;
@@ -374,14 +455,6 @@ int main(int argc, char *argv[])
         vdagentd_socket = g_strdup(VDAGENTD_SOCKET);
     }
 
-    memset(&act, 0, sizeof(act));
-    act.sa_flags = SA_RESTART;
-    act.sa_handler = quit_handler;
-    sigaction(SIGINT, &act, NULL);
-    sigaction(SIGHUP, &act, NULL);
-    sigaction(SIGTERM, &act, NULL);
-    sigaction(SIGQUIT, &act, NULL);
-
     openlog("spice-vdagent", do_daemonize ? LOG_PID : (LOG_PID | LOG_PERROR),
             LOG_USER);
 
@@ -402,44 +475,13 @@ reconnect:
 
     agent = g_object_new(SPICE_TYPE_VDAGENT, NULL);
     agent->parent_socket = parent_socket;
-    if (!spice_vdagent_init_sync(agent)) {
-        quit = 1;
-        goto end_main;
-    }
-
-    if (parent_socket) {
-        if (write(parent_socket, "OK", 2) != 2)
-            syslog(LOG_WARNING, "Parent already gone.");
-        close(parent_socket);
-        parent_socket = 0;
-    }
-
-    while (agent->conn && !quit) {
-        FD_ZERO(&readfds);
-        FD_ZERO(&writefds);
-
-        nfds = udscs_client_fill_fds(agent->conn, &readfds, &writefds);
-        x11_fd = vdagent_x11_get_fd(agent->x11);
-        FD_SET(x11_fd, &readfds);
-        if (x11_fd >= nfds)
-            nfds = x11_fd + 1;
-
-        n = select(nfds, &readfds, &writefds, NULL, NULL);
-        if (n == -1) {
-            if (errno == EINTR)
-                continue;
-            syslog(LOG_ERR, "Fatal error select: %s", strerror(errno));
-            break;
-        }
+    g_timeout_add_seconds(1, spice_vdagent_init_async_cb, agent);
 
-        if (FD_ISSET(x11_fd, &readfds))
-            vdagent_x11_do_read(agent->x11);
-        udscs_client_handle_fds(&agent->conn, &readfds, &writefds);
-    }
+    g_main_loop_run(agent->loop);
 
-end_main:
     g_clear_object(&agent);
-    if (!quit && do_daemonize)
+
+    if (do_daemonize)
         goto reconnect;
 
     g_free(fx_dir);
diff --git a/src/vdagent/vdagent.h b/src/vdagent/vdagent.h
index f98c9b2..66ac208 100644
--- a/src/vdagent/vdagent.h
+++ b/src/vdagent/vdagent.h
@@ -20,6 +20,7 @@
 #define __VDAGENT_H_
 
 #include <glib-object.h>
+#include <glib.h>
 #include "udscs.h"
 #include "x11.h"
 #include "file-xfers.h"
@@ -40,6 +41,11 @@ typedef struct _SpiceVDAgent {
     struct vdagent_file_xfers      *xfers;
     struct udscs_connection        *conn;
 
+    GMainLoop                      *loop;
+    GIOChannel                     *conn_channel;
+    GIOChannel                     *x11_channel;
+    guint                           conn_out_source_id;
+
     int                             parent_socket;
 } SpiceVDAgent;
 
@@ -49,6 +55,13 @@ typedef struct _SpiceVDAgentClass {
 
 GType spice_vdagent_get_type(void);
 
+/* Queue a message for delivery to the client connected through conn.
+ * This function must be used on the vdagent side instead of udscs_write()
+ * for the agent to work correctly.
+ */
+void spice_vdagent_write_msg(struct udscs_connection *conn, uint32_t type, uint32_t arg1,
+                             uint32_t arg2, const uint8_t *data, uint32_t size);
+
 G_END_DECLS
 
 #endif
\ No newline at end of file
diff --git a/src/vdagent/x11-randr.c b/src/vdagent/x11-randr.c
index aade5ca..ffbab90 100644
--- a/src/vdagent/x11-randr.c
+++ b/src/vdagent/x11-randr.c
@@ -33,6 +33,7 @@
 #include "vdagentd-proto.h"
 #include "x11.h"
 #include "x11-priv.h"
+#include "vdagent.h"
 
 #define MM_PER_INCH (25.4)
 
@@ -972,8 +973,8 @@ no_info:
                    res[i].height, res[i].x, res[i].y);
     }
 
-    udscs_write(x11->vdagentd, VDAGENTD_GUEST_XORG_RESOLUTION, width, height,
-                (uint8_t *)res, screen_count * sizeof(*res));
+    spice_vdagent_write_msg(x11->vdagentd, VDAGENTD_GUEST_XORG_RESOLUTION, width, height,
+                            (uint8_t *)res, screen_count * sizeof(*res));
     free(res);
     return;
 no_mem:
diff --git a/src/vdagent/x11.c b/src/vdagent/x11.c
index 6e47ea1..e6dd196 100644
--- a/src/vdagent/x11.c
+++ b/src/vdagent/x11.c
@@ -44,6 +44,7 @@
 #include "vdagentd-proto.h"
 #include "x11.h"
 #include "x11-priv.h"
+#include "vdagent.h"
 
 /* Stupid X11 API, there goes our encapsulate all data in a struct design */
 int (*vdagent_x11_prev_error_handler)(Display *, XErrorEvent *);
@@ -384,8 +385,8 @@ static void vdagent_x11_set_clipboard_owner(struct vdagent_x11 *x11,
                 once = 0;
             }
             if (x11->vdagentd)
-                udscs_write(x11->vdagentd, VDAGENTD_CLIPBOARD_DATA, selection,
-                            VD_AGENT_CLIPBOARD_NONE, NULL, 0);
+                spice_vdagent_write_msg(x11->vdagentd, VDAGENTD_CLIPBOARD_DATA, selection,
+                                        VD_AGENT_CLIPBOARD_NONE, NULL, 0);
             if (curr_conv == x11->conversion_req) {
                 x11->conversion_req = next_conv;
                 x11->clipboard_data_size = 0;
@@ -403,8 +404,8 @@ static void vdagent_x11_set_clipboard_owner(struct vdagent_x11 *x11,
         /* When going from owner_guest to owner_none we need to send a
            clipboard release message to the client */
         if (x11->clipboard_owner[selection] == owner_guest && x11->vdagentd) {
-            udscs_write(x11->vdagentd, VDAGENTD_CLIPBOARD_RELEASE, selection,
-                        0, NULL, 0);
+            spice_vdagent_write_msg(x11->vdagentd, VDAGENTD_CLIPBOARD_RELEASE, selection,
+                                    0, NULL, 0);
         }
         x11->clipboard_type_count[selection] = 0;
     }
@@ -851,8 +852,8 @@ static void vdagent_x11_handle_selection_notify(struct vdagent_x11 *x11,
         len = 0;
     }
 
-    udscs_write(x11->vdagentd, VDAGENTD_CLIPBOARD_DATA, selection, type,
-                data, len);
+    spice_vdagent_write_msg(x11->vdagentd, VDAGENTD_CLIPBOARD_DATA, selection, type,
+                            data, len);
     vdagent_x11_get_selection_free(x11, data, incr);
 
     vdagent_x11_next_conversion_request(x11);
@@ -935,10 +936,10 @@ static void vdagent_x11_handle_targets_notify(struct vdagent_x11 *x11,
     }
 
     if (*type_count) {
-        udscs_write(x11->vdagentd, VDAGENTD_CLIPBOARD_GRAB, selection, 0,
-                    (uint8_t *)x11->clipboard_agent_types[selection],
-                    *type_count * sizeof(uint32_t));
-        vdagent_x11_set_clipboard_owner(x11, selection, owner_guest);
+        spice_vdagent_write_msg(x11->vdagentd, VDAGENTD_CLIPBOARD_GRAB, selection, 0,
+                                (uint8_t *)x11->clipboard_agent_types[selection],
+                                *type_count * sizeof(uint32_t));
+                                vdagent_x11_set_clipboard_owner(x11, selection, owner_guest);
     }
 
     vdagent_x11_get_selection_free(x11, (unsigned char *)atoms, 0);
@@ -1066,8 +1067,8 @@ static void vdagent_x11_handle_selection_request(struct vdagent_x11 *x11)
         return;
     }
 
-    udscs_write(x11->vdagentd, VDAGENTD_CLIPBOARD_REQUEST, selection, type,
-                NULL, 0);
+    spice_vdagent_write_msg(x11->vdagentd, VDAGENTD_CLIPBOARD_REQUEST, selection, type,
+                            NULL, 0);
 }
 
 static void vdagent_x11_handle_property_delete_notify(struct vdagent_x11 *x11,
@@ -1174,8 +1175,8 @@ void vdagent_x11_clipboard_request(struct vdagent_x11 *x11,
     return;
 
 none:
-    udscs_write(x11->vdagentd, VDAGENTD_CLIPBOARD_DATA,
-                selection, VD_AGENT_CLIPBOARD_NONE, NULL, 0);
+    spice_vdagent_write_msg(x11->vdagentd, VDAGENTD_CLIPBOARD_DATA,
+                            selection, VD_AGENT_CLIPBOARD_NONE, NULL, 0);
 }
 
 void vdagent_x11_clipboard_grab(struct vdagent_x11 *x11, uint8_t selection,
-- 
2.13.5



More information about the Spice-devel mailing list