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

[Frediano Ziglio]

> 
> 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).
> 

What do you have in mind?

> 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.
> 

I also noted that some drivers use some kind of polling too.

> 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
> 

I'm not against the patch.

Frediano