Hi Victor,
On Tue, Aug 28, 2018 at 9:38 AM Victor Toso <victortoso@xxxxxxxxxx> wrote:
>
> 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.
We surely can :) but the situation in vdagent.c was simpler, IMHO.
In vdagent.c, we started using GMainLoop and g_io_add_watch() both in
one commit which was relatively small.
If we wanted to do the same here, we'd have to squash these 3 commits:
* "vdagentd: use GMainLoop"
* "udscs: use VDAgentConnection"
* "vport: use VDAgentConnection"
Apart from that, g_io_add_watch() would need to be used with the
session_info_get_fd().
The resulting patch would be huge, so I opted for this variant.
I don't know what's the best way to go, I'll leave it up to you ;)
Cheers,
Jakub
>
> 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
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel