[Spice-devel] [PATCH spice-gtk 04/10] usbredir: Use new libusbredirhost write flush callback
Christophe Fergeau
cfergeau at redhat.com
Fri Dec 23 09:26:22 PST 2011
On Mon, Dec 19, 2011 at 12:24:37PM +0100, Hans de Goede wrote:
> The new (in usbredir-0.3.2) usbredirhost_open_full()
> function allows us to be notified whenever usb packets completing
> result in data to be send to the host. This removes the need for using
> g_usb_source_set_callback and seeing if there is data to write on
> any of the usb channels each time some usb event happens.
>
> This also bumps the minimum needed libusbredirhost version to 0.3.2
>
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
> configure.ac | 2 +-
> gtk/channel-usbredir-priv.h | 2 --
> gtk/channel-usbredir.c | 43 +++++++++++++++++++++----------------------
> gtk/usb-device-manager.c | 23 -----------------------
> 4 files changed, 22 insertions(+), 48 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index e38aad1..db9ff7d 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -327,7 +327,7 @@ if test "x$enable_usbredir" = "xno"; then
> else
> PKG_CHECK_MODULES(GUDEV, gudev-1.0)
> PKG_CHECK_MODULES(LIBUSB, libusb-1.0 >= 1.0.9)
> - PKG_CHECK_MODULES(LIBUSBREDIRHOST, libusbredirhost >= 0.3.1)
> + PKG_CHECK_MODULES(LIBUSBREDIRHOST, libusbredirhost >= 0.3.2)
> AC_DEFINE(USE_USBREDIR, [1], [Define if supporting usbredir proxying])
> AM_CONDITIONAL(WITH_USBREDIR, true)
> if test "x$enable_polkit" = "xno"; then
> diff --git a/gtk/channel-usbredir-priv.h b/gtk/channel-usbredir-priv.h
> index 8bb42a5..305e897 100644
> --- a/gtk/channel-usbredir-priv.h
> +++ b/gtk/channel-usbredir-priv.h
> @@ -42,8 +42,6 @@ void spice_usbredir_channel_disconnect(SpiceUsbredirChannel *channel);
>
> GUsbDevice *spice_usbredir_channel_get_device(SpiceUsbredirChannel *channel);
>
> -void spice_usbredir_channel_do_write(SpiceUsbredirChannel *channel);
> -
> G_END_DECLS
>
> #endif /* __SPICE_CLIENT_USBREDIR_CHANNEL_PRIV_H__ */
> diff --git a/gtk/channel-usbredir.c b/gtk/channel-usbredir.c
> index 6574e32..58a4c21 100644
> --- a/gtk/channel-usbredir.c
> +++ b/gtk/channel-usbredir.c
> @@ -73,7 +73,6 @@ struct _SpiceUsbredirChannelPrivate {
> /* Data passed from channel handle msg to the usbredirhost read cb */
> const uint8_t *read_buf;
> int read_buf_size;
> - SpiceMsgOut *msg_out;
> enum SpiceUsbredirChannelState state;
> #if USE_POLKIT
> GSimpleAsyncResult *result;
> @@ -89,6 +88,7 @@ static void usbredir_handle_msg(SpiceChannel *channel, SpiceMsgIn *in);
> static void usbredir_log(void *user_data, int level, const char *msg);
> static int usbredir_read_callback(void *user_data, uint8_t *data, int count);
> static int usbredir_write_callback(void *user_data, uint8_t *data, int count);
> +static void usbredir_write_flush_callback(void *user_data);
> #endif
>
> G_DEFINE_TYPE(SpiceUsbredirChannel, spice_usbredir_channel, SPICE_TYPE_CHANNEL)
> @@ -178,10 +178,16 @@ static gboolean spice_usbredir_channel_open_device(
> }
>
> priv->catch_error = err;
> - priv->host = usbredirhost_open(_g_usb_context_get_context(priv->context),
> + priv->host = usbredirhost_open_full(
> + _g_usb_context_get_context(priv->context),
> handle, usbredir_log,
> usbredir_read_callback,
> usbredir_write_callback,
> + usbredir_write_flush_callback,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> channel, PACKAGE_STRING,
> spice_util_get_debug() ? usbredirparser_debug : usbredirparser_warning,
> usbredirhost_fl_write_cb_owns_buffer);
> @@ -345,26 +351,17 @@ GUsbDevice *spice_usbredir_channel_get_device(SpiceUsbredirChannel *channel)
> return channel->priv->device;
> }
>
> -G_GNUC_INTERNAL
> -void spice_usbredir_channel_do_write(SpiceUsbredirChannel *channel)
> +/* Note that this function must be re-entrant safe, as it can get called
> + from both the main thread as well as from the usb event handling thread */
> +static void usbredir_write_flush_callback(void *user_data)
> {
> + SpiceUsbredirChannel *channel = SPICE_USBREDIR_CHANNEL(user_data);
> SpiceUsbredirChannelPrivate *priv = channel->priv;
>
> - /* No recursion allowed! */
> - g_return_if_fail(priv->msg_out == NULL);
> -
> - if (priv->state != STATE_CONNECTED ||
> - !usbredirhost_has_data_to_write(priv->host))
> + if (priv->state != STATE_CONNECTED)
If this function can run from the usb event handling thread, is there
something preventing the check on priv->state from racing with code running
in the main thread?
Christophe
> return;
>
> - priv->msg_out = spice_msg_out_new(SPICE_CHANNEL(channel),
> - SPICE_MSGC_SPICEVMC_DATA);
> -
> - /* Collect all pending writes in priv->msg_out->marshaller */
> usbredirhost_write_guest_data(priv->host);
> -
> - spice_msg_out_send(priv->msg_out);
> - priv->msg_out = NULL;
> }
>
> /* ------------------------------------------------------------------ */
> @@ -424,10 +421,14 @@ static void usbredir_free_write_cb_data(uint8_t *data, void *user_data)
> static int usbredir_write_callback(void *user_data, uint8_t *data, int count)
> {
> SpiceUsbredirChannel *channel = user_data;
> - SpiceUsbredirChannelPrivate *priv = channel->priv;
> -
> - spice_marshaller_add_ref_full(priv->msg_out->marshaller, data, count,
> + SpiceMsgOut *msg_out;
> +
> + msg_out = spice_msg_out_new(SPICE_CHANNEL(channel),
> + SPICE_MSGC_SPICEVMC_DATA);
> + spice_marshaller_add_ref_full(msg_out->marshaller, data, count,
> usbredir_free_write_cb_data, channel);
> + spice_msg_out_send(msg_out);
> +
> return count;
> }
>
> @@ -459,7 +460,7 @@ static void spice_usbredir_channel_up(SpiceChannel *c)
>
> priv->state = STATE_CONNECTED;
> /* Flush any pending writes */
> - spice_usbredir_channel_do_write(channel);
> + usbredirhost_write_guest_data(priv->host);
> }
>
> static void usbredir_handle_msg(SpiceChannel *c, SpiceMsgIn *in)
> @@ -479,8 +480,6 @@ static void usbredir_handle_msg(SpiceChannel *c, SpiceMsgIn *in)
> priv->read_buf_size = size;
>
> usbredirhost_read_guest_data(priv->host);
> - /* Send any acks, etc. which may be queued now */
> - spice_usbredir_channel_do_write(channel);
> }
>
> #endif /* USE_USBREDIR */
> diff --git a/gtk/usb-device-manager.c b/gtk/usb-device-manager.c
> index ea43b75..fcd1685 100644
> --- a/gtk/usb-device-manager.c
> +++ b/gtk/usb-device-manager.c
> @@ -404,25 +404,6 @@ static void channel_destroy(SpiceSession *session, SpiceChannel *channel,
> }
>
> #ifdef USE_USBREDIR
> -static gboolean spice_usb_device_manager_source_callback(gpointer user_data)
> -{
> - SpiceUsbDeviceManager *self = user_data;
> - SpiceUsbDeviceManagerPrivate *priv = self->priv;
> - guint i;
> -
> - /*
> - * Flush any writes which may have been caused by async usb packets
> - * completing.
> - */
> - for (i = 0; i < priv->channels->len; i++) {
> - SpiceUsbredirChannel *channel = g_ptr_array_index(priv->channels, i);
> -
> - spice_usbredir_channel_do_write(channel);
> - }
> -
> - return TRUE;
> -}
> -
> static void spice_usb_device_manager_auto_connect_cb(GObject *gobject,
> GAsyncResult *res,
> gpointer user_data)
> @@ -638,10 +619,6 @@ void spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,
> g_simple_async_result_take_error(result, e);
> goto done;
> }
> -
> - g_usb_source_set_callback(priv->source,
> - spice_usb_device_manager_source_callback,
> - self, NULL);
> }
>
> for (i = 0; i < priv->channels->len; i++) {
> --
> 1.7.7.4
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20111223/111b72c7/attachment-0001.pgp>
More information about the Spice-devel
mailing list