[Spice-devel] [PATCH spice v0.12.6] Flush display commands before validating the surface.

[Jeremy White]

On 12/14/2015 07:42 AM, Frediano Ziglio wrote:
>>
>> On Tue, Dec 08, 2015 at 09:11:08AM -0600, Jeremy White wrote:
>>> This fixes a display glitch in xspice which is caused when
>>> a surface create is queued, but then a direct call to update
>>> the area is issued.  Unless we flush the queue, the surface
>>> does not exist, and we fail.
>>
>> This also matches what handle_dev_update_async() is doing (flush, call
>> VALIDATE_SURFACE_RET()).
>>
>> Acked-by: Christophe Fergeau <cfergeau at redhat.com>
>>
>> I'll push this once a 0.12 branch exists.
>>
>> Christophe
>>
> 
> I would ack too, the patch is not harmful
> 
> However: Is this patch fixing a bug on spice-server or is just a workaround
> of a bug of xspice?
> 
> I personally think the second. If you issue an asynchronous command you should
> not try to issue another synchronous command depending on the first one but
> you first should wait for first command finish and then issue the second.
> Unless these commands are all defined to go into a single queue to they are
> all serialized together.


That's an interesting point.

Researching further, I think the fundamental issue is that Xspice is a
bit of a hybrid.  That is, this works for the regular qxl driver because
both the surface creation and the update area go through the command
ring.  But in Xspice, some things go through the ring, and others are
done as direct calls.

To me, the logical next step would be to modify the Spice API to allow
Xspice to do everything directly.  That would be faster and make for a
much cleaner interface (not to mention a more sensible use of memory).

And there isn't any Xspice mechanism to wait for command completion,
that I could find.  That is, I didn't see a way to make the surface
creation into a sync operation, and I'm hesitant to take the performance
hit in making the update_area go through the ring.

So I'd continue to advocate for this patch.  I'm hoping that after all
this refactoring, it will be easier for me to propose the changes that
Xspice would need to be more sensible.

Cheers,

Jeremy