Fixes 100% CPU usage with usbredirect as TCP Server

Backporting this fix too to avoid reworking too much the upstream
patch for the related bug below.

Related: https://bugzilla.redhat.com/show_bug.cgi?id=2157520
Signed-off-by: Victor Toso <victortoso@redhat.com>

0002-Use-typedef-on-redirect-structure-to-simplify-some-s.patch

@@ -0,0 +1,135 @@
+From c1246d5d8332890df0dab7b29de86a42c2b7b36a Mon Sep 17 00:00:00 2001
+From: Frediano Ziglio <freddy77@gmail.com>
+Date: Fri, 16 Sep 2022 20:14:28 +0100
+Subject: [PATCH 2/4] Use typedef on redirect structure to simplify some
+ statements
+
+Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
+---
+ tools/usbredirect.c | 26 +++++++++++++-------------
+ 1 file changed, 13 insertions(+), 13 deletions(-)
+
+diff --git a/tools/usbredirect.c b/tools/usbredirect.c
+index ff910ab..a479c55 100644
+--- a/tools/usbredirect.c
++++ b/tools/usbredirect.c
+@@ -22,7 +22,7 @@
+ #include <gio/gwin32outputstream.h>
+ #endif
+ 
+-struct redirect {
++typedef struct redirect {
+     struct {
+         int vendor;
+         int product;
+@@ -40,7 +40,7 @@ struct redirect {
+     int watch_server_id;
+ 
+     GMainLoop *main_loop;
+-};
++} redirect;
+ 
+ static bool
+ parse_opt_device(const char *device, int *vendor, int *product)
+@@ -125,7 +125,7 @@ parse_opt_uri(const char *uri, char **adr, int *port)
+     return true;
+ }
+ 
+-static struct redirect *
++static redirect *
+ parse_opts(int *argc, char ***argv)
+ {
+     char *device = NULL;
+@@ -133,7 +133,7 @@ parse_opts(int *argc, char ***argv)
+     char *localaddr = NULL;
+     gboolean keepalive = FALSE;
+     gint verbosity = 0; /* none */
+-    struct redirect *self = NULL;
++    redirect *self = NULL;
+ 
+     GOptionEntry entries[] = {
+         { "device", 0, 0, G_OPTION_ARG_STRING, &device, "Local USB device to be redirected", NULL },
+@@ -162,7 +162,7 @@ parse_opts(int *argc, char ***argv)
+         goto end;
+     }
+ 
+-    self = g_new0(struct redirect, 1);
++    self = g_new0(redirect, 1);
+     if (!parse_opt_device(device, &self->device.vendor, &self->device.product)) {
+         g_printerr("Failed to parse device: '%s' - expected: vendor:product or busnum-devnum\n", device);
+         g_clear_pointer(&self, g_free);
+@@ -202,7 +202,7 @@ end:
+ static gpointer
+ thread_handle_libusb_events(gpointer user_data)
+ {
+-    struct redirect *self = (struct redirect *) user_data;
++    redirect *self = (redirect *) user_data;
+ 
+     int res = 0;
+     const char *desc = "";
+@@ -280,7 +280,7 @@ usbredir_log_cb(void *priv, int level, const char *msg)
+ static int
+ usbredir_read_cb(void *priv, uint8_t *data, int count)
+ {
+-    struct redirect *self = (struct redirect *) priv;
++    redirect *self = (redirect *) priv;
+     GIOStream *iostream = G_IO_STREAM(self->connection);
+     GError *err = NULL;
+ 
+@@ -308,7 +308,7 @@ usbredir_read_cb(void *priv, uint8_t *data, int count)
+ static int
+ usbredir_write_cb(void *priv, uint8_t *data, int count)
+ {
+-    struct redirect *self = (struct redirect *) priv;
++    redirect *self = (redirect *) priv;
+     GIOStream *iostream = G_IO_STREAM(self->connection);
+     GError *err = NULL;
+ 
+@@ -336,7 +336,7 @@ usbredir_write_cb(void *priv, uint8_t *data, int count)
+ static void
+ usbredir_write_flush_cb(void *user_data)
+ {
+-    struct redirect *self = (struct redirect *) user_data;
++    redirect *self = (redirect *) user_data;
+     if (!self || !self->usbredirhost) {
+         return;
+     }
+@@ -387,7 +387,7 @@ usbredir_unlock_lock(void *user_data)
+ static gboolean
+ connection_handle_io_cb(GIOChannel *source, GIOCondition condition, gpointer user_data)
+ {
+-    struct redirect *self = (struct redirect *) user_data;
++    redirect *self = (redirect *) user_data;
+ 
+     if (condition & G_IO_ERR || condition & G_IO_HUP) {
+         g_warning("Connection: err=%d, hup=%d - exiting", (condition & G_IO_ERR), (condition & G_IO_HUP));
+@@ -419,7 +419,7 @@ end:
+ static gboolean
+ signal_handler(gpointer user_data)
+ {
+-    struct redirect *self = (struct redirect *) user_data;
++    redirect *self = (redirect *) user_data;
+     g_main_loop_quit(self->main_loop);
+     return G_SOURCE_REMOVE;
+ }
+@@ -431,7 +431,7 @@ connection_incoming_cb(GSocketService    *service,
+                        GObject           *source_object,
+                        gpointer           user_data)
+ {
+-    struct redirect *self = (struct redirect *) user_data;
++    redirect *self = (redirect *) user_data;
+     self->connection = g_object_ref(client_connection);
+ 
+     /* Add a GSource watch to handle polling for us and handle IO in the callback */
+@@ -456,7 +456,7 @@ main(int argc, char *argv[])
+         goto err_init;
+     }
+ 
+-    struct redirect *self = parse_opts(&argc, &argv);
++    redirect *self = parse_opts(&argc, &argv);
+     if (!self) {
+         /* specific issues logged in parse_opts() */
+         return 1;
+-- 
+2.39.0
+

0003-Factor-out-a-function-to-create-watches.patch

@@ -0,0 +1,74 @@
+From 307747e2a73cf68a239ddd7b70333bbddf7f3e3b Mon Sep 17 00:00:00 2001
+From: Frediano Ziglio <freddy77@gmail.com>
+Date: Fri, 16 Sep 2022 20:14:28 +0100
+Subject: [PATCH 3/4] Factor out a function to create watches
+
+---
+ tools/usbredirect.c | 37 ++++++++++++++++++++-----------------
+ 1 file changed, 20 insertions(+), 17 deletions(-)
+
+diff --git a/tools/usbredirect.c b/tools/usbredirect.c
+index a479c55..afe9dee 100644
+--- a/tools/usbredirect.c
++++ b/tools/usbredirect.c
+@@ -415,6 +415,24 @@ end:
+     return G_SOURCE_REMOVE;
+ }
+ 
++static void
++create_watch(redirect *self)
++{
++    GSocket *socket = g_socket_connection_get_socket(self->connection);
++    int socket_fd = g_socket_get_fd(socket);
++    GIOChannel *io_channel =
++#ifdef G_OS_UNIX
++        g_io_channel_unix_new(socket_fd);
++#else
++        g_io_channel_win32_new_socket(socket_fd);
++#endif
++
++    self->watch_server_id = g_io_add_watch(io_channel,
++            G_IO_IN | G_IO_OUT | G_IO_HUP | G_IO_ERR,
++            connection_handle_io_cb,
++            self);
++}
++
+ #ifdef G_OS_UNIX
+ static gboolean
+ signal_handler(gpointer user_data)
+@@ -437,12 +455,7 @@ connection_incoming_cb(GSocketService    *service,
+     /* Add a GSource watch to handle polling for us and handle IO in the callback */
+     GSocket *connection_socket = g_socket_connection_get_socket(self->connection);
+     g_socket_set_keepalive(connection_socket, self->keepalive);
+-    int socket_fd = g_socket_get_fd(connection_socket);
+-    GIOChannel *io_channel = g_io_channel_unix_new(socket_fd);
+-    self->watch_server_id = g_io_add_watch(io_channel,
+-            G_IO_IN | G_IO_OUT | G_IO_HUP | G_IO_ERR,
+-            connection_handle_io_cb,
+-            self);
++    create_watch(self);
+     return G_SOURCE_REMOVE;
+ }
+ 
+@@ -552,17 +565,7 @@ main(int argc, char *argv[])
+ 
+         GSocket *connection_socket = g_socket_connection_get_socket(self->connection);
+         g_socket_set_keepalive(connection_socket, self->keepalive);
+-        int socket_fd = g_socket_get_fd(connection_socket);
+-        GIOChannel *io_channel =
+-#ifdef G_OS_UNIX
+-            g_io_channel_unix_new(socket_fd);
+-#else
+-            g_io_channel_win32_new_socket(socket_fd);
+-#endif
+-        self->watch_server_id = g_io_add_watch(io_channel,
+-                G_IO_IN | G_IO_OUT | G_IO_HUP | G_IO_ERR,
+-                connection_handle_io_cb,
+-                self);
++        create_watch(self);
+     } else {
+         GSocketService *socket_service;
+ 
+-- 
+2.39.0
+

0004-Recreate-watch-if-needed.patch

@@ -0,0 +1,106 @@
+From 3fcbd4a2569f227ae6fad6a37c8864d33271e5f4 Mon Sep 17 00:00:00 2001
+From: Frediano Ziglio <freddy77@gmail.com>
+Date: Sat, 17 Sep 2022 09:28:08 +0100
+Subject: [PATCH 4/4] Recreate watch if needed
+
+Do not always watch for output buffer.
+Watching for output buffer if we don't have nothing to write
+(which is the usual case) is consuming a lot of CPU.
+
+This fixes https://gitlab.freedesktop.org/spice/usbredir/-/issues/24.
+
+Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
+---
+ tools/usbredirect.c | 28 ++++++++++++++++++++++++++--
+ 1 file changed, 26 insertions(+), 2 deletions(-)
+
+diff --git a/tools/usbredirect.c b/tools/usbredirect.c
+index afe9dee..59452aa 100644
+--- a/tools/usbredirect.c
++++ b/tools/usbredirect.c
+@@ -29,6 +29,7 @@ typedef struct redirect {
+     } device;
+     bool is_client;
+     bool keepalive;
++    bool watch_inout;
+     char *addr;
+     int port;
+     int verbosity;
+@@ -42,6 +43,8 @@ typedef struct redirect {
+     GMainLoop *main_loop;
+ } redirect;
+ 
++static void create_watch(redirect *self);
++
+ static bool
+ parse_opt_device(const char *device, int *vendor, int *product)
+ {
+@@ -163,6 +166,7 @@ parse_opts(int *argc, char ***argv)
+     }
+ 
+     self = g_new0(redirect, 1);
++    self->watch_inout = true;
+     if (!parse_opt_device(device, &self->device.vendor, &self->device.product)) {
+         g_printerr("Failed to parse device: '%s' - expected: vendor:product or busnum-devnum\n", device);
+         g_clear_pointer(&self, g_free);
+@@ -277,6 +281,20 @@ usbredir_log_cb(void *priv, int level, const char *msg)
+     g_log_structured(G_LOG_DOMAIN, glog_level, "MESSAGE", msg);
+ }
+ 
++static void
++update_watch(redirect *self)
++{
++    const bool watch_inout = usbredirhost_has_data_to_write(self->usbredirhost) != 0;
++    if (watch_inout == self->watch_inout) {
++        return;
++    }
++    g_source_remove(self->watch_server_id);
++    self->watch_server_id = 0;
++    self->watch_inout = watch_inout;
++
++    create_watch(self);
++}
++
+ static int
+ usbredir_read_cb(void *priv, uint8_t *data, int count)
+ {
+@@ -322,6 +340,7 @@ usbredir_write_cb(void *priv, uint8_t *data, int count)
+         if (g_error_matches(err, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK)) {
+             /* Try again later */
+             nbytes = 0;
++            update_watch(self);
+         } else {
+             if (err != NULL) {
+                 g_warning("Failure at %s: %s", __func__, err->message);
+@@ -401,13 +420,18 @@ connection_handle_io_cb(GIOChannel *source, GIOCondition condition, gpointer use
+             goto end;
+         }
+     }
+-    if (condition & G_IO_OUT) {
++    // try to write data in any case, to avoid having another iteration and
++    // creation of another watch if there is space in output buffer
++    if (usbredirhost_has_data_to_write(self->usbredirhost) != 0) {
+         int ret = usbredirhost_write_guest_data(self->usbredirhost);
+         if (ret < 0) {
+             g_critical("%s: Failed to write to guest", __func__);
+             goto end;
+         }
+     }
++
++    // update the watch if needed
++    update_watch(self);
+     return G_SOURCE_CONTINUE;
+ 
+ end:
+@@ -428,7 +452,7 @@ create_watch(redirect *self)
+ #endif
+ 
+     self->watch_server_id = g_io_add_watch(io_channel,
+-            G_IO_IN | G_IO_OUT | G_IO_HUP | G_IO_ERR,
++            G_IO_IN | G_IO_HUP | G_IO_ERR | (self->watch_inout ? G_IO_OUT : 0),
+             connection_handle_io_cb,
+             self);
+ }
+-- 
+2.39.0
+

usbredir.spec

@@ -1,11 +1,14 @@
 Name:           usbredir
 Version:        0.13.0
-Release:        1%{?dist}
+Release:        2%{?dist}
 Summary:        USB network redirection protocol libraries
 License:        LGPLv2+
 URL:            https://spice-space.org/usbredir.html
 Source0:        http://spice-space.org/download/%{name}/%{name}-%{version}.tar.xz
 Patch0001:      0001-Revert-remove-usbredirserver.patch
+Patch0002:      0002-Use-typedef-on-redirect-structure-to-simplify-some-s.patch
+Patch0003:      0003-Factor-out-a-function-to-create-watches.patch
+Patch0004:      0004-Recreate-watch-if-needed.patch
 BuildRequires:  gcc
 BuildRequires:  glib2-devel
 BuildRequires:  libusb1-devel >= 1.0.9
@@ -85,6 +88,10 @@ A simple USB host TCP server, using libusbredirhost.
 
 
 %changelog
+* Thu Jan 05 2023 Victor Toso <victortoso@redhat.com> - 0.13.0-2
+- Fixes 100% CPU usage when usbredirect used as TCP server
+  Related: rhbz#2157520
+
 * Wed Nov 30 2022 Victor Toso <victortoso@redhat.com> - 0.13.0-1
 - Rebase to latest upstream: 0.13.0
 - Keeps usbredirserver binary (removed upstream)