[Spice-devel] [PATCH spice-gtk 2/5] Use libphodav-2 (breaks webdav server temporarily)
Marc-André Lureau
mlureau at redhat.com
Tue Mar 3 05:32:41 PST 2015
ok, changed
----- Original Message -----
> 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
>
More information about the Spice-devel
mailing list