Re: [RFC spice-vdagent 03/18] vdagentd: use GMainLoop

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>

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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]