[Spice-devel] [PATCH spice-gtk v5 13/18] usb-backend: Rewrite USB emulation support

Frediano Ziglio fziglio at redhat.com
Wed Aug 28 14:14:16 UTC 2019


Make initialization easier.
Initialize both parser and host during spice_usb_backend_channel_new.
parser is always initialized after creation making sure the state
is consistent.
Reduce number of state variables.
Use a single state variable, data flows into/from a single parser.
Support not having libusb context (no physical devices).

Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
---
 src/usb-backend.c | 241 +++++++++++++++++++++++-----------------------
 1 file changed, 121 insertions(+), 120 deletions(-)

diff --git a/src/usb-backend.c b/src/usb-backend.c
index 857ffdf7..63517ce8 100644
--- a/src/usb-backend.c
+++ b/src/usb-backend.c
@@ -76,21 +76,21 @@ struct _SpiceUsbBackend
     uint32_t own_devices_mask;
 };
 
+typedef enum {
+    USB_CHANNEL_STATE_INITIALIZING,
+    USB_CHANNEL_STATE_HOST,
+    USB_CHANNEL_STATE_PARSER,
+} SpiceUsbBackendChannelState;
+
 struct _SpiceUsbBackendChannel
 {
     struct usbredirhost *usbredirhost;
-    struct usbredirhost *hidden_host;
     struct usbredirparser *parser;
-    struct usbredirparser *hidden_parser;
+    SpiceUsbBackendChannelState state;
     uint8_t *read_buf;
     int read_buf_size;
     struct usbredirfilter_rule *rules;
     int rules_count;
-    uint32_t host_caps;
-    uint32_t parser_init_done  : 1;
-    uint32_t parser_with_hello : 1;
-    uint32_t hello_done_parser : 1;
-    uint32_t hello_sent        : 1;
     uint32_t rejected          : 1;
     uint32_t wait_disconnect_ack : 1;
     SpiceUsbBackendDevice *attached;
@@ -403,15 +403,16 @@ from both the main thread as well as from the usb event handling thread */
 static void usbredir_write_flush_callback(void *user_data)
 {
     SpiceUsbBackendChannel *ch = user_data;
+    if (ch->parser == NULL) {
+        return;
+    }
     if (is_channel_ready(ch->user_data)) {
-        if (ch->usbredirhost) {
+        if (ch->state == USB_CHANNEL_STATE_HOST) {
             SPICE_DEBUG("%s ch %p -> usbredirhost", __FUNCTION__, ch);
             usbredirhost_write_guest_data(ch->usbredirhost);
-        } else if (ch->parser) {
+        } else {
             SPICE_DEBUG("%s ch %p -> parser", __FUNCTION__, ch);
             usbredirparser_do_write(ch->parser);
-        } else {
-            SPICE_DEBUG("%s ch %p (NOT ACTIVE)", __FUNCTION__, ch);
         }
     } else {
         SPICE_DEBUG("%s ch %p (not ready)", __FUNCTION__, ch);
@@ -671,21 +672,47 @@ static void usbredir_log(void *user_data, int level, const char *msg)
     }
 }
 
+static struct usbredirparser *create_parser(SpiceUsbBackendChannel *ch);
+
 static int usbredir_write_callback(void *user_data, uint8_t *data, int count)
 {
     SpiceUsbBackendChannel *ch = user_data;
     int res;
     SPICE_DEBUG("%s ch %p, %d bytes", __FUNCTION__, ch, count);
-    if (!ch->hello_sent) {
-        /* hello is short header (12) + hello struct (64) + capabilities (4) */
-        int hello_size = sizeof(struct usb_redir_header) + sizeof(struct usb_redir_hello_header);
-        ch->hello_sent = 1;
-        if (count == hello_size) {
-            memcpy(&ch->host_caps, data + hello_size - sizeof(ch->host_caps),
-                   sizeof(ch->host_caps));
-            SPICE_DEBUG("%s ch %p, sending first hello, caps %08X",
-                        __FUNCTION__, ch, ch->host_caps);
+
+    // handle first packet (HELLO) creating parser from capabilities
+    if (SPICE_UNLIKELY(ch->parser == NULL)) {
+        // Here we return 0 from this function to keep the packet in
+        // the queue. The packet will then be sent to the guest.
+        // We are initializing SpiceUsbBackendChannel, the
+        // SpiceUsbredirChannel is not ready to receive packets.
+
+        // we are still initializing the host
+        if (ch->usbredirhost == NULL) {
+            return 0;
         }
+
+        ch->parser = create_parser(ch);
+        if (!ch->parser) {
+            return 0;
+        }
+
+        /* hello is short header (12) + hello struct (64) */
+        const int hello_size = 12 + sizeof(struct usb_redir_hello_header);
+        g_assert(count >= hello_size + 4);
+        g_assert(SPICE_ALIGNED_CAST(struct usb_redir_header *, data)->type == usb_redir_hello);
+
+        const uint32_t flags =
+            usbredirparser_fl_write_cb_owns_buffer | usbredirparser_fl_usb_host |
+            usbredirparser_fl_no_hello;
+
+        usbredirparser_init(ch->parser,
+                            PACKAGE_STRING,
+                            SPICE_ALIGNED_CAST(uint32_t *, data + hello_size),
+                            (count - hello_size) / sizeof(uint32_t),
+                            flags);
+
+        return 0;
     }
     res = spice_usbredir_write_callback(ch->user_data, data, count);
     return res;
@@ -705,31 +732,33 @@ int spice_usb_backend_read_guest_data(SpiceUsbBackendChannel *ch, uint8_t *data,
 
     ch->read_buf = data;
     ch->read_buf_size = count;
-    if (ch->usbredirhost) {
-        res = usbredirhost_read_guest_data(ch->usbredirhost);
-        if (!ch->hello_done_parser) {
-            int res_parser;
+    if (SPICE_UNLIKELY(ch->state == USB_CHANNEL_STATE_INITIALIZING)) {
+        if (ch->usbredirhost != NULL) {
+            res = usbredirhost_read_guest_data(ch->usbredirhost);
+            if (res != 0) {
+                return res;
+            }
+            ch->state = USB_CHANNEL_STATE_HOST;
+
             /* usbredirhost should consume hello response */
             g_return_val_if_fail(ch->read_buf == NULL, USB_REDIR_ERROR_READ_PARSE);
+        } else {
+            ch->state = USB_CHANNEL_STATE_PARSER;
+        }
 
-            ch->read_buf = data;
-            ch->read_buf_size = count;
-            ch->hello_done_parser = 1;
-            if (ch->attached && ch->attached->edev) {
-                /* case of CD sharing on connect */
-                ch->usbredirhost = NULL;
-                ch->parser = ch->hidden_parser;
-                SPICE_DEBUG("%s: switch %p to parser", __FUNCTION__, ch);
-            }
-            res_parser = usbredirparser_do_read(ch->hidden_parser);
-            if (res >= 0) {
-                res = res_parser;
-            }
+        ch->read_buf = data;
+        ch->read_buf_size = count;
+        if (ch->attached && ch->attached->edev) {
+            /* case of CD sharing on connect */
+            ch->state = USB_CHANNEL_STATE_PARSER;
+            SPICE_DEBUG("%s: switch %p to parser", __FUNCTION__, ch);
         }
-    } else if (ch->parser) {
-        res = usbredirparser_do_read(ch->parser);
+        return usbredirparser_do_read(ch->parser);
+    }
+    if (ch->state == USB_CHANNEL_STATE_HOST) {
+        res = usbredirhost_read_guest_data(ch->usbredirhost);
     } else {
-        res = USB_REDIR_ERROR_IO;
+        res = usbredirparser_do_read(ch->parser);
     }
     switch (res)
     {
@@ -781,14 +810,12 @@ GError *spice_usb_backend_get_error_details(int error_code, gchar *desc)
 
 void spice_usb_backend_return_write_data(SpiceUsbBackendChannel *ch, void *data)
 {
-    if (ch->usbredirhost) {
+    if (ch->state == USB_CHANNEL_STATE_HOST) {
         SPICE_DEBUG("%s ch %p -> usbredirhost", __FUNCTION__, ch);
         usbredirhost_free_write_buffer(ch->usbredirhost, data);
-    } else if (ch->parser) {
+    } else {
         SPICE_DEBUG("%s ch %p -> parser", __FUNCTION__, ch);
         usbredirparser_free_write_buffer(ch->parser, data);
-    } else {
-        SPICE_DEBUG("%s ch %p - NOBODY TO CALL", __FUNCTION__, ch);
     }
 }
 
@@ -1003,10 +1030,10 @@ static void usbredir_device_disconnect_ack(void *priv)
 {
     SpiceUsbBackendChannel *ch = priv;
     SPICE_DEBUG("%s ch %p", __FUNCTION__, ch);
-    if (ch->parser && ch->wait_disconnect_ack) {
-        ch->parser = NULL;
+    if (ch->state == USB_CHANNEL_STATE_PARSER && ch->usbredirhost != NULL &&
+        ch->wait_disconnect_ack) {
+        ch->state = USB_CHANNEL_STATE_HOST;
         SPICE_DEBUG("%s switch to usbredirhost", __FUNCTION__);
-        ch->usbredirhost = ch->hidden_host;
     }
     ch->wait_disconnect_ack = 0;
 }
@@ -1025,9 +1052,6 @@ usbredir_hello(void *priv, struct usb_redir_hello_header *hello)
     SPICE_DEBUG("%s %p %sattached %s", __FUNCTION__, ch,
         edev ? "" : "not ",  hello ? "" : "(internal)");
 
-    if (hello) {
-        ch->hello_done_parser = 1;
-    }
     if (!edev) {
         return;
     }
@@ -1077,53 +1101,24 @@ usbredir_hello(void *priv, struct usb_redir_hello_header *hello)
     usbredir_write_flush_callback(ch);
 }
 
-static void initialize_parser(SpiceUsbBackendChannel *ch, gboolean do_hello)
+static void initialize_parser(SpiceUsbBackendChannel *ch)
 {
     uint32_t flags, caps[USB_REDIR_CAPS_SIZE] = { 0 };
 
-    g_return_if_fail(ch->hidden_parser != NULL);
-    if (ch->parser_init_done) {
-        if (ch->parser_with_hello != do_hello) {
-            g_return_if_reached();
-        }
-        return;
-    }
+    g_assert(ch->usbredirhost == NULL);
 
-    ch->parser_init_done = 1;
-    ch->parser_with_hello = do_hello;
     flags = usbredirparser_fl_write_cb_owns_buffer | usbredirparser_fl_usb_host;
 
-    if (do_hello) {
-        ch->hello_sent = 1;
-        ch->host_caps |= 1 << usb_redir_cap_connect_device_version;
-        ch->host_caps |= 1 << usb_redir_cap_device_disconnect_ack;
-        ch->host_caps |= 1 << usb_redir_cap_ep_info_max_packet_size;
-        ch->host_caps |= 1 << usb_redir_cap_64bits_ids;
-        ch->host_caps |= 1 << usb_redir_cap_32bits_bulk_length;
-    } else {
-        flags |= usbredirparser_fl_no_hello;
-    }
-
-    if (ch->host_caps & (1 << usb_redir_cap_connect_device_version)) {
-        usbredirparser_caps_set_cap(caps, usb_redir_cap_connect_device_version);
-    }
+    usbredirparser_caps_set_cap(caps, usb_redir_cap_connect_device_version);
     usbredirparser_caps_set_cap(caps, usb_redir_cap_filter);
-    if (ch->host_caps & (1 << usb_redir_cap_device_disconnect_ack)) {
-        usbredirparser_caps_set_cap(caps, usb_redir_cap_device_disconnect_ack);
-    }
-    if (ch->host_caps & (1 << usb_redir_cap_ep_info_max_packet_size)) {
-        usbredirparser_caps_set_cap(caps, usb_redir_cap_ep_info_max_packet_size);
-    }
-    if (ch->host_caps & (1 << usb_redir_cap_64bits_ids)) {
-        usbredirparser_caps_set_cap(caps, usb_redir_cap_64bits_ids);
-    }
-    if (ch->host_caps & (1 << usb_redir_cap_32bits_bulk_length)) {
-        usbredirparser_caps_set_cap(caps, usb_redir_cap_32bits_bulk_length);
-    }
-    if (ch->host_caps & (1 << usb_redir_cap_bulk_streams)) {
-        usbredirparser_caps_set_cap(caps, usb_redir_cap_bulk_streams);
-    }
-    usbredirparser_init(ch->hidden_parser, PACKAGE_STRING, caps, USB_REDIR_CAPS_SIZE, flags);
+    usbredirparser_caps_set_cap(caps, usb_redir_cap_device_disconnect_ack);
+    usbredirparser_caps_set_cap(caps, usb_redir_cap_ep_info_max_packet_size);
+    usbredirparser_caps_set_cap(caps, usb_redir_cap_64bits_ids);
+    usbredirparser_caps_set_cap(caps, usb_redir_cap_32bits_bulk_length);
+    usbredirparser_caps_set_cap(caps, usb_redir_cap_bulk_receiving);
+    usbredirparser_caps_set_cap(caps, usb_redir_cap_bulk_streams);
+
+    usbredirparser_init(ch->parser, PACKAGE_STRING, caps, USB_REDIR_CAPS_SIZE, flags);
 }
 
 /*
@@ -1178,7 +1173,7 @@ static gboolean attach_edev(SpiceUsbBackendChannel *ch,
            _("Failed to redirect device %d"), 1);
         return FALSE;
     }
-    if (ch->usbredirhost && !ch->hello_done_parser) {
+    if (ch->state == USB_CHANNEL_STATE_INITIALIZING) {
         /*
             we can't initialize parser until we see hello from usbredir
             and the parser can't work until it sees the hello response.
@@ -1188,15 +1183,13 @@ static gboolean attach_edev(SpiceUsbBackendChannel *ch,
         SPICE_DEBUG("%s waiting until the channel is ready", __FUNCTION__);
 
     } else {
-        initialize_parser(ch, ch->hidden_host == NULL);
-        ch->usbredirhost = NULL;
-        ch->parser = ch->hidden_parser;
+        ch->state = USB_CHANNEL_STATE_PARSER;
     }
     ch->wait_disconnect_ack = 0;
     ch->attached = dev;
     dev->attached_to = ch;
-    device_ops(dev->edev)->attach(dev->edev, ch->hidden_parser);
-    if (ch->hello_done_parser) {
+    device_ops(dev->edev)->attach(dev->edev, ch->parser);
+    if (ch->state == USB_CHANNEL_STATE_PARSER) {
         /* send device info */
         usbredir_hello(ch, NULL);
     }
@@ -1216,9 +1209,15 @@ gboolean spice_usb_backend_channel_attach(SpiceUsbBackendChannel *ch,
         return attach_edev(ch, dev, error);
     }
 
+    // no physical device enabled
+    if (ch->usbredirhost == NULL) {
+        return FALSE;
+    }
+
     libusb_device_handle *handle = NULL;
-    ch->usbredirhost = ch->hidden_host;
-    ch->parser = NULL;
+    if (ch->state != USB_CHANNEL_STATE_INITIALIZING) {
+        ch->state = USB_CHANNEL_STATE_HOST;
+    }
 
     /*
        Under Windows we need to avoid updating
@@ -1259,10 +1258,10 @@ void spice_usb_backend_channel_detach(SpiceUsbBackendChannel *ch)
         SPICE_DEBUG("%s: nothing to detach", __FUNCTION__);
         return;
     }
-    if (ch->usbredirhost) {
+    if (ch->state == USB_CHANNEL_STATE_HOST) {
         /* it will call libusb_close internally */
         usbredirhost_set_device(ch->usbredirhost, NULL);
-    } else if (ch->parser) {
+    } else {
         if (edev) {
             device_ops(edev)->detach(edev);
         }
@@ -1270,9 +1269,8 @@ void spice_usb_backend_channel_detach(SpiceUsbBackendChannel *ch)
         usbredir_write_flush_callback(ch);
         ch->wait_disconnect_ack =
             usbredirparser_peer_has_cap(ch->parser, usb_redir_cap_device_disconnect_ack);
-        if (!ch->wait_disconnect_ack) {
-            ch->usbredirhost = ch->hidden_host;
-            ch->parser = NULL;
+        if (!ch->wait_disconnect_ack && ch->usbredirhost != NULL) {
+            ch->state = USB_CHANNEL_STATE_HOST;
         }
     }
     SPICE_DEBUG("%s ch %p, detach done", __FUNCTION__, ch);
@@ -1309,16 +1307,22 @@ SpiceUsbBackendChannel *spice_usb_backend_channel_new(SpiceUsbBackend *be,
                                         usbredirparser_warning,
                                    usbredirhost_fl_write_cb_owns_buffer);
         g_warn_if_fail(ch->usbredirhost != NULL);
+        if (ch->usbredirhost != NULL) {
+            usbredirhost_set_buffered_output_size_cb(ch->usbredirhost,
+                                                     usbredir_buffered_output_size_callback);
+            // force flush of HELLO packet and creation of parser
+            usbredirhost_write_guest_data(ch->usbredirhost);
+        }
+    } else {
+        // no physical device support, only emulated, create the
+        // parser
+        ch->parser = create_parser(ch);
+        if (ch->parser != NULL) {
+            initialize_parser(ch);
+        }
     }
 
-    if (ch->usbredirhost) {
-        ch->hidden_parser = create_parser(ch);
-        ch->hidden_host = ch->usbredirhost;
-        usbredirhost_set_buffered_output_size_cb(ch->usbredirhost,
-                                                 usbredir_buffered_output_size_callback);
-    }
-
-    if (!ch->hidden_parser) {
+    if (!ch->parser) {
         spice_usb_backend_channel_delete(ch);
         ch = NULL;
     }
@@ -1330,12 +1334,9 @@ SpiceUsbBackendChannel *spice_usb_backend_channel_new(SpiceUsbBackend *be,
 void spice_usb_backend_channel_flush_writes(SpiceUsbBackendChannel *ch)
 {
     SPICE_DEBUG("%s %p is up", __FUNCTION__, ch);
-    if (ch->usbredirhost) {
+    if (ch->state != USB_CHANNEL_STATE_PARSER && ch->usbredirhost != NULL) {
         usbredirhost_write_guest_data(ch->usbredirhost);
-        initialize_parser(ch, FALSE);
     } else {
-        initialize_parser(ch, TRUE);
-        ch->parser = ch->hidden_parser;
         usbredirparser_do_write(ch->parser);
     }
 }
@@ -1346,11 +1347,11 @@ void spice_usb_backend_channel_delete(SpiceUsbBackendChannel *ch)
     if (!ch) {
         return;
     }
-    if (ch->hidden_host) {
-        usbredirhost_close(ch->hidden_host);
+    if (ch->usbredirhost) {
+        usbredirhost_close(ch->usbredirhost);
     }
-    if (ch->hidden_parser) {
-        usbredirparser_destroy(ch->hidden_parser);
+    if (ch->parser) {
+        usbredirparser_destroy(ch->parser);
     }
 
     if (ch->rules) {
@@ -1370,7 +1371,7 @@ spice_usb_backend_channel_get_guest_filter(SpiceUsbBackendChannel *ch,
     int i;
     *r = NULL;
     *count = 0;
-    if (ch->usbredirhost) {
+    if (ch->usbredirhost != NULL) {
         usbredirhost_get_guest_filter(ch->usbredirhost, r, count);
     }
     if (*r == NULL) {
-- 
2.20.1



More information about the Spice-devel mailing list