Re: [Qemu-devel] [Spice-devel] [PATCH] Add new client_present and client capabilities fields to QXLRom
Hi, Hmm, when the server is able to translate a8 ops into non-a8 ops using server-side rendering, then there is no need to notify the guest about the client capabilities. To be clear, this ability doesn't exist at the moment, and it would be a significant chunk of work to add it. But it's much simpler to just say that the guest should stop referring to a8 surfaces if the client can't handle them. Not sure about that, this move might just shift the complexity from spice-server to the guest qxl driver. The ability to handle this is already pretty much present in at least the X driver (and I'm pretty sure the Windows driver has it as well) because any time something can't be expressed in the SPICE protocol, it has to fall back to software rendering. Ie., it has to read all the involved surfaces back from video memory, do software rendering, then upload the result as an image. Dealing with a disappearing ability to handle a8 surfaces would simply be a matter of reading back the a8 surfaces to guest RAM and then not attempt to acccelerate any operations involving them any more. It looks much more involved to do it in spice-server because it would probably involve adding a new concept of emulated surface that needs to be handled specially in a bunch of cases. Ok, so the tradeoff seems clear. I trust you on that one, you know the guest drivers alot better than I do. Lets go forward with the client capability notification for the guest, /me awaits a new revision of the patches. cheers, Gerd
Re: [Qemu-devel] [Spice-devel] [PATCH] Add new client_present and client capabilities fields to QXLRom
Gerd Hoffmann kra...@redhat.com writes: The scheme I had in mind was this: - When a new non-a8-capable client appears, don't send it any of the a8 surfaces - If the client doesn't understand a8 surfaces, - keep all a8 surfaces rendered on the server side - if the guest sends a command using an a8 surface as a destination, simply render the command on the server side - if the client sends a command using an a8 surface as a source, rewrite the image object to be a real image referring to the server side bits (which are also sent or possibly cached) rather than a surface Hmm, when the server is able to translate a8 ops into non-a8 ops using server-side rendering, then there is no need to notify the guest about the client capabilities. To be clear, this ability doesn't exist at the moment, and it would be a significant chunk of work to add it. But it's much simpler to just say that the guest should stop referring to a8 surfaces if the client can't handle them. Not sure about that, this move might just shift the complexity from spice-server to the guest qxl driver. The ability to handle this is already pretty much present in at least the X driver (and I'm pretty sure the Windows driver has it as well) because any time something can't be expressed in the SPICE protocol, it has to fall back to software rendering. Ie., it has to read all the involved surfaces back from video memory, do software rendering, then upload the result as an image. Dealing with a disappearing ability to handle a8 surfaces would simply be a matter of reading back the a8 surfaces to guest RAM and then not attempt to acccelerate any operations involving them any more. It looks much more involved to do it in spice-server because it would probably involve adding a new concept of emulated surface that needs to be handled specially in a bunch of cases. Søren
Re: [Qemu-devel] [Spice-devel] [PATCH] Add new client_present and client capabilities fields to QXLRom
On Wed, Aug 29, 2012 at 02:58:14AM +0200, Søren Sandmann wrote:
Gerd Hoffmann kra...@redhat.com writes:
On 08/27/12 19:20, Søren Sandmann Pedersen wrote:
From: Søren Sandmann Pedersen s...@redhat.com
The client_present field is a byte that is set of non-zero when a
client is connected and to zero when no client is connected.
The client_capabilities[58] array contains 464 bits that indicate the
capabilities of the client.
What is supposed to happen in case multiple clients are connected?
Is this case supported at all?
If it is, I'd say that the guest should not be aware of it and the bits
advertise should be interpreted as these are the capabilities that
spice-server will marshall on to the clients that are
connected. Presumably spice-server would then set the bit vector to the
intersection of all the clients.
How do you handle the race conditions, especially on capability
downgrade? There might be not-yet processed commands in the command
queue which the client is unable to handle, or existing surfaces using
formats the client doesn't understand ...
Good question.
I don't know of a good way to deal with the situation where the new
client is unable to handle existing surfaces. I suppose in principle
spice-server could emulate their existence, sending them as images, but
I'm not familiar enough with spice-server to say whether that is
feasible.
Sending a surface with a format the client doesn't recognize as an image
- how does that help? you'd want to render anything dependent on that
surface. And then the guest will be notified of the reduced
capabilities and needs to never use those surfaces again (better yet,
destroy them since they are just taking space).
The rendering is already accomplished in on_new_display_channel_client
with the red_flush_current(worker, 0) call, which recursively goes to
all the dependent surfaces of each drawable and renders it too.
For commands, would it work for spice-server to just process everything
in the command ring after changing the capability bits (ie., in possibly
brief moment before a new client connects)? It seems that would be a
good thing to do even without capability bits.
This should work. Something like this I guess: (probably only if
capabilities have changed - otherwise no need. And without
MAX_PIPE_SIZE, although I'm not sure what you would put instead.):
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -9493,6 +9493,10 @@ static void
on_new_display_channel_client(DisplayChannelClient *dcc)
}
red_channel_client_ack_zero_messages_window(dcc-common.base);
if (worker-surfaces[0].context.canvas) {
+int ring_is_empty;
+
+while (red_process_commands(worker, MAX_PIPE_SIZE, ring_is_empty)) {
+}
red_current_flush(worker, 0);
push_new_primary_surface(dcc);
red_push_surface_image(dcc, 0);
Søren
___
Spice-devel mailing list
spice-de...@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Qemu-devel] [Spice-devel] [PATCH] Add new client_present and client capabilities fields to QXLRom
Alon Levy al...@redhat.com writes: Good question. I don't know of a good way to deal with the situation where the new client is unable to handle existing surfaces. I suppose in principle spice-server could emulate their existence, sending them as images, but I'm not familiar enough with spice-server to say whether that is feasible. Sending a surface with a format the client doesn't recognize as an image - how does that help? you'd want to render anything dependent on that surface. And then the guest will be notified of the reduced capabilities and needs to never use those surfaces again (better yet, destroy them since they are just taking space). The rendering is already accomplished in on_new_display_channel_client with the red_flush_current(worker, 0) call, which recursively goes to all the dependent surfaces of each drawable and renders it too. The scheme I had in mind was this: - When a new non-a8-capable client appears, don't send it any of the a8 surfaces - If the client doesn't understand a8 surfaces, - keep all a8 surfaces rendered on the server side - if the guest sends a command using an a8 surface as a destination, simply render the command on the server side - if the client sends a command using an a8 surface as a source, rewrite the image object to be a real image referring to the server side bits (which are also sent or possibly cached) rather than a surface But it's much simpler to just say that the guest should stop referring to a8 surfaces if the client can't handle them. Ie., the same scheme as for commands: When a client disconnects, spice-server changes the capability bits, then processes everything in the ring. After this, the guest is expected to stop referring to a8 surfaces (and may indeed want to destroy them as you say). Soren
Re: [Qemu-devel] [Spice-devel] [PATCH] Add new client_present and client capabilities fields to QXLRom
Hi, The scheme I had in mind was this: - When a new non-a8-capable client appears, don't send it any of the a8 surfaces - If the client doesn't understand a8 surfaces, - keep all a8 surfaces rendered on the server side - if the guest sends a command using an a8 surface as a destination, simply render the command on the server side - if the client sends a command using an a8 surface as a source, rewrite the image object to be a real image referring to the server side bits (which are also sent or possibly cached) rather than a surface Hmm, when the server is able to translate a8 ops into non-a8 ops using server-side rendering, then there is no need to notify the guest about the client capabilities. But it's much simpler to just say that the guest should stop referring to a8 surfaces if the client can't handle them. Not sure about that, this move might just shift the complexity from spice-server to the guest qxl driver. cheers, Gerd
