[Spice-devel] [spice-gtk 09/13] channel: talk to giostream instead of gsocket

Christophe Fergeau cfergeau at redhat.com
Tue Feb 11 09:30:28 PST 2014


On Mon, Feb 03, 2014 at 07:02:40PM +0100, Marc-André Lureau wrote:
> From: Marc-André Lureau <marcandre.lureau at redhat.com>
> 
> When using glib >= 2.28, use stream API rather than gsocket directly.
> 
> This allows for more flexible streams usage, including encoded streams
> from proxy connections.
> ---
>  gtk/spice-channel-priv.h |  2 ++
>  gtk/spice-channel.c      | 46 ++++++++++++++++++++++++++++++++++------------
>  spice-common             |  2 +-
>  3 files changed, 37 insertions(+), 13 deletions(-)
> 
> diff --git a/gtk/spice-channel-priv.h b/gtk/spice-channel-priv.h
> index 0816061..fd08b2a 100644
> --- a/gtk/spice-channel-priv.h
> +++ b/gtk/spice-channel-priv.h
> @@ -82,6 +82,8 @@ struct _SpiceChannelPrivate {
>      SpiceOpenSSLVerify          *sslverify;
>      GSocket                     *sock;
>      GSocketConnection           *conn;
> +    GInputStream                *in;
> +    GOutputStream               *out;
>  
>  #if HAVE_SASL
>      sasl_conn_t                 *sasl_conn;
> diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
> index 9e97f28..7459a49 100644
> --- a/gtk/spice-channel.c
> +++ b/gtk/spice-channel.c
> @@ -763,7 +763,8 @@ static void spice_channel_flush_wire(SpiceChannel *channel,
>      GIOCondition cond;
>  
>      while (offset < datalen) {
> -        int ret;
> +        gssize ret;
> +        GError *error = NULL;
>  
>          if (c->has_error) return;
>  
> @@ -779,9 +780,12 @@ static void spice_channel_flush_wire(SpiceChannel *channel,
>                  ret = -1;
>              }
>          } else {
> -            GError *error = NULL;
> -            ret = g_socket_send(c->sock, ptr+offset, datalen-offset,
> -                                NULL, &error);
> +#if GLIB_CHECK_VERSION(2, 28, 0)
> +            ret = g_pollable_output_stream_write_nonblocking(G_POLLABLE_OUTPUT_STREAM(c->out),
> +                                                             ptr+offset, datalen-offset, NULL, &error);
> +#else
> +            ret = g_socket_send(c->sock, ptr+offset, datalen-offset, NULL, &error);
> +#endif
>              if (ret < 0) {
>                  if (g_error_matches(error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK)) {
>                      cond = G_IO_OUT;
> @@ -794,6 +798,7 @@ static void spice_channel_flush_wire(SpiceChannel *channel,
>          }
>          if (ret == -1) {
>              if (cond != 0) {
> +                // TODO: should use g_pollable_input/output_stream_create_source() in 2.28 ?
>                  g_coroutine_socket_wait(&c->coroutine, c->sock, cond);
>                  continue;
>              } else {
> @@ -888,7 +893,7 @@ static void spice_channel_write_msg(SpiceChannel *channel, SpiceMsgOut *out)
>  static int spice_channel_read_wire(SpiceChannel *channel, void *data, size_t len)
>  {
>      SpiceChannelPrivate *c = channel->priv;
> -    int ret;
> +    gssize ret;
>      GIOCondition cond;
>  
>  reread:
> @@ -908,7 +913,13 @@ reread:
>          }
>      } else {
>          GError *error = NULL;
> -        ret = g_socket_receive(c->sock, data, len, NULL, &error);
> +#if GLIB_CHECK_VERSION(2, 28, 0)
> +        ret = g_pollable_input_stream_read_nonblocking(G_POLLABLE_INPUT_STREAM(c->in),
> +                                                       data, len, NULL, &error);
> +#else
> +        ret = g_socket_receive(c->sock,
> +                               data, len, NULL, &error);
> +#endif
>          if (ret < 0) {
>              if (g_error_matches(error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK)) {
>                  cond = G_IO_IN;
> @@ -922,6 +933,7 @@ reread:
>  
>      if (ret == -1) {
>          if (cond != 0) {
> +            // TODO: should use g_pollable_input/output_stream_create_source() ?
>              g_coroutine_socket_wait(&c->coroutine, c->sock, cond);
>              goto reread;
>          } else {
> @@ -2061,7 +2073,7 @@ static void spice_channel_iterate_read(SpiceChannel *channel)
>      /* treat all incoming data (block on message completion) */
>      while (!c->has_error &&
>             c->state != SPICE_CHANNEL_STATE_MIGRATING &&
> -           g_socket_condition_check(c->sock, G_IO_IN) & G_IO_IN) {
> +           g_pollable_input_stream_is_readable(G_POLLABLE_INPUT_STREAM(c->in))) {

Doesn't this need #if GLIB_CHECK_VERSION(2, 28, 0) as well?

>  
>          do
>              spice_channel_recv_msg(channel,
> @@ -2236,9 +2248,11 @@ static void *spice_channel_coroutine(void *data)
>  
>          g_socket_set_blocking(c->sock, FALSE);
>          g_socket_set_keepalive(c->sock, TRUE);
> +        c->conn = g_socket_connection_factory_create_connection(c->sock);
>          goto connected;
>      }
>  
> +

Dunno if this blank space addition is intentional.

>  reconnect:
>      c->conn = spice_session_channel_open_host(c->session, channel, &c->tls);
>      if (c->conn == NULL) {
> @@ -2299,7 +2313,11 @@ reconnect:
>          }
>  
>  
> +#if GLIB_CHECK_VERSION(2, 28, 0)
> +        BIO *bio = bio_new_giostream(G_IO_STREAM(c->conn));
> +#else
>          BIO *bio = bio_new_gsocket(c->sock);
> +#endif
>          SSL_set_bio(c->ssl, bio, bio);
>  
>          {
> @@ -2330,6 +2348,9 @@ ssl_reconnect:
>      }
>  
>  connected:
> +    c->in = g_io_stream_get_input_stream(G_IO_STREAM(c->conn));
> +    c->out = g_io_stream_get_output_stream(G_IO_STREAM(c->conn));
> +
>      rc = setsockopt(g_socket_get_fd(c->sock), IPPROTO_TCP, TCP_NODELAY,
>                      (const char*)&delay_val, sizeof(delay_val));
>      if ((rc != 0)
> @@ -2503,10 +2524,9 @@ static void channel_reset(SpiceChannel *channel, gboolean migrating)
>          g_object_unref(c->conn);
>          c->conn = NULL;
>      }
> -    if (c->sock) {
> -        g_object_unref(c->sock);
> -        c->sock = NULL;
> -    }
> +
> +    g_clear_object(&c->sock);
> +
>      c->fd = -1;
>  
>      free(c->peer_msg);
> @@ -2732,8 +2752,10 @@ void spice_channel_swap(SpiceChannel *channel, SpiceChannel *swap, gboolean swap
>  
>      /* TODO: split channel in 2 objects: a controller and a swappable
>         state object */
> -    SWAP(conn);
>      SWAP(sock);
> +    SWAP(conn);
> +    SWAP(in);
> +    SWAP(out);
>      SWAP(ctx);
>      SWAP(ssl);
>      SWAP(sslverify);
> diff --git a/spice-common b/spice-common
> index 57ce430..f7a6d3f 160000
> --- a/spice-common
> +++ b/spice-common
> @@ -1 +1 @@
> -Subproject commit 57ce430ccd66bd1ca2447c14503234cfb88e2365
> +Subproject commit f7a6d3fa97b874f87497978babc69d0f65609f31

Is this needed?

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20140211/9871ca57/attachment-0001.pgp>


More information about the Spice-devel mailing list