[Spice-devel] [PATCH spice-gtk] webdav: fix usecase with multiple concurrent connections

[Victor Toso]

Hi,

On Fri, Aug 23, 2019 at 11:57:55AM +0200, Jakub Janku wrote:
> ping

Yep, sorry

> 
> Also forgot to mention that this fixes a regression introduced by me
> in 9f5aee05.

Ok. I just went over it before coming back here.

> On Thu, Aug 8, 2019 at 11:03 AM Jakub Janků <jjanku at redhat.com> wrote:
> >
> > GOutputStream does not allow simultaneous tasks on a single
> > stream. An attempt to transfer two files therefore
> > results into one of the clients being removed in
> > mux_msg_flushed_cb() with the error
> > "Stream has outstanding operation".
> >
> > To fix this, use spice_vmc_write_async() directly.

The major difference in implementation that this patch proposes
is to avoid a GTask creation and being handled by vmcstream.c on
spice_vmc_output_stream_write_async(), correct?

I'm a bit confused about that, maybe because it was working
before? I mean, the fact that we changed the way to deal with
the buffers made our implementation on GOutputStream not
support simultaneous tasks or that should never actually work in
the first place?

I have to refresh my memory on the implementation details on this
but the concept of pipe per 'client' could work with multiple
files because of the inner protocol that mux/demux things on
client/server, but perhaps I'm misremembering.

So, again, my question would be: Did the overall behavior
changed? e.g: This client still works with old spice-webdavd for
instance. 

Thanks,

> > Signed-off-by: Jakub Janků <jjanku at redhat.com>
> > ---
> >  src/channel-webdav.c | 17 +++++++----------
> >  1 file changed, 7 insertions(+), 10 deletions(-)
> >
> > diff --git a/src/channel-webdav.c b/src/channel-webdav.c
> > index 14d4e05..09ef9f7 100644
> > --- a/src/channel-webdav.c
> > +++ b/src/channel-webdav.c
> > @@ -235,7 +235,7 @@ mux_msg_flushed_cb(GObject *source_object,
> >  {
> >      Client *client = user_data;
> >
> > -    if (!g_output_stream_write_all_finish(G_OUTPUT_STREAM(source_object), result, NULL, NULL) ||
> > +    if (spice_vmc_write_finish(SPICE_CHANNEL(source_object), result, NULL) == -1 ||
> >          client->mux.size == 0 ||
> >          !client_start_read(client)) {
> >          remove_client(client);
> > @@ -249,8 +249,6 @@ static void server_reply_cb(GObject *source_object,
> >                              gpointer user_data)
> >  {
> >      Client *client = user_data;
> > -    SpiceWebdavChannelPrivate *c = client->self->priv;
> > -    GOutputStream *mux_out;
> >      GError *err = NULL;
> >      gssize size;
> >
> > @@ -262,13 +260,12 @@ static void server_reply_cb(GObject *source_object,
> >      g_return_if_fail(size >= 0);
> >      client->mux.size = GUINT16_TO_LE(size);
> >
> > -    mux_out = g_io_stream_get_output_stream(G_IO_STREAM(c->stream));
> > -
> > -    /* this internally uses spice_vmc_write_async(), priority is ignored;
> > -     * the callback is invoked once the msg is written out to the socket */
> > -    g_output_stream_write_all_async(mux_out, (guint8 *)&client->mux, sizeof(gint64) + sizeof(guint16) + size,
> > -        G_PRIORITY_DEFAULT, client->cancellable, mux_msg_flushed_cb, client);
> > -
> > +    spice_vmc_write_async(SPICE_CHANNEL(client->self),
> > +                          &client->mux,
> > +                          sizeof(gint64) + sizeof(guint16) + size,
> > +                          client->cancellable,
> > +                          mux_msg_flushed_cb,
> > +                          client);
> >      return;
> >
> >  end:
> > --
> > 2.21.0
> >
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20190823/3370f4cb/attachment.sig>