[Spice-devel] [PATCH spice-gtk 2/5] Use libphodav-2 (breaks webdav server temporarily)

Christophe Fergeau cfergeau at redhat.com
Tue Mar 3 05:26:13 PST 2015


On Sat, Feb 21, 2015 at 01:40:13AM +0100, Marc-André Lureau wrote:
> This change breaks webdav server, since libphodav-2 no longer
> set up a TCP service running in a thread. It's up to the client
> to decide how best to accept and handle new connections.
> 
> This commits remove all the hacks related to proxying the incoming
> connections to a TCP socket, and protected with a magic sequence.
> 
> The following commit will use GIOStream pipes to handle each client
> connections.
> ---
>  configure.ac         |   2 +-
>  gtk/channel-webdav.c | 187 +++------------------------------------------------
>  gtk/spice-session.c  |  41 ++++++-----
>  3 files changed, 29 insertions(+), 201 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index d98e502..84cf568 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -278,7 +278,7 @@ AC_ARG_ENABLE([webdav],
>  if test "x$enable_webdav" = "xno"; then
>    have_phodav="no"
>  else
> -  PKG_CHECK_MODULES(PHODAV, [libphodav-1.0 glib-2.0 >= 2.43.90], [have_phodav=yes], [have_phodav=no])
> +  PKG_CHECK_MODULES(PHODAV, [libphodav-2.0 glib-2.0 >= 2.43.90 libsoup-2.4 >= 2.50], [have_phodav=yes], [have_phodav=no])
>    AC_SUBST(PHODAV_CFLAGS)
>    AC_SUBST(PHODAV_LIBS)
>  
> diff --git a/gtk/channel-webdav.c b/gtk/channel-webdav.c
> index 75b027b..8cf53cf 100644
> --- a/gtk/channel-webdav.c
> +++ b/gtk/channel-webdav.c
> @@ -25,8 +25,6 @@
>  #include "glib-compat.h"
>  #include "vmcstream.h"
>  
> -static PhodavServer* phodav_server_get(SpiceSession *session, gint *port);
> -
>  /**
>   * SECTION:channel-webdav
>   * @short_description: exports a directory
> @@ -187,7 +185,7 @@ typedef struct Client
>  {
>      guint refs;
>      SpiceWebdavChannel *self;
> -    GSocketConnection *conn;
> +    GIOStream *conn;
>      OutputQueue *output;
>      gint64 id;
>      GCancellable *cancellable;
> @@ -327,102 +325,31 @@ static void demux_to_client(SpiceWebdavChannel *self,
>      }
>  }
>  
> -static void magic_written(GObject *source_object,
> -                          GAsyncResult *res,
> -                          gpointer user_data)
> -{
> -    Client *client = user_data;
> -    SpiceWebdavChannel *self = client->self;
> -    SpiceWebdavChannelPrivate *c = self->priv;
> -    gssize bytes_written;
> -    GError *err = NULL;
> -
> -    bytes_written = g_output_stream_write_finish(G_OUTPUT_STREAM(source_object),
> -                                                 res, &err);
> -
> -    if (err || bytes_written != WEBDAV_MAGIC_SIZE)
> -        goto error;
> -
> -    client_start_read(self, client);
> -    g_hash_table_insert(c->clients, &client->id, client);
> -
> -    demux_to_client(self, client);
> -
> -    return;
> -
> -error:
> -    if (err) {
> -        g_critical("socket creation failed %s", err->message);
> -        g_clear_error(&err);
> -    }
> -    if (client) {
> -        client_unref(client);
> -    }
> -}
> -
> -static void client_connected(GObject *source_object,
> -                             GAsyncResult *res,
> -                             gpointer user_data)
> +static void start_client(SpiceWebdavChannel *self)
>  {
> -    SpiceWebdavChannel *self = user_data;
>      SpiceWebdavChannelPrivate *c = self->priv;
> -    GSocketClient *sclient = G_SOCKET_CLIENT(source_object);
> -    GError *err = NULL;
> -    GSocketConnection *conn;
> -    SpiceSession *session;
> -    Client *client = NULL;
>      GOutputStream *output;
> +    Client *client;
>  
> -    session = spice_channel_get_session(SPICE_CHANNEL(self));
> -
> -    conn = g_socket_client_connect_to_host_finish(sclient, res, &err);
> -    g_object_unref(sclient);
> -    if (err)
> -        goto error;
> +    CHANNEL_DEBUG(self, "starting client %" G_GINT64_FORMAT, c->demux.client);
>  
> +    /* FIXME: connect to server */
>      client = g_new0(Client, 1);
>      client->refs = 1;
>      client->id = c->demux.client;
>      client->self = self;
> -    client->conn = conn;
>      client->mux.id = GINT64_TO_LE(client->id);
>      client->mux.buf = g_malloc0(MAX_MUX_SIZE);
>      client->cancellable = g_cancellable_new();
>  
> -    output = g_buffered_output_stream_new(g_io_stream_get_output_stream(G_IO_STREAM(conn)));
> +    output = g_buffered_output_stream_new(g_io_stream_get_output_stream(G_IO_STREAM(client->conn)));
>      client->output = output_queue_new(output);
>      g_object_unref(output);
>  
> -    g_output_stream_write_async(g_io_stream_get_output_stream(G_IO_STREAM(conn)),
> -                                spice_session_get_webdav_magic(session), WEBDAV_MAGIC_SIZE,
> -                                G_PRIORITY_DEFAULT, c->cancellable,
> -                                magic_written, client);
> -    return;
> -
> -error:
> -    if (err) {
> -        g_critical("socket creation failed %s", err->message);
> -        g_clear_error(&err);
> -    }
> -    if (client) {
> -        client_unref(client);
> -    }
> -}
> -
> -static void start_client(SpiceWebdavChannel *self)
> -{
> -    SpiceWebdavChannelPrivate *c = self->priv;
> -    GSocketClient *sclient;
> -    gint davport = -1;
> -    SpiceSession *session;
> -
> -    session = spice_channel_get_session(SPICE_CHANNEL(self));
> -    phodav_server_get(session, &davport);
> -    CHANNEL_DEBUG(self, "starting client %" G_GINT64_FORMAT, c->demux.client);
> +    client_start_read(self, client);
> +    g_hash_table_insert(c->clients, &client->id, client);
>  
> -    sclient = g_socket_client_new();
> -    g_socket_client_connect_to_host_async(sclient, "localhost", davport,
> -                                          c->cancellable, client_connected, self);
> +    demux_to_client(self, client);
>  }
>  
>  static void data_read_cb(GObject *source_object,
> @@ -639,99 +566,3 @@ static void spice_webdav_handle_msg(SpiceChannel *channel, SpiceMsgIn *msg)
>      else
>          g_return_if_reached();
>  }
> -
> -
> -
> -#ifdef USE_PHODAV
> -static void new_connection(SoupSocket *sock,
> -                           SoupSocket *new,
> -                           gpointer    user_data)
> -{
> -    SpiceSession *session = user_data;
> -    SoupAddress *addr;
> -    GSocketAddress *gaddr;
> -    GInetAddress *iaddr;
> -    guint port;
> -    guint8 magic[WEBDAV_MAGIC_SIZE];
> -    gsize nread;
> -    gboolean success = FALSE;
> -    SoupSocketIOStatus status;
> -
> -    /* note: this is sync calls, since webdav server is in a seperate thread */
> -    addr = soup_socket_get_remote_address(new);
> -    gaddr = soup_address_get_gsockaddr(addr);
> -    iaddr = g_inet_socket_address_get_address(G_INET_SOCKET_ADDRESS(gaddr));
> -    port = g_inet_socket_address_get_port(G_INET_SOCKET_ADDRESS(gaddr));
> -
> -    SPICE_DEBUG("port %d %p", port, iaddr);
> -    if (!g_inet_address_get_is_loopback(iaddr)) {
> -        g_warn_if_reached();
> -        goto end;
> -    }
> -
> -    g_object_set(new, "non-blocking", FALSE, NULL);
> -    status = soup_socket_read(new, magic, sizeof(magic), &nread, NULL, NULL);
> -    if (status != SOUP_SOCKET_OK) {
> -        g_warning("bad initial socket read: %d", status);
> -        goto end;
> -    }
> -    g_object_set(new, "non-blocking", TRUE, NULL);
> -
> -    /* check we got the right magic */
> -    if (memcmp(spice_session_get_webdav_magic(session), magic, sizeof(magic))) {
> -        g_warn_if_reached();
> -        goto end;
> -    }
> -
> -    success = TRUE;
> -
> -end:
> -    if (!success) {
> -        g_warn_if_reached();
> -        soup_socket_disconnect(new);
> -        g_signal_stop_emission_by_name(sock, "new_connection");
> -    }
> -    g_object_unref(gaddr);
> -}
> -
> -G_GNUC_INTERNAL
> -PhodavServer* channel_webdav_server_new(SpiceSession *session)
> -{
> -    PhodavServer *dav;
> -    SoupServer *server;
> -    SoupSocket *listener;
> -    const char *shared_dir;
> -
> -    shared_dir = spice_session_get_shared_dir(session);
> -    if (shared_dir == NULL) {
> -        g_debug("No shared dir set, not creating webdav channel");
> -        return NULL;
> -    }
> -
> -    dav = phodav_server_new(0, shared_dir);
> -
> -    server = phodav_server_get_soup_server(dav);
> -    listener = soup_server_get_listener(server);
> -    spice_g_signal_connect_object(listener, "new_connection",
> -                                  G_CALLBACK(new_connection), session,
> -                                  0);
> -
> -    return dav;
> -}
> -#endif /* USE_PHODAV */
> -
> -static PhodavServer* phodav_server_get(SpiceSession *session, gint *port)
> -{
> -    g_return_val_if_fail(SPICE_IS_SESSION(session), NULL);
> -
> -#ifdef USE_PHODAV
> -    PhodavServer *server = spice_session_get_webdav_server(session);
> -
> -    if (port)
> -        *port = phodav_server_get_port(server);
> -
> -    return server;
> -#else
> -    g_return_val_if_reached(NULL);
> -#endif
> -}
> diff --git a/gtk/spice-session.c b/gtk/spice-session.c
> index 82ea55f..ab7b25c 100644
> --- a/gtk/spice-session.c
> +++ b/gtk/spice-session.c
> @@ -122,7 +122,6 @@ struct _SpiceSessionPrivate {
>      SpiceUsbDeviceManager *usb_manager;
>      SpicePlaybackChannel *playback_channel;
>      PhodavServer      *webdav;
> -    guint8             webdav_magic[WEBDAV_MAGIC_SIZE];
>  };
>  
>  
> @@ -2604,36 +2603,34 @@ gboolean spice_session_get_smartcard_enabled(SpiceSession *session)
>  }
>  
>  G_GNUC_INTERNAL
> -const guint8* spice_session_get_webdav_magic(SpiceSession *session)
> -{
> -    g_return_val_if_fail(SPICE_IS_SESSION(session), NULL);
> -
> -    return session->priv->webdav_magic;
> -}
> -
> -G_GNUC_INTERNAL
>  PhodavServer* spice_session_get_webdav_server(SpiceSession *session)
>  {
> +    SpiceSessionPrivate *priv;
> +
>      g_return_val_if_fail(SPICE_IS_SESSION(session), NULL);
> +    priv = session->priv;
>  
>  #ifdef USE_PHODAV
> -    static GStaticMutex mutex = G_STATIC_MUTEX_INIT;
> -    int i;
> -
> -    g_static_mutex_lock(&mutex);
> -    if (!session->priv->webdav) {
> -        for (i = 0; i < sizeof(session->priv->webdav_magic); i++)
> -            session->priv->webdav_magic[i] = g_random_int_range(0, 255);
> +    static GMutex mutex;
>  
> -        session->priv->webdav = channel_webdav_server_new(session);
> -        if (session->priv->webdav != NULL) {
> -            phodav_server_run(session->priv->webdav);
> -        }
> +    const gchar *shared_dir = spice_session_get_shared_dir(session);
> +    if (shared_dir == NULL) {
> +        g_debug("No shared dir set, not creating webdav server");
> +        return NULL;
>      }
> -    g_static_mutex_unlock(&mutex);
> +
> +    g_mutex_lock(&mutex);
> +
> +    if (priv->webdav)
> +        goto end;
> +
> +    priv->webdav = phodav_server_new(shared_dir);

Hmm looking again at that code, I'd write this as

g_mutex_lock(&mutex);

if (priv->webdav == NULL) {
    priv->webdav = phodav_server_new(shared_dir);
}

g_mutex_unlock(&mutex);

The 'end' label is not needed.

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


More information about the Spice-devel mailing list