[Spice-devel] [spice-gtk v3 5/9] usb-redir: extend USB backend to support emulated devices
Frediano Ziglio
fziglio at redhat.com
Tue Aug 13 11:59:03 UTC 2019
>
> Redirection of emulated devices requires special approach,
> as usbredirhost can't be used for that (it works only with
> libusb devices). For emulated devices we create instance of
> usbredirparser that implements USB redirection protocol.
Make sense. Actually usbredirhost internally uses a parser to
parse protocol messages and redirect to the physical device.
But the parse is internal and is used by usbredirhost so
would be hard to reuse.
> In order to work with the same set of protocol capabilities
> that usbredirhost sets up with remote side, the parser shall:
> - not send its own 'hello' to the server
> - initialize the same capabilities that usbredirhost
> - receive the same 'hello' response
> For that we analyze 'hello' sent by USB redir parser and
> extract set of capabilities from it and upon receive of
> 'hello' response we provide it also to the parser.
This make sense but some part of the code does not exactly that.
In some path the code send the 'hello' is sent by the parser.
It would be more clear to implement just what you described
above, why the implementation is a bit different.
> When libusb device is redirected via a channel, instance of
> usbredirhost serves it.
> When emulated device is redirected via a channel, instance
> of usbredirparser does the job.
>
> Signed-off-by: Yuri Benditovich <yuri.benditovich at daynix.com>
> ---
> src/usb-backend.c | 616 +++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 580 insertions(+), 36 deletions(-)
>
> diff --git a/src/usb-backend.c b/src/usb-backend.c
> index aa11c79..9aee6c7 100644
> --- a/src/usb-backend.c
> +++ b/src/usb-backend.c
> @@ -55,6 +55,7 @@ struct _SpiceUsbBackendDevice
> gint ref_count;
> SpiceUsbBackendChannel *attached_to;
> UsbDeviceInformation device_info;
> + gboolean edev_configured;
This variable is never reset, I would expect that at least
when device is detached (this should be equivalent to physical
removal).
Why not moving to SpiceUsbBackendChannel?
> };
>
> struct _SpiceUsbBackend
> @@ -80,10 +81,20 @@ struct _SpiceUsbBackend
> struct _SpiceUsbBackendChannel
> {
> struct usbredirhost *usbredirhost;
> + struct usbredirhost *hidden_host;
> + struct usbredirparser *parser;
> + struct usbredirparser *hidden_parser;
I don't understand all these variable.
Won't be easier to have parser initialized for emulated
devices and being NULL (so freed) when emulated device
is detached?
> 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_disc_ack : 1;
The "disc" naming is confusing, also taking into account the
"Compact Disc" addition. Reading the code this "disc" is a shorten
for "disconnect", maybe would be better to just use "wait_disconnect_ack"?
> SpiceUsbBackendDevice *attached;
> SpiceUsbredirChannel *user_data;
OT: this definitively should be renamed!
> SpiceUsbBackend *backend;
> @@ -355,7 +366,11 @@ gboolean
> spice_usb_backend_device_isoch(SpiceUsbBackendDevice *dev)
> gint i, j, k;
> int rc;
>
> - g_return_val_if_fail(libdev != NULL, 0);
> + g_return_val_if_fail(libdev != NULL || dev->edev != NULL, 0);
> + if (dev->edev) {
> + /* currently we do not emulate isoch devices */
> + return FALSE;
> + }
>
> rc = libusb_get_active_config_descriptor(libdev, &conf_desc);
> if (rc) {
> @@ -390,13 +405,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->usbredirhost) {
> - /* just to be on the safe side */
> - return;
> - }
These check should be in a preparatory patch. Nothing wrong with this patch,
but the code (master) is confusing. usbredirhost is initialized as soon as
possible, can be NULL only before usbredirhost_open_full returns.
Actually can be NULL in this call for this reason (this callback is called
inside usbredirhost_open_full).
> if (is_channel_ready(ch->user_data)) {
> - SPICE_DEBUG("%s ch %p -> usbredirhost", __FUNCTION__, ch);
> - usbredirhost_write_guest_data(ch->usbredirhost);
> + if (ch->usbredirhost) {
> + SPICE_DEBUG("%s ch %p -> usbredirhost", __FUNCTION__, ch);
> + usbredirhost_write_guest_data(ch->usbredirhost);
> + } else if (ch->parser) {
> + SPICE_DEBUG("%s ch %p -> parser", __FUNCTION__, ch);
> + usbredirparser_do_write(ch->parser);
> + } else {
> + SPICE_DEBUG("%s ch %p (NOT ACTIVE)", __FUNCTION__, ch);
This third case should never be possible. This is one aspect that master
code is confusing about.
> + }
> } else {
> SPICE_DEBUG("%s ch %p (not ready)", __FUNCTION__, ch);
> }
> @@ -551,13 +569,52 @@ void
> spice_usb_backend_device_unref(SpiceUsbBackendDevice *dev)
> }
> }
>
> -int spice_usb_backend_device_check_filter(
> - SpiceUsbBackendDevice *dev,
> - const struct usbredirfilter_rule *rules,
> - int count)
> +static int check_edev_device_filter(SpiceUsbBackendDevice *dev,
> + const struct usbredirfilter_rule *rules,
> + int count)
> {
> - return usbredirhost_check_device_filter(
> - rules, count, dev->libusb_device, 0);
> + SpiceUsbEmulatedDevice *edev = dev->edev;
> + uint8_t cls[32], subcls[32], proto[32], *cfg, ifnum = 0;
> + uint16_t size, offset = 0;
> + if (!edev) {
> + return 1;
> + }
> + if (!device_ops(edev)->get_descriptor(edev, LIBUSB_DT_CONFIG, 0, (void
> **)&cfg, &size)) {
> + return 1;
> + }
> + while ((offset + 1) < size) {
> + uint8_t len = cfg[offset];
> + uint8_t type = cfg[offset + 1];
> + if ((offset + len) > size) {
> + break;
> + }
> + if (type == LIBUSB_DT_INTERFACE) {
> + cls[ifnum] = cfg[offset + 5];
> + subcls[ifnum] = cfg[offset + 6];
> + proto[ifnum] = cfg[offset + 7];
> + ifnum++;
> + }
> + offset += len;
> + }
> +
> + return usbredirfilter_check(rules, count,
> + dev->device_info.class,
> + dev->device_info.subclass,
> + dev->device_info.protocol,
> + cls, subcls, proto, ifnum,
> + dev->device_info.vid,
> + dev->device_info.pid,
> + dev->device_info.bcdUSB, 0);
> +}
> +
> +int spice_usb_backend_device_check_filter(SpiceUsbBackendDevice *dev,
> + const struct usbredirfilter_rule
> *rules, int count)
> +{
> + if (dev->libusb_device) {
> + return usbredirhost_check_device_filter(rules, count,
> dev->libusb_device, 0);
> + } else {
> + return check_edev_device_filter(dev, rules, count);
> + }
> }
>
> static int usbredir_read_callback(void *user_data, uint8_t *data, int count)
> @@ -621,6 +678,17 @@ 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) {
Is confusing that the same callback is used for both usbredir parser and
usbredir host but the hello should be parsed only for the host.
Maybe disabling hello for the parser and check the type (hello) here?
> + /* hello is short header (12) + hello struct (64) + capabilities (4)
> */
> + int hello_size = sizeof(struct usb_redir_header) + sizeof(struct
> usb_redir_hello_header);
This formulae is confusing with the description. sizeof(struct usb_redir_header)
is 16 but is not the short (32bit) header.
> + ch->hello_sent = 1;
> + if (count == hello_size) {
I would say "count >= hello_size" to support future extensions of the
capability array.
> + 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);
> + }
> + }
> res = spice_usbredir_write_callback(ch->user_data, data, count);
> return res;
> }
> @@ -641,6 +709,27 @@ int
> spice_usb_backend_read_guest_data(SpiceUsbBackendChannel *ch, uint8_t *data,
> ch->read_buf_size = count;
> if (ch->usbredirhost) {
Maybe this could be "ch->usbredirhost || !ch->hello_done_parser" to
parse the 'hello' always with usbredir host ?
> res = usbredirhost_read_guest_data(ch->usbredirhost);
> + if (!ch->hello_done_parser) {
> + int res_parser;
> + /* usbredirhost should consume hello response */
> + g_return_val_if_fail(ch->read_buf == NULL,
> USB_REDIR_ERROR_READ_PARSE);
> +
> + 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;
> + }
> + }
> + } else if (ch->parser) {
> + res = usbredirparser_do_read(ch->parser);
> } else {
> res = USB_REDIR_ERROR_IO;
> }
> @@ -661,6 +750,11 @@ int
> spice_usb_backend_read_guest_data(SpiceUsbBackendChannel *ch, uint8_t *data,
> }
> SPICE_DEBUG("%s ch %p, %d bytes, res %d", __FUNCTION__, ch, count, res);
>
> + if (ch->rejected) {
> + ch->rejected = 0;
> + res = USB_REDIR_ERROR_DEV_REJECTED;
> + }
Why this is after the read? Won't it discard the value?
> +
> return res;
> }
>
> @@ -690,13 +784,426 @@ 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) {
> - SPICE_DEBUG("%s ch %p", __FUNCTION__, ch);
> + SPICE_DEBUG("%s ch %p -> usbredirhost", __FUNCTION__, ch);
> usbredirhost_free_write_buffer(ch->usbredirhost, data);
> + } else if (ch->parser) {
> + SPICE_DEBUG("%s ch %p -> parser", __FUNCTION__, ch);
> + usbredirparser_free_write_buffer(ch->parser, data);
A bit scary that during a swicth host <-> parser this can call the wrong
function. But currently the implementation for both is to free 'data'.
> } else {
> SPICE_DEBUG("%s ch %p - NOBODY TO CALL", __FUNCTION__, ch);
> }
> }
>
> +static void usbredir_control_packet(void *priv,
> + uint64_t id, struct usb_redir_control_packet_header *h,
> + uint8_t *data, int data_len)
This style is new, I suppose copied from usbredir, but is not
spice-gtk.
> +{
> + SpiceUsbBackendChannel *ch = priv;
> + SpiceUsbBackendDevice *d = ch->attached;
> + SpiceUsbEmulatedDevice *edev = d ? d->edev : NULL;
> + struct usb_redir_control_packet_header response = *h;
> + uint8_t reqtype = h->requesttype & 0x7f;
> + gboolean done = FALSE;
> + void *out_buffer = NULL;
> +
> + response.status = usb_redir_stall;
> + SPICE_DEBUG("%s %p: TRVIL %02X %02X %04X %04X %04X",
> + __FUNCTION__,
> + ch, h->requesttype, h->request,
> + h->value, h->index, h->length);
> +
> + if (!edev) {
> + SPICE_DEBUG("%s: device not attached", __FUNCTION__);
> + response.status = usb_redir_ioerror;
> + done = TRUE;
> + } else if (reqtype == (LIBUSB_REQUEST_TYPE_STANDARD |
> LIBUSB_RECIPIENT_DEVICE) &&
> + h->request == LIBUSB_REQUEST_GET_DESCRIPTOR) {
> + uint16_t size;
> + done = device_ops(edev)->get_descriptor(edev, h->value >> 8,
> h->value & 0xff,
> + &out_buffer, &size);
> + response.length = size;
> + if (done) {
> + response.status = 0;
> + }
> + done = TRUE;
> + }
> +
> + if (!done) {
> + device_ops(edev)->control_request(edev, data, data_len, &response,
> &out_buffer);
> + done = TRUE;
> + }
> +
> + if (response.status) {
> + response.length = 0;
> + } else if (response.length > h->length) {
> + response.length = h->length;
> + }
> +
> + SPICE_DEBUG("%s responding with payload of %02X, status %X",
> + __FUNCTION__, response.length, response.status);
> + usbredirparser_send_control_packet(ch->parser, id, &response,
> + response.length ? out_buffer : NULL,
> + response.length);
> +
> + usbredir_write_flush_callback(ch);
> + usbredirparser_free_packet_data(ch->parser, data);
> +}
> +
> +static void usbredir_bulk_packet(void *priv,
> + uint64_t id, struct usb_redir_bulk_packet_header *h,
> + uint8_t *data, int data_len)
> +{
> + SpiceUsbBackendChannel *ch = priv;
> + SpiceUsbBackendDevice *d = ch->attached;
> + SpiceUsbEmulatedDevice *edev = d ? d->edev : NULL;
> + struct usb_redir_bulk_packet_header hout = *h;
> + uint32_t len = (h->length_high << 16) | h->length;
> + SPICE_DEBUG("%s %p: ep %X, len %u, id %" G_GUINT64_FORMAT, __FUNCTION__,
> + ch, h->endpoint, len, id);
> +
> + if (!edev) {
> + SPICE_DEBUG("%s: device not attached", __FUNCTION__);
> + hout.status = usb_redir_ioerror;
> + hout.length = hout.length_high = 0;
> + SPICE_DEBUG("%s: responding with ZLP status %d", __FUNCTION__,
> hout.status);
> + } else if (h->endpoint & LIBUSB_ENDPOINT_IN) {
> + if (device_ops(edev)->bulk_in_request(edev, id, &hout)) {
> + usbredirparser_free_packet_data(ch->parser, data);
> + /* completion is asynchronous */
> + return;
> + }
> + } else {
> + hout.status = usb_redir_stall;
> + device_ops(edev)->bulk_out_request(edev, h->endpoint, data,
> data_len, &hout.status);
> + SPICE_DEBUG("%s: responding status %d", __FUNCTION__, hout.status);
> + }
> +
> + usbredirparser_send_bulk_packet(ch->parser, id, &hout, NULL, 0);
> + usbredirparser_free_packet_data(ch->parser, data);
> + usbredir_write_flush_callback(ch);
> +}
> +
> +static void usbredir_device_reset(void *priv)
> +{
> + SpiceUsbBackendChannel *ch = priv;
> + SpiceUsbBackendDevice *d = ch->attached;
> + SpiceUsbEmulatedDevice *edev = d ? d->edev : NULL;
> + SPICE_DEBUG("%s ch %p", __FUNCTION__, ch);
> + if (edev) {
> + device_ops(edev)->reset(edev);
> + }
> +}
> +
> +static void usbredir_interface_info(void *priv,
> + struct usb_redir_interface_info_header *interface_info)
> +{
> + SpiceUsbBackendChannel *ch = priv;
> + SPICE_DEBUG("%s not implemented %p", __FUNCTION__, ch);
> +}
> +
> +static void usbredir_interface_ep_info(void *priv,
> + struct usb_redir_ep_info_header *ep_info)
> +{
> + SpiceUsbBackendChannel *ch = priv;
> + SPICE_DEBUG("%s not implemented %p", __FUNCTION__, ch);
> +}
> +
> +static void usbredir_set_configuration(void *priv,
> + uint64_t id, struct usb_redir_set_configuration_header
> *set_configuration)
> +{
> + SpiceUsbBackendChannel *ch = priv;
> + struct usb_redir_configuration_status_header h;
> + h.status = 0;
> + h.configuration = set_configuration->configuration;
> + SPICE_DEBUG("%s ch %p, cfg %d", __FUNCTION__, ch, h.configuration);
> + if (ch->attached) {
> + ch->attached->edev_configured = h.configuration != 0;
> + }
> + usbredirparser_send_configuration_status(ch->parser, id, &h);
> + usbredir_write_flush_callback(ch);
> +}
> +
> +static void usbredir_get_configuration(void *priv, uint64_t id)
> +{
> + SpiceUsbBackendChannel *ch = priv;
> + struct usb_redir_configuration_status_header h;
> + h.status = 0;
> + h.configuration = ch->attached && ch->attached->edev_configured;
> + SPICE_DEBUG("%s ch %p, cfg %d", __FUNCTION__, ch, h.configuration);
> + usbredirparser_send_configuration_status(ch->parser, id, &h);
> + usbredir_write_flush_callback(ch);
> +}
> +
> +static void usbredir_set_alt_setting(void *priv,
> + uint64_t id, struct usb_redir_set_alt_setting_header *s)
> +{
> + SpiceUsbBackendChannel *ch = priv;
> + struct usb_redir_alt_setting_status_header sh;
> + sh.status = (!s->interface && !s->alt) ? 0 : usb_redir_stall;
> + sh.interface = s->interface;
> + sh.alt = s->alt;
> + SPICE_DEBUG("%s ch %p, %d:%d", __FUNCTION__, ch, s->interface, s->alt);
> + usbredirparser_send_alt_setting_status(ch->parser, id, &sh);
> + usbredir_write_flush_callback(ch);
> +}
> +
> +static void usbredir_get_alt_setting(void *priv,
> + uint64_t id, struct usb_redir_get_alt_setting_header *s)
> +{
> + SpiceUsbBackendChannel *ch = priv;
> + struct usb_redir_alt_setting_status_header sh;
> + sh.status = (s->interface == 0) ? 0 : usb_redir_stall;
> + sh.interface = s->interface;
> + sh.alt = 0;
> + SPICE_DEBUG("%s ch %p, if %d", __FUNCTION__, ch, s->interface);
> + usbredirparser_send_alt_setting_status(ch->parser, id, &sh);
> + usbredir_write_flush_callback(ch);
> +}
> +
> +static void usbredir_cancel_data(void *priv, uint64_t id)
> +{
> + SpiceUsbBackendChannel *ch = priv;
> + SpiceUsbBackendDevice *d = ch->attached;
> + SpiceUsbEmulatedDevice *edev = d ? d->edev : NULL;
> + if (!edev) {
> + SPICE_DEBUG("%s: device not attached", __FUNCTION__);
> + return;
> + }
> + device_ops(edev)->cancel_request(edev, id);
> +}
> +
> +static void usbredir_filter_reject(void *priv)
> +{
> + SpiceUsbBackendChannel *ch = priv;
> + SPICE_DEBUG("%s %p", __FUNCTION__, ch);
> + ch->rejected = 1;
> +}
> +
> +/* Note that the ownership of the rules array is passed on to the callback.
> */
> +static void usbredir_filter_filter(void *priv,
> + struct usbredirfilter_rule *r, int count)
> +{
> + SpiceUsbBackendChannel *ch = priv;
> + SPICE_DEBUG("%s ch %p %d filters", __FUNCTION__, ch, count);
> +
> + free(ch->rules);
> +
> + ch->rules = r;
> + ch->rules_count = count;
> + if (count) {
> + int i;
> + for (i = 0; i < count; i++) {
> + SPICE_DEBUG("%s class %d, %X:%X",
> + r[i].allow ? "allowed" : "denied", r[i].device_class,
> + (uint32_t)r[i].vendor_id, (uint32_t)r[i].product_id);
> + }
> + }
> +}
> +
> +static void usbredir_device_disconnect_ack(void *priv)
> +{
> + SpiceUsbBackendChannel *ch = priv;
> + SPICE_DEBUG("%s ch %p", __FUNCTION__, ch);
> + if (ch->parser && ch->wait_disc_ack) {
> + ch->parser = NULL;
> + SPICE_DEBUG("%s switch to usbredirhost", __FUNCTION__);
> + ch->usbredirhost = ch->hidden_host;
> + }
> + ch->wait_disc_ack = 0;
> +}
> +
> +static void usbredir_hello(void *priv,
> + struct usb_redir_hello_header *hello)
> +{
> + SpiceUsbBackendChannel *ch = priv;
> + SpiceUsbBackendDevice *d = ch->attached;
> + SpiceUsbEmulatedDevice *edev = d ? d->edev : NULL;
> + struct usb_redir_device_connect_header device_connect;
> + struct usb_redir_ep_info_header ep_info = { 0 };
> + struct usb_redir_interface_info_header interface_info = { 0 };
> + uint8_t *cfg;
> + uint16_t size, offset = 0;
> + SPICE_DEBUG("%s %p %sattached %s", __FUNCTION__, ch,
> + edev ? "" : "not ", hello ? "" : "(internal)");
> +
> + if (hello) {
> + ch->hello_done_parser = 1;
> + }
> + if (!edev) {
> + return;
> + }
> + if (!device_ops(edev)->get_descriptor(edev, LIBUSB_DT_CONFIG, 0, (void
> **)&cfg, &size)) {
> + return;
> + }
> + while ((offset + 1) < size) {
> + uint8_t len = cfg[offset];
> + uint8_t type = cfg[offset + 1];
> + if ((offset + len) > size) {
> + break;
> + }
> + if (type == LIBUSB_DT_INTERFACE) {
> + uint32_t i = interface_info.interface_count;
> + uint8_t class, subclass, protocol;
> + class = cfg[offset + 5];
> + subclass = cfg[offset + 6];
> + protocol = cfg[offset + 7];
> + interface_info.interface_class[i] = class;
> + interface_info.interface_subclass[i] = subclass;
> + interface_info.interface_protocol[i] = protocol;
> + interface_info.interface_count++;
> + SPICE_DEBUG("%s IF%d: %d/%d/%d", __FUNCTION__, i, class,
> subclass, protocol);
> + } else if (type == LIBUSB_DT_ENDPOINT) {
> + uint8_t address = cfg[offset + 2];
> + uint16_t max_packet_size = 0x100 * cfg[offset + 5] + cfg[offset
> + 4];
> + uint8_t index = address & 0xf;
> + if (address & 0x80) index += 0x10;
> + ep_info.type[index] = cfg[offset + 3] & 0x3;
> + ep_info.max_packet_size[index] = max_packet_size;
> + SPICE_DEBUG("%s EP[%02X]: %d/%d", __FUNCTION__, index,
> ep_info.type[index], max_packet_size);
> + }
> + offset += len;
> + }
> +
> + usbredirparser_send_interface_info(ch->parser, &interface_info);
> + usbredirparser_send_ep_info(ch->parser, &ep_info);
> +
> + device_connect.device_class = 0; //d->device_info.class;
> + device_connect.device_subclass = 0; //d->device_info.subclass;
> + device_connect.device_protocol = 0; //d->device_info.protocol;;
> + device_connect.vendor_id = d->device_info.vid;
> + device_connect.product_id = d->device_info.pid;
> + device_connect.device_version_bcd = d->device_info.bcdUSB;
> + device_connect.speed = usb_redir_speed_high;
> + usbredirparser_send_device_connect(ch->parser, &device_connect);
> + usbredir_write_flush_callback(ch);
> +}
> +
> +static void initialize_parser(SpiceUsbBackendChannel *ch, gboolean do_hello)
> +{
> + 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;
> + }
> +
> + 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;
In which case you need to do the hello in the parser instead of the host?
This could cause issues switching from parser back to host in case you
remove an emulated device and you add a real one.
> + 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_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);
> +}
> +
> +/*
> + We can initialize the usbredirparser with HELLO enabled only in case
> + the libusb is not active and the usbredirhost does not function.
> + Then the parser sends session HELLO and receives server's response.
> + Otherwise (usbredirparser initialized with HELLO disabled):
> + - the usbredirhost sends session HELLO
> + - we look into it to know set of capabilities we shall initialize
> + the parser with
> + - when we receive server's response to HELLO we provide it also to
> + parser to let it synchronize with server's capabilities
> +*/
> +static struct usbredirparser *create_parser(SpiceUsbBackendChannel *ch)
> +{
> + struct usbredirparser *parser = usbredirparser_create();
> +
> + g_return_val_if_fail(parser != NULL, NULL);
> +
> + parser->priv = ch;
> + parser->log_func = usbredir_log;
> + parser->read_func = usbredir_read_callback;
> + parser->write_func = usbredir_write_callback;
> + parser->reset_func = usbredir_device_reset;
> + parser->set_configuration_func = usbredir_set_configuration;
> + parser->get_configuration_func = usbredir_get_configuration;
> + parser->set_alt_setting_func = usbredir_set_alt_setting;
> + parser->get_alt_setting_func = usbredir_get_alt_setting;
> + parser->cancel_data_packet_func = usbredir_cancel_data;
> + parser->control_packet_func = usbredir_control_packet;
> + parser->bulk_packet_func = usbredir_bulk_packet;
> + parser->alloc_lock_func = usbredir_alloc_lock;
> + parser->lock_func = usbredir_lock_lock;
> + parser->unlock_func = usbredir_unlock_lock;
> + parser->free_lock_func = usbredir_free_lock;
> + parser->hello_func = usbredir_hello;
> + parser->filter_reject_func = usbredir_filter_reject;
> + parser->device_disconnect_ack_func = usbredir_device_disconnect_ack;
> + parser->interface_info_func = usbredir_interface_info;
> + parser->ep_info_func = usbredir_interface_ep_info;
> + parser->filter_filter_func = usbredir_filter_filter;
> +
> + return parser;
> +}
> +
> +static gboolean attach_edev(SpiceUsbBackendChannel *ch,
> + SpiceUsbBackendDevice *dev,
> + GError **error)
> +{
> + if (!dev->edev) {
> + g_set_error(error, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> + _("Failed to redirect device %d"), 1);
Minor: I would say more an assert level error, the device should be real or emulated.
> + return FALSE;
> + }
> + if (ch->usbredirhost && !ch->hello_done_parser) {
> + /*
> + we can't initialize parser until we see hello from usbredir
> + and the parser can't work until it sees the hello response.
> + this is case of autoconnect when emulated device is attached
> + before the channel is up
> + */
> + 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->wait_disc_ack = 0;
> + ch->attached = dev;
> + dev->attached_to = ch;
> + device_ops(dev->edev)->attach(dev->edev, ch->hidden_parser);
> + if (ch->hello_done_parser) {
> + /* send device info */
> + usbredir_hello(ch, NULL);
> + }
> + return TRUE;
> +}
> +
> gboolean spice_usb_backend_channel_attach(SpiceUsbBackendChannel *ch,
> SpiceUsbBackendDevice *dev,
> GError **error)
> @@ -706,7 +1213,13 @@ gboolean
> spice_usb_backend_channel_attach(SpiceUsbBackendChannel *ch,
>
> g_return_val_if_fail(dev != NULL, FALSE);
>
> + if (!dev->libusb_device) {
> + return attach_edev(ch, dev, error);
> + }
> +
> libusb_device_handle *handle = NULL;
> + ch->usbredirhost = ch->hidden_host;
> + ch->parser = NULL;
>
> /*
> Under Windows we need to avoid updating
> @@ -740,18 +1253,33 @@ gboolean
> spice_usb_backend_channel_attach(SpiceUsbBackendChannel *ch,
>
> void spice_usb_backend_channel_detach(SpiceUsbBackendChannel *ch)
> {
> + SpiceUsbBackendDevice *d = ch->attached;
> + SpiceUsbEmulatedDevice *edev = d ? d->edev : NULL;
> SPICE_DEBUG("%s >> ch %p, was attached %p", __FUNCTION__, ch,
> ch->attached);
> - if (!ch->attached) {
> + if (!d) {
> SPICE_DEBUG("%s: nothing to detach", __FUNCTION__);
> return;
> }
> if (ch->usbredirhost) {
> /* it will call libusb_close internally */
> usbredirhost_set_device(ch->usbredirhost, NULL);
> + } else if (ch->parser) {
> + if (edev) {
> + device_ops(edev)->detach(edev);
> + }
> + usbredirparser_send_device_disconnect(ch->parser);
> + usbredir_write_flush_callback(ch);
> + ch->wait_disc_ack = usbredirparser_peer_has_cap(ch->parser,
> +
> usb_redir_cap_device_disconnect_ack);
> + if (!ch->wait_disc_ack) {
> + ch->usbredirhost = ch->hidden_host;
> + ch->parser = NULL;
> + }
> }
> SPICE_DEBUG("%s ch %p, detach done", __FUNCTION__, ch);
> ch->attached->attached_to = NULL;
> ch->attached = NULL;
> + ch->rejected = 0;
> }
>
> SpiceUsbBackendChannel *spice_usb_backend_channel_new(SpiceUsbBackend *be,
> @@ -766,26 +1294,33 @@ SpiceUsbBackendChannel
> *spice_usb_backend_channel_new(SpiceUsbBackend *be,
> ch->user_data = SPICE_USBREDIR_CHANNEL(user_data);
> if (be->libusb_context) {
This can never be NULL, if we are not able to allocate it inside
spice_usb_backend_new the function returns NULL.
This should be in a preliminary patch to make this clear.
> ch->backend = be;
> - ch->usbredirhost = usbredirhost_open_full(
> - be->libusb_context,
> - NULL,
> - usbredir_log,
> - usbredir_read_callback,
> - usbredir_write_callback,
> - usbredir_write_flush_callback,
> - usbredir_alloc_lock,
> - usbredir_lock_lock,
> - usbredir_unlock_lock,
> - usbredir_free_lock,
> - ch, PACKAGE_STRING,
> - spice_util_get_debug() ? usbredirparser_debug :
> usbredirparser_warning,
> - usbredirhost_fl_write_cb_owns_buffer);
> + ch->usbredirhost =
> + usbredirhost_open_full(be->libusb_context,
> + NULL,
> + usbredir_log,
> + usbredir_read_callback,
> + usbredir_write_callback,
> + usbredir_write_flush_callback,
> + usbredir_alloc_lock,
> + usbredir_lock_lock,
> + usbredir_unlock_lock,
> + usbredir_free_lock,
> + ch, PACKAGE_STRING,
> + spice_util_get_debug() ?
> usbredirparser_debug :
> + usbredirparser_warning,
> + usbredirhost_fl_write_cb_owns_buffer);
> g_warn_if_fail(ch->usbredirhost != NULL);
> }
> +
> if (ch->usbredirhost) {
> - usbredirhost_set_buffered_output_size_cb(ch->usbredirhost,
> usbredir_buffered_output_size_callback);
> - } else {
> - g_free(ch);
> + 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) {
> + spice_usb_backend_channel_delete(ch);
> ch = NULL;
> }
>
> @@ -795,9 +1330,14 @@ SpiceUsbBackendChannel
> *spice_usb_backend_channel_new(SpiceUsbBackend *be,
>
> void spice_usb_backend_channel_flush_writes(SpiceUsbBackendChannel *ch)
> {
> - SPICE_DEBUG("%s %p, host %p", __FUNCTION__, ch, ch->usbredirhost);
> + SPICE_DEBUG("%s %p is up", __FUNCTION__, ch);
> if (ch->usbredirhost) {
> usbredirhost_write_guest_data(ch->usbredirhost);
> + initialize_parser(ch, FALSE);
> + } else {
> + initialize_parser(ch, TRUE);
Why initialize_parse is called here?
> + ch->parser = ch->hidden_parser;
> + usbredirparser_do_write(ch->parser);
> }
> }
>
> @@ -807,12 +1347,16 @@ void
> spice_usb_backend_channel_delete(SpiceUsbBackendChannel *ch)
> if (!ch) {
> return;
> }
> - if (ch->usbredirhost) {
> - usbredirhost_close(ch->usbredirhost);
> + if (ch->hidden_host) {
> + usbredirhost_close(ch->hidden_host);
> + }
> + if (ch->hidden_parser) {
> + usbredirparser_destroy(ch->hidden_parser);
> }
>
> if (ch->rules) {
> - g_free(ch->rules);
> + /* rules were allocated by usbredirparser */
> + free(ch->rules);
Minor :I suppose this small change can go to a preliminary patch too
> }
>
> g_free(ch);
Frediano
More information about the Spice-devel
mailing list