On Fri, Jan 17, 2020 at 12:04:08PM +0100, Francesco Giudici wrote: > When remote-viewer is started from terminal, CTRL-C sends a SIGINT > signal to the program causing immediate termination. On linux clients > usb redirected devices are left without any kernel driver attached, > causing them to appear as no more available to the host. > Add a SIGINT handler to allow a clean exit, in particular to allow > the kernel to attach back the host driver. > The issue is present on linux only. > > To perform usb device redirection, virt-viewer leverages spice-gtk > library, which in turn relies on usbredir library, which uses libusb. > In order to take control of the usb device the auto-loaded kernel > driver must be detached. This is achieved (in the very end) with > libusb_detach_kernel_driver(). Then the device interfaces can be claimed > with libusb_claim_interface() and get in control to the application. > During normal application termination, the usb channel is teared down, > performing a reset of the usb device and giving back the control of the > device to the kernel (libusb_attach_kernel_driver()). > If the application quits without doing this, the device interfaces will > end up with no driver attached, making them not usable in the host > system. > > Note that enabling libusb_set_auto_detach_kernel_driver() does not solve > the issue, as this is just a convenient API from libusb: libusb will > take care of detaching/attaching the driver to the interfaces of the usb > device each time a call to libusb_release_interface() and > libusb_claim_interface() is performed. An unexpected quit of the > application will skip the libusb_release_interface() call too, leaving > the interfaces without any driver attached. > > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1713311 > > Signed-off-by: Francesco Giudici <fgiudici@xxxxxxxxxx> > --- > src/virt-viewer-app.c | 79 +++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 79 insertions(+) > > diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c > index da8cfa9..06e237b 100644 > --- a/src/virt-viewer-app.c > +++ b/src/virt-viewer-app.c > @@ -36,6 +36,7 @@ > #include <glib/gprintf.h> > #include <glib/gi18n.h> > #include <errno.h> > +#include <fcntl.h> > > #ifdef HAVE_SYS_SOCKET_H > #include <sys/socket.h> > @@ -1756,6 +1757,74 @@ static gboolean opt_fullscreen = FALSE; > static gboolean opt_kiosk = FALSE; > static gboolean opt_kiosk_quit = FALSE; > > +#ifndef G_OS_WIN32 > +static int sigint_pipe[2]; > + > +static void > +sigint_handler(int signum) > +{ > + int savedErrno; > + > + g_return_if_fail(signum == SIGINT); > + > + savedErrno = errno; > + if (write(sigint_pipe[1], "x", 1) == -1 && errno != EAGAIN) > + g_debug("SIGINT handler failure\n"); > + errno = savedErrno; > +} > + > +static void > +register_sigint_handler() > +{ > + int flags, i; > + struct sigaction sa; > + > + if (pipe(sigint_pipe) == -1) > + goto err; > + > + for (i = 0; i < 2; i++) { > + flags = fcntl(sigint_pipe[i], F_GETFL); > + if (flags == -1) > + goto err; > + flags |= O_NONBLOCK; > + if (fcntl(sigint_pipe[i], F_SETFL, flags) == -1) > + goto err; > + } > + > + sigemptyset(&sa.sa_mask); > + sa.sa_flags = SA_RESTART; > + sa.sa_handler = sigint_handler; > + if (sigaction(SIGINT, &sa, NULL) == -1) > + goto err; > + > + return; > + > +err: > + g_debug("Cannot register SIGINT handler\n"); > +} > + > +static gboolean > +sigint_cb(GIOChannel *source, > + GIOCondition condition, > + gpointer data) > +{ > + VirtViewerApp *self = VIRT_VIEWER_APP(data); > + VirtViewerAppPrivate *priv = self->priv; > + gchar sbuf; > + > + g_assert(condition == G_IO_IN); > + > + g_debug("got SIGINT, quitting\n"); > + if (priv->started) > + virt_viewer_app_quit(self); > + else > + exit(0); > + > + g_io_channel_read_chars (source, &sbuf, 1, NULL, NULL); > + return TRUE; > +} > +#endif > + > static void > title_maybe_changed(VirtViewerApp *self, GParamSpec* pspec G_GNUC_UNUSED, gpointer user_data G_GNUC_UNUSED) > { > @@ -1766,10 +1835,20 @@ static void > virt_viewer_app_init(VirtViewerApp *self) > { > GError *error = NULL; > +#ifndef G_OS_WIN32 > + GIOChannel *sigint_channel = NULL; > +#endif > + > self->priv = virt_viewer_app_get_instance_private(self); > > gtk_window_set_default_icon_name("virt-viewer"); > > +#ifndef G_OS_WIN32 > + register_sigint_handler(); > + sigint_channel = g_io_channel_unix_new(sigint_pipe[0]); > + g_io_add_watch(sigint_channel, G_IO_IN, sigint_cb, self); > +#endif Just yesterday I learnt that GLib has native support for signal handling in its event loop which allows this to be simplified: g_unix_signal_add (SIGINT, sigint_cb, self); > self->priv->displays = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL, g_object_unref); > self->priv->config = g_key_file_new(); > self->priv->config_file = g_build_filename(g_get_user_config_dir(), > -- > 2.21.1 > > Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
