[Spice-devel] [PATCH spice-server v4 0/4] Better handling reset for streaming device

[Christophe de Dinechin]

> On 12 Feb 2018, at 09:04, Frediano Ziglio <fziglio at redhat.com> wrote:
> 
>> 
>> On 01/30/2018 04:33 PM, Christophe de Dinechin wrote:
>>> 
>>> Christophe de Dinechin writes:
>>> 
>>>>> On 30 Jan 2018, at 12:56, Frediano Ziglio <fziglio at redhat.com> wrote:
>>>>> 
>>>>>> 
>>>>>> Hi Frediano,
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>>> On 30 Jan 2018, at 11:50, Frediano Ziglio <fziglio at redhat.com> wrote:
>>>>>>> 
>>>>>>> ping the series
>>>>>> 
>>>>>> I’m currently looking at it. Is it supposed to fix the read errors I had
>>>>>> when
>>>>>> restarting the streaming agent?
>>>>>> 
>>>>> 
>>>>> Yes, make the reset more stable.
>>>>> When you close the device the state will be more consistent allowing
>>>>> basically to kill the process using the device in any state and opening
>>>>> again. Obviously if you continue to send wrong commands the device will
>>>>> keep rejecting them.
>>>>> 
>>>>> I tried to reproduce the issues reported on IRC and these helped me,
>>>>> now I avoid entirely to reboot the guest.
>>>> 
>>>> OK, right now I get a QEMU crash whenever I do any kind of activity
>>>> (the keyboard seems to be what triggers it).
>>>> 
>>>> I’m trying to reproduce on master to see if your patch is the cause.
>>>> That host has gone through some unusual nastiness, and may be
>>>> in a geborked state.
>>> 
>>> Reverting the server to master, I am back to the behavior I had before,
>>> where the same series of events leads to
>>> 
>>> DISPLAY=:1 spice-streaming-agent -c noblock=yes
>>> spice-streaming-agent[2240]: UNKNOWN msg of type 5
>>> spice-streaming-agent[2240]: BAD VERSION 0 (expected is 1)
>>> spice-streaming-agent[2240]: BAD VERSION 108 (expected is 1)
>>> spice-streaming-agent[2240]: BAD VERSION 97 (expected is 1)
>>> spice-streaming-agent[2240]: read command from device FAILED -- read 1
>>> expected 8
>>> spice-streaming-agent[2240]: FAILED to read command
>> 
>> 
>> Hi Christophe,
>> 
>> There are some problems here:
>> 1. (I guess) your spice-streaming-agent sends CURSOR messages
>>    which currently the spice-server does not know to handle.
>>    (Frediano sent patches for that but still no review).
>> 
>>    For now try to comment out the code in spice-streaming-agent
>>    that sends CURSOR commands.
>> 
> 
> Would not make sense to review the patches instead of having
> to write another workaround also taking into account that 
> the review process is taking more than 6 months?
> 
>> 2. spice-server gets an unknown command. It sends a message to the
>>    spice-streaming-agent to let it know the server read an invalid
>>    message. spice-streaming-agent is missing a code to handle such
>>    a message.
>> 
>>    This should be fixed.
>> 
>> 3. When such messages are in play, they are not fully read (code
>>    breaks after reading the header). This makes the spice-server
>>    and spice-streaming-agent go out of sync).
>> 
>>    This may be fixed. I'm not sure we can recover
>>    all errors, and perhaps the right thing to do is sync
>>    with close/open of the virtio-serial-port.
> 
> This is what this patch series is doing.

And indeed, that patch series improves things if you have a recent enough QEMU. But it triggers a but in 2.9.1, which crashes QEMU and kills the VM. I saw that on my test machine, Frediano now reproduced.

> 
>>    However reading the whole message (if it's not too big) should
>>    at least keep the server/agent synchronized, even if an unknown
>>    message encountered.
>> 
> 
> No, is not enough. If the message is expected to change the server
> state discarding the message will make the dialog out of sync anyway.
> Would make sense if the server could send back an error for a specific
> message (currently if you send a batch of messages you don't know
> the relationship between error and message). Also would mean that
> the client MUST handle the error from a message and as the messages
> replies are async to keep a queue of messages.
> To me it seems that at the end would be just more complicated than
> expected instead of graceful.
> 
>> Regards,
>>     Uri.
>> 
>> 
> 
> Frediano
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel