[Spice-devel] [spice-server 2/3] Fix some small red_channel_client_msg_sent() regression

[Christophe Fergeau]

Hey,

On Tue, Apr 11, 2017 at 12:52:25PM -0400, Frediano Ziglio wrote:
> Something in this code looks weird...
> Beside resetting the blocked flags this code is disabling the WRITE
> event from the network.
> Note that this function get called when the entire message is written
> to the network layer.
> This is not causing a delay as if there are other pending data other
> data are appended to the network till we get a EAGAIN (or error) and
> in this case the event (WRITE) is added again. So, if the network
> output buffer get some space we'll write data.
> However... is this code needed here? We need to be called on WRITE
> if there are pending data still to send but this function only consider
> the blocking flag so it will reset the event event if there are other
> pending data in the queue (that is other messages to be sent). I
> think this is the reason why the dead code worked too... there's no
> need to reset the WRITE event or clear the blocked flag here!
> Note the the WRITE event is reset also in red_channel_client_push
> that is where data are sent and this function is called when a WRITE
> event is received (so would be reset by this function in any case).

I would say the reasoning is that we start watching for WRITE events when the
buffer is full, and we stop watching for WRITE events when it's no
longer full. Right after having sent a message, the code assumes the
buffer is no longer going to be full. This is also the initial state
which is set in RedChannelClient constructor.

It seems 5c460de1 has different expectations (the "we need to be called
on WRITE if there are pending data" part of your message), so it
probably needs to be dropped now.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170428/e6264972/attachment.sig>