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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> >  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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]