[Spice-devel] [RFC spice-vdagent 03/18] vdagentd: use GMainLoop
Victor Toso
victortoso at redhat.com
Tue Aug 28 07:38:56 UTC 2018
Hi,
On Tue, Aug 14, 2018 at 08:53:37PM +0200, Jakub Janků wrote:
> This is purely a preparatory patch as it renders
> the vdagentd non-functional.
I would rather not break it unless it really helps a lot. It was
possible to do it for vdagent code at 3fcf2e944ae3bf7, not sure
why we can't here.
If it is really the case that it is better to have a commit with
non functional vdagentd, the commit log must state the rationale
behind it and which commit it is expected to have it working
again (the immediate follow-up is better, I guess)
Reviewed-by: Victor Toso <victortoso at redhat.com>
Victor
> Remove main while loop with FD polling.
>
> udscs, virtio-port and session-info will be integrated
> into the GMainLoop in the following commits.
>
> Use g_unix_signal_add() to handle SIGINT, SIGHUP, SIGTERM.
> SIGQUIT handling is not supported by GLib.
> ---
> src/vdagentd/vdagentd.c | 129 ++++++++++++++--------------------------
> 1 file changed, 44 insertions(+), 85 deletions(-)
>
> diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c
> index d88bbc7..8abc63c 100644
> --- a/src/vdagentd/vdagentd.c
> +++ b/src/vdagentd/vdagentd.c
> @@ -31,10 +31,9 @@
> #include <errno.h>
> #include <signal.h>
> #include <syslog.h>
> -#include <sys/select.h>
> #include <sys/stat.h>
> #include <spice/vd_agent.h>
> -#include <glib.h>
> +#include <glib-unix.h>
>
> #ifdef WITH_SYSTEMD_SOCKET_ACTIVATION
> #include <systemd/sd-daemon.h>
> @@ -82,11 +81,12 @@ static const char *active_session = NULL;
> static unsigned int session_count = 0;
> static struct udscs_connection *active_session_conn = NULL;
> static int agent_owns_clipboard[256] = { 0, };
> -static int quit = 0;
> static int retval = 0;
> static int client_connected = 0;
> static int max_clipboard = -1;
>
> +static GMainLoop *loop;
> +
> /* utility functions */
> static void virtio_msg_uint32_to_le(uint8_t *_msg, uint32_t size, uint32_t offset)
> {
> @@ -175,7 +175,7 @@ void do_client_mouse(struct vdagentd_uinput **uinputp, VDAgentMouseState *mouse)
> if (!*uinputp) {
> syslog(LOG_CRIT, "Fatal uinput error");
> retval = 1;
> - quit = 1;
> + g_main_loop_quit(loop);
> }
> }
> }
> @@ -588,6 +588,33 @@ static int virtio_port_read_complete(
> return 0;
> }
>
> +static void virtio_port_disconnect_cb(struct vdagent_virtio_port *vport,
> + gboolean by_user)
> +{
> + virtio_port = NULL;
> +
> + if (!g_main_loop_is_running(loop))
> + return;
> +
> + if (by_user == FALSE) {
> + /* virtio_port was destroyed because of an internal error */
> + gboolean old_client_connected = client_connected;
> + syslog(LOG_CRIT, "AIIEEE lost spice client connection, reconnecting");
> + virtio_port = vdagent_virtio_port_create(portdev,
> + virtio_port_read_complete,
> + virtio_port_disconnect_cb);
> + if (virtio_port == NULL) {
> + syslog(LOG_CRIT, "Fatal error opening vdagent virtio channel");
> + retval = 1;
> + g_main_loop_quit(loop);
> + return;
> + }
> + do_client_disconnect();
> + client_connected = old_client_connected;
> + } else if (only_once)
> + g_main_loop_quit(loop);
> +}
> +
> static void virtio_write_clipboard(uint8_t selection, uint32_t msg_type,
> uint32_t data_type, uint8_t *data, uint32_t data_size)
> {
> @@ -727,7 +754,7 @@ static void check_xorg_resolution(void)
> if (!uinput) {
> syslog(LOG_CRIT, "Fatal uinput error");
> retval = 1;
> - quit = 1;
> + g_main_loop_quit(loop);
> return;
> }
>
> @@ -735,11 +762,11 @@ static void check_xorg_resolution(void)
> syslog(LOG_INFO, "opening vdagent virtio channel");
> virtio_port = vdagent_virtio_port_create(portdev,
> virtio_port_read_complete,
> - NULL);
> + virtio_port_disconnect_cb);
> if (!virtio_port) {
> syslog(LOG_CRIT, "Fatal error opening vdagent virtio channel");
> retval = 1;
> - quit = 1;
> + g_main_loop_quit(loop);
> return;
> }
> send_capabilities(virtio_port, 1);
> @@ -992,76 +1019,10 @@ static void daemonize(void)
> }
> }
>
> -static void main_loop(void)
> +static gboolean signal_handler(gpointer user_data)
> {
> - fd_set readfds, writefds;
> - int n, nfds;
> - int ck_fd = 0;
> - int once = 0;
> -
> - while (!quit) {
> - FD_ZERO(&readfds);
> - FD_ZERO(&writefds);
> -
> - nfds = udscs_server_fill_fds(server, &readfds, &writefds);
> - n = vdagent_virtio_port_fill_fds(virtio_port, &readfds, &writefds);
> - if (n >= nfds)
> - nfds = n + 1;
> -
> - if (session_info) {
> - ck_fd = session_info_get_fd(session_info);
> - FD_SET(ck_fd, &readfds);
> - if (ck_fd >= nfds)
> - nfds = ck_fd + 1;
> - }
> -
> - n = select(nfds, &readfds, &writefds, NULL, NULL);
> - if (n == -1) {
> - if (errno == EINTR)
> - continue;
> - syslog(LOG_CRIT, "Fatal error select: %m");
> - retval = 1;
> - break;
> - }
> -
> - udscs_server_handle_fds(server, &readfds, &writefds);
> -
> - if (virtio_port) {
> - once = 1;
> - vdagent_virtio_port_handle_fds(&virtio_port, &readfds, &writefds);
> - if (!virtio_port) {
> - int old_client_connected = client_connected;
> - syslog(LOG_CRIT,
> - "AIIEEE lost spice client connection, reconnecting");
> - virtio_port = vdagent_virtio_port_create(portdev,
> - virtio_port_read_complete,
> - NULL);
> - if (!virtio_port) {
> - syslog(LOG_CRIT,
> - "Fatal error opening vdagent virtio channel");
> - retval = 1;
> - break;
> - }
> - do_client_disconnect();
> - client_connected = old_client_connected;
> - }
> - }
> - else if (only_once && once)
> - {
> - syslog(LOG_INFO, "Exiting after one client session.");
> - break;
> - }
> -
> - if (session_info && FD_ISSET(ck_fd, &readfds)) {
> - active_session = session_info_get_active_session(session_info);
> - update_active_session_connection(NULL);
> - }
> - }
> -}
> -
> -static void quit_handler(int sig)
> -{
> - quit = 1;
> + g_main_loop_quit(loop);
> + return G_SOURCE_REMOVE;
> }
>
> static gboolean parse_debug_level_cb(const gchar *option_name,
> @@ -1115,7 +1076,6 @@ int main(int argc, char *argv[])
> {
> GOptionContext *context;
> GError *err = NULL;
> - struct sigaction act;
> gboolean own_socket = TRUE;
>
> context = g_option_context_new(NULL);
> @@ -1138,13 +1098,9 @@ int main(int argc, char *argv[])
> if (uinput_device == NULL)
> uinput_device = g_strdup(DEFAULT_UINPUT_DEVICE);
>
> - 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);
> + g_unix_signal_add(SIGINT, signal_handler, NULL);
> + g_unix_signal_add(SIGHUP, signal_handler, NULL);
> + g_unix_signal_add(SIGTERM, signal_handler, NULL);
>
> openlog("spice-vdagentd", do_daemonize ? 0 : LOG_PERROR, LOG_USER);
>
> @@ -1214,7 +1170,9 @@ int main(int argc, char *argv[])
> syslog(LOG_WARNING, "no session info, max 1 session agent allowed");
>
> active_xfers = g_hash_table_new(g_direct_hash, g_direct_equal);
> - main_loop();
> +
> + loop = g_main_loop_new(NULL, FALSE);
> + g_main_loop_run(loop);
>
> release_clipboards();
>
> @@ -1223,6 +1181,7 @@ int main(int argc, char *argv[])
> vdagent_virtio_port_destroy(&virtio_port);
> session_info_destroy(session_info);
> udscs_destroy_server(server);
> + g_main_loop_unref(loop);
>
> /* leave the socket around if it was provided by systemd */
> if (own_socket) {
> --
> 2.17.1
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180828/23e05dc8/attachment.sig>
More information about the Spice-devel
mailing list