[Spice-devel] [PATCH vdagent v3 7/8] vdagent: Use GMainLoop

Jakub Janků janku.jakub.jj at gmail.com
Tue Oct 10 21:21:52 UTC 2017


Replace existing main while-loop with GMainLoop.

Use GIOChannel with g_io_add_watch() to manage IO
from x11 connections in the main loop.

udscs_connect() now internally creates GSources
by calling g_io_add_watch().
This enables GMainLoop integration,
clients no longer need to call
udscs_client_fill_fds() and
udscs_client_handle_fds() in a loop.
Read callback and udscs_write() functions as before.

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.

Signed-off-by: Victor Toso <victortoso at redhat.com>
---
 src/udscs.c           |  60 ++++++++++++++++++++
 src/vdagent/vdagent.c | 148 ++++++++++++++++++++++++++------------------------
 src/vdagent/vdagent.h |   8 +++
 3 files changed, 145 insertions(+), 71 deletions(-)

diff --git a/src/udscs.c b/src/udscs.c
index 8b16f89..3a1ea44 100644
--- a/src/udscs.c
+++ b/src/udscs.c
@@ -31,6 +31,8 @@
 #include <errno.h>
 #include <sys/socket.h>
 #include <sys/un.h>
+#include <glib.h>
+#include <glib-unix.h>
 #include "udscs.h"
 
 struct udscs_buf {
@@ -66,8 +68,16 @@ struct udscs_connection {
 
     struct udscs_connection *next;
     struct udscs_connection *prev;
+
+    GIOChannel                     *io_channel;
+    guint                           write_watch_id;
+    guint                           read_watch_id;
 };
 
+static gboolean udscs_io_channel_cb(GIOChannel *source,
+                                    GIOCondition condition,
+                                    gpointer data);
+
 struct udscs_connection *udscs_connect(const char *socketname,
     udscs_read_callback read_callback,
     udscs_disconnect_callback disconnect_callback,
@@ -103,6 +113,17 @@ struct udscs_connection *udscs_connect(const char *socketname,
         return NULL;
     }
 
+    conn->io_channel = g_io_channel_unix_new(conn->fd);
+    if (!conn->io_channel) {
+        udscs_destroy_connection(&conn);
+        return NULL;
+    }
+    conn->read_watch_id =
+        g_io_add_watch(conn->io_channel,
+                       G_IO_IN | G_IO_ERR | G_IO_NVAL,
+                       udscs_io_channel_cb,
+                       conn);
+
     conn->read_callback = read_callback;
     conn->disconnect_callback = disconnect_callback;
 
@@ -141,6 +162,12 @@ void udscs_destroy_connection(struct udscs_connection **connp)
 
     close(conn->fd);
 
+    if (conn->write_watch_id != 0)
+        g_source_remove(conn->write_watch_id);
+    if (conn->read_watch_id != 0)
+        g_source_remove(conn->read_watch_id);
+    g_clear_pointer(&conn->io_channel, g_io_channel_unref);
+
     if (conn->debug)
         syslog(LOG_DEBUG, "%p disconnected", conn);
 
@@ -198,6 +225,13 @@ int udscs_write(struct udscs_connection *conn, uint32_t type, uint32_t arg1,
                    conn, type, arg1, arg2, size);
     }
 
+    if (conn->io_channel && conn->write_watch_id == 0)
+        conn->write_watch_id =
+            g_io_add_watch(conn->io_channel,
+                           G_IO_OUT | G_IO_ERR | G_IO_NVAL,
+                           udscs_io_channel_cb,
+                           conn);
+
     if (!conn->write_buf) {
         conn->write_buf = new_wbuf;
         return 0;
@@ -353,6 +387,32 @@ int udscs_client_fill_fds(struct udscs_connection *conn, fd_set *readfds,
     return conn->fd + 1;
 }
 
+static gboolean udscs_io_channel_cb(GIOChannel *source,
+                                    GIOCondition condition,
+                                    gpointer data)
+{
+    struct udscs_connection *conn = data;
+
+    if (condition & G_IO_IN) {
+        udscs_do_read(&conn);
+        if (conn == NULL)
+            return G_SOURCE_REMOVE;
+        return G_SOURCE_CONTINUE;
+    }
+    if (condition & G_IO_OUT) {
+        udscs_do_write(&conn);
+        if (conn == NULL)
+            return G_SOURCE_REMOVE;
+        if (conn->write_buf)
+            return G_SOURCE_CONTINUE;
+        conn->write_watch_id = 0;
+        return G_SOURCE_REMOVE;
+    }
+
+    udscs_destroy_connection(&conn);
+    return G_SOURCE_REMOVE;
+}
+
 
 #ifndef UDSCS_NO_SERVER
 
diff --git a/src/vdagent/vdagent.c b/src/vdagent/vdagent.c
index 462b5fe..3474f17 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 (VDAgent, vdagent, G_TYPE_OBJECT);
 
-static int quit = 0;
 static int version_mismatch = 0;
 
 /* Command line options */
@@ -170,7 +169,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;
@@ -228,25 +227,12 @@ static void daemon_read_complete(struct udscs_connection **connp,
     }
 }
 
-static struct udscs_connection *client_setup_sync(VDAgent *agent)
+static void daemon_disconnect_cb(struct udscs_connection *conn)
 {
-    struct udscs_connection *conn = NULL;
-
-    while (!quit) {
-        conn = udscs_connect(vdagentd_socket, daemon_read_complete, 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;
+    VDAgent *agent = udscs_get_user_data(conn);
+    agent->conn = NULL;
+    if (agent->loop)
+        g_main_loop_quit(agent->loop);
 }
 
 /* When we daemonize, it is useful to have the main process
@@ -304,17 +290,48 @@ static int file_test(const char *path)
     return stat(path, &buffer);
 }
 
+static gboolean x11_io_channel_cb(GIOChannel *source,
+                                  GIOCondition condition,
+                                  gpointer data)
+{
+    VDAgent *agent = data;
+    vdagent_x11_do_read(agent->x11);
+
+    return G_SOURCE_CONTINUE;
+}
+
+gboolean vdagent_signal_handler(gpointer user_data)
+{
+    VDAgent *agent = user_data;
+    agent->quit = TRUE;
+    g_main_loop_quit(agent->loop);
+    return G_SOURCE_REMOVE;
+}
+
 static void vdagent_init(VDAgent *self)
 {
+    self->loop = g_main_loop_new(NULL, FALSE);
+
+    g_unix_signal_add(SIGINT, vdagent_signal_handler, self);
+    g_unix_signal_add(SIGHUP, vdagent_signal_handler, self);
+    g_unix_signal_add(SIGTERM, vdagent_signal_handler, self);
 }
 
 static void vdagent_finalize(GObject *gobject)
 {
     VDAgent *self = VDAGENT(gobject);
+
     vdagent_finalize_file_xfer(self);
     vdagent_x11_destroy(self->x11, self->conn == NULL);
     udscs_destroy_connection(&self->conn);
 
+    if (self->timer_source_id > 0)
+        g_source_remove(self->timer_source_id);
+    if (self->x11_io_watch_id > 0)
+        g_source_remove(self->x11_io_watch_id);
+    g_clear_pointer(&self->x11_channel, g_io_channel_unref);
+    g_clear_pointer(&self->loop, g_main_loop_unref);
+
     G_OBJECT_CLASS(vdagent_parent_class)->finalize(gobject);
 }
 
@@ -324,29 +341,53 @@ static void vdagent_class_init(VDAgentClass  *klass)
     gobject_class->finalize = vdagent_finalize;
 }
 
-static gboolean vdagent_init_sync(VDAgent *agent)
+static gboolean vdagent_init_async_cb(gpointer user_data)
 {
-    agent->conn = client_setup_sync(agent);
+    VDAgent *agent = user_data;
+
+    agent->conn = udscs_connect(vdagentd_socket,
+                                daemon_read_complete, daemon_disconnect_cb,
+                                vdagentd_messages, VDAGENTD_NO_MESSAGES, debug);
     if (agent->conn == NULL)
-        return FALSE;
+        return G_SOURCE_CONTINUE;
     udscs_set_user_data(agent->conn, agent);
 
     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;
+
+    agent->x11_io_watch_id =
+        g_io_add_watch(agent->x11_channel,
+                       G_IO_IN,
+                       x11_io_channel_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;
+    }
+
+    agent->timer_source_id = 0;
+    return G_SOURCE_REMOVE;
+
+err_init:
+    g_main_loop_quit(agent->loop);
+    agent->timer_source_id = 0;
+    agent->quit = TRUE;
+    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;
     VDAgent *agent;
@@ -372,14 +413,6 @@ int main(int argc, char *argv[])
     if (vdagentd_socket == NULL)
         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);
 
@@ -400,45 +433,18 @@ reconnect:
 
     agent = g_object_new(TYPE_VDAGENT, NULL);
     agent->parent_socket = parent_socket;
-    if (!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;
-    }
+    if (vdagent_init_async_cb(agent) == G_SOURCE_CONTINUE)
+        agent->timer_source_id = g_timeout_add_seconds(1, vdagent_init_async_cb, agent);
 
-    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;
-        }
+    if (!agent->quit)
+        g_main_loop_run(agent->loop);
 
-        if (FD_ISSET(x11_fd, &readfds))
-            vdagent_x11_do_read(agent->x11);
-        udscs_client_handle_fds(&agent->conn, &readfds, &writefds);
+    if (!agent->quit && do_daemonize) {
+        g_clear_object(&agent);
+        goto reconnect;
     }
-
-end_main:
     g_clear_object(&agent);
-    if (!quit && do_daemonize)
-        goto reconnect;
 
     g_free(fx_dir);
     g_free(portdev);
diff --git a/src/vdagent/vdagent.h b/src/vdagent/vdagent.h
index 8376315..cd1d788 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"
@@ -39,8 +40,15 @@ typedef struct VDAgent {
     struct vdagent_x11             *x11;
     struct vdagent_file_xfers      *xfers;
     struct udscs_connection        *conn;
+    GIOChannel                     *x11_channel;
+
+    GMainLoop                      *loop;
+    guint                           timer_source_id;
+    guint                           x11_io_watch_id;
 
     int                             parent_socket;
+
+    gboolean                        quit;
 } VDAgent;
 
 typedef struct VDAgentClass {
-- 
2.13.6



More information about the Spice-devel mailing list