On Tue, 2018-02-13 at 15:54 +0000, Frediano Ziglio wrote:
> If messages are sent together by the agent the device is reading
> only part of the data. This cause Qemu to not poll for new data and
> stream_device_read_msg_from_dev won't be called again.
> This can cause a stall. To avoid this continue handling data
> after a full message was processed.
>
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> ---
> server/stream-device.c | 29 ++++++++++++++++++++++++-----
> 1 file changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/server/stream-device.c b/server/stream-device.c
> index 4eaa959b..3a7cb306 100644
> --- a/server/stream-device.c
> +++ b/server/stream-device.c
> @@ -71,16 +71,15 @@ static StreamMsgHandler handle_msg_format,
> handle_msg_data;
> static bool handle_msg_invalid(StreamDevice *dev,
> SpiceCharDeviceInstance *sin,
> const char *error_msg)
> SPICE_GNUC_WARN_UNUSED_RESULT;
>
> -static RedPipeItem *
> -stream_device_read_msg_from_dev(RedCharDevice *self,
> SpiceCharDeviceInstance *sin)
> +static bool
> +stream_device_partial_read(StreamDevice *dev,
> SpiceCharDeviceInstance *sin)
> {
> - StreamDevice *dev = STREAM_DEVICE(self);
> SpiceCharDeviceInterface *sif;
> int n;
> bool handled = false;
>
> if (dev->has_error || dev->flow_stopped || !dev->stream_channel)
> {
> - return NULL;
> + return false;
> }
>
> sif = spice_char_device_get_interface(sin);
> @@ -89,7 +88,7 @@ stream_device_read_msg_from_dev(RedCharDevice
> *self, SpiceCharDeviceInstance *si
> while (dev->hdr_pos < sizeof(dev->hdr)) {
> n = sif->read(sin, (uint8_t *) &dev->hdr + dev->hdr_pos,
> sizeof(dev->hdr) - dev->hdr_pos);
> if (n <= 0) {
> - return NULL;
> + return false;
> }
> dev->hdr_pos += n;
> if (dev->hdr_pos >= sizeof(dev->hdr)) {
> @@ -123,6 +122,26 @@ stream_device_read_msg_from_dev(RedCharDevice
> *self, SpiceCharDeviceInstance *si
> dev->hdr_pos = 0;
> }
>
> + if (handled || dev->has_error) {
> + // Qemu put the device on blocking state if we don't read
> all data
> + // so schedule another read.
> + // We arrive here if we processed that entire message or we
> + // got an error, try to read another message or discard the
> + // wrong data
> + return true;
> + }
> +
> + return false;
> +}
> +
> +static RedPipeItem *
> +stream_device_read_msg_from_dev(RedCharDevice *self,
> SpiceCharDeviceInstance *sin)
> +{
> + StreamDevice *dev = STREAM_DEVICE(self);
> +
> + while (stream_device_partial_read(dev, sin)) {
> + continue;
> + }
> return NULL;
> }
>
Thanks. I prefer this approach instead of calling
red_char_device_wakeup(). The name stream_device_partial_read() is OK,
but it's not great. It is really reading up until the end of the next
message *or* until there is no more data to read. I thought about
stream_device_read_to_end_of_message(), but that's not very nice
either. I can't think of a great name.
Acked-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel