[Spice-devel] [spice-server v2 2/3] channel: Remove red_channel_client_disconnect_if_pending_send()

[Frediano Ziglio]

> 
> > On 15 Sep 2017, at 10:43, Frediano Ziglio <fziglio at redhat.com> wrote:
> > 
> >> 
> >>> On 14 Sep 2017, at 17:55, Frediano Ziglio <fziglio at redhat.com> wrote:
> >>> 
> >>>> 
> >>>> Wasn’t there a discussion about also renaming the function? I believe
> >>>> “pending_send” is a symptom, not the reason for disconnecting.
> >>>> 
> >>>> Christophe
> >>>> 
> >>> 
> >>> The discussion was on red_channel_wait_all_sent, we decide a comment
> >>> was enough. Said that we can have always updates.
> >> 
> >> OK
> >> 
> >>> 
> >>> Are you talking about red_channel_client_disconnect_if_pending_send ?
> >> 
> >> Yes. What is the rationale behind disconnecting if “pending_send”?
> >> 
> >> 
> >> Christophe
> > 
> > Do you mean the function name or the reason? The function name didn't
> > change
> > so there was no much discussion on it.
> > The reason why we disconnect is that in these code path we want to flush
> > data to clients to make sure some resources are freed.
> 
> Hmm, now I’m even more confused. Do we disconnect to free resources?

Some resources are attached to the queue.
Don't remember entirely the paths but I think this was the idea.

> Or is disconnecting the only way to flush data to clients (I doubt it)?

Not that you cannot go to clients and beat them until they read all queued
data :-)
You could argue that in some cases the queue can be shrunk in these cases
but this is another issue.

> Or do we flush because we disconnect, and want to make sure the last drops
> are out?
> 

Don't think so. In some cases code wants the queue to be empty.

> > Not that I 100% agree with disconnecting clients on these cases but this
> > is not a regression of this patch.
> 
> No, but since this patch rewrites the function and its call sites anyway, we
> may
> as well give it a name that is slightly more explanatory.
> 

Suggestions welcome.

Frediano