[Spice-devel] [PATCH 6/6] server/red_worker: send surface images on demand (UPDATE_MEM)

Alon Levy alevy at redhat.com
Thu Jun 30 04:43:47 PDT 2011


On Thu, Jun 30, 2011 at 01:06:43PM +0300, Yonit Halperin wrote:
> On 06/30/2011 12:58 PM, Alon Levy wrote:
> >On Thu, Jun 30, 2011 at 12:36:27PM +0300, Yonit Halperin wrote:
> >>On 06/20/2011 02:18 PM, Alon Levy wrote:
> >>>From: Yonit Halperin<yhalperi at redhat.com>
> >>>
> >>>When surfaces are being reloaded to the worker (e.g., after UPDATE_MEM), we
> >>>will send them to the client only if and when it needs them.
> >>>---
> >>>  server/red_worker.c |   33 +++++++++++++++++++--------------
> >>>  1 files changed, 19 insertions(+), 14 deletions(-)
> >>>
> >>>diff --git a/server/red_worker.c b/server/red_worker.c
> >>>index f5a2091..5a5073b 100644
> >>>--- a/server/red_worker.c
> >>>+++ b/server/red_worker.c
> >>>@@ -3404,9 +3404,9 @@ static inline void red_process_drawable(RedWorker *worker, RedDrawable *drawable
> >>>
> >>>  static inline void red_create_surface(RedWorker *worker, uint32_t surface_id,uint32_t width,
> >>>                                        uint32_t height, int32_t stride, uint32_t format,
> >>>-                                      void *line_0, int data_is_valid);
> >>>+                                      void *line_0, int data_is_valid, int send_client);
> >>>
> >>>-static inline void red_process_surface(RedWorker *worker, RedSurfaceCmd *surface, uint32_t group_id, int data_is_valid)
> >>>+static inline void red_process_surface(RedWorker *worker, RedSurfaceCmd *surface, uint32_t group_id)
> >>>  {
> >>>      int surface_id;
> >>>      RedSurface *red_surface;
> >>>@@ -3428,7 +3428,9 @@ static inline void red_process_surface(RedWorker *worker, RedSurfaceCmd *surface
> >>>          }
> >>>          red_create_surface(worker, surface_id, surface->u.surface_create.width,
> >>>                             height, stride, surface->u.surface_create.format, data,
> >>>-                           data_is_valid);
> >>>+                           surface->flags&   QXL_SURF_FLAG_KEEP_DATA,
> >>>+                           // reloaded surfaces will be sent on demand
> >>>+                           !(surface->flags&   QXL_SURF_FLAG_KEEP_DATA));
> >>>          set_surface_release_info(worker, surface_id, 1, surface->release_info, group_id);
> >>>          break;
> >>>      }
> >>>@@ -4290,8 +4292,7 @@ static int red_process_commands(RedWorker *worker, uint32_t max_pipe_size, int *
> >>>
> >>>              red_get_surface_cmd(&worker->mem_slots, ext_cmd.group_id,
> >>>                                  surface, ext_cmd.cmd.data);
> >>>-            red_process_surface(worker, surface, ext_cmd.group_id,
> >>>-                                surface->flags&   QXL_SURF_FLAG_KEEP_DATA);
> >>>+            red_process_surface(worker, surface, ext_cmd.group_id);
> >>>              break;
> >>>          }
> >>>          default:
> >>>@@ -8452,7 +8453,7 @@ static inline void red_create_surface_item(RedWorker *worker, int surface_id)
> >>>
> >>>  static inline void red_create_surface(RedWorker *worker, uint32_t surface_id, uint32_t width,
> >>>                                        uint32_t height, int32_t stride, uint32_t format,
> >>>-                                      void *line_0, int data_is_valid)
> >>>+                                      void *line_0, int data_is_valid, int send_client)
> >>>  {
> >>>      uint32_t i;
> >>>      RedSurface *surface =&worker->surfaces[surface_id];
> >>>@@ -8485,9 +8486,11 @@ static inline void red_create_surface(RedWorker *worker, uint32_t surface_id, ui
> >>>              PANIC("drawing canvas creating failed - can`t create same type canvas");
> >>>          }
> >>>
> >>>-        red_create_surface_item(worker, surface_id);
> >>>-        if (data_is_valid) {
> >>>-            red_add_surface_image(worker, surface_id);
> >>>+        if (send_client) {
> >>>+            red_create_surface_item(worker, surface_id);
> >>>+            if (data_is_valid) {
> >>>+                red_add_surface_image(worker, surface_id);
> >>>+            }
> >>>          }
> >>>          return;
> >>>      }
> >>>@@ -8498,9 +8501,11 @@ static inline void red_create_surface(RedWorker *worker, uint32_t surface_id, ui
> >>>                                                              surface->context.format, line_0);
> >>>          if (surface->context.canvas) { //no need canvas check
> >>>              worker->renderer = worker->renderers[i];
> >>>-            red_create_surface_item(worker, surface_id);
> >>>-            if (data_is_valid) {
> >>>-                red_add_surface_image(worker, surface_id);
> >>>+            if (send_client) {
> >>>+                red_create_surface_item(worker, surface_id);
> >>>+                if (data_is_valid) {
> >>>+                    red_add_surface_image(worker, surface_id);
> >>>+                }
> >>>              }
> >>>              return;
> >>>          }
> >>>@@ -9634,7 +9639,7 @@ static inline void handle_dev_create_primary_surface(RedWorker *worker)
> >>>      }
> >>>
> >>>      red_create_surface(worker, 0, surface.width, surface.height, surface.stride, surface.format,
> >>>-                       line_0, surface.flags&   QXL_SURF_FLAG_KEEP_DATA);
> >>>+                       line_0, surface.flags&   QXL_SURF_FLAG_KEEP_DATA, TRUE);
> >>>
> >>>      if (worker->display_channel) {
> >>>          red_pipe_add_verb(&worker->display_channel->common.base, SPICE_MSG_DISPLAY_MARK);
> >>>@@ -9945,7 +9950,7 @@ static void handle_dev_input(EventListener *listener, uint32_t events)
> >>>                  surface_cmd = spice_new0(RedSurfaceCmd, 1);
> >>>                  red_get_surface_cmd(&worker->mem_slots, ext.group_id,
> >>>                                      surface_cmd, ext.cmd.data);
> >>>-                red_process_surface(worker, surface_cmd, ext.group_id, 1);
> >>>+                red_process_surface(worker, surface_cmd, ext.group_id);
> >>
> >>Hi,
> >>I need to fix this - on LOADVM, the replayed commands are without
> >>QXL_SURF_FLAG_KEEP_DATA, but ideally they should have been (but we
> >>track the original create surface commands). red_process_surface
> >>should have "loadvm" parameter, and then when creating a surface we
> >>will zero it only if !loadvm&&  !QXL_SURF_FLAG_KEEP_DATA. And we
> >>will also send loaded surfaces on-demand.
> >
> >The whole tracking thing being in qemu/hw/qxl and not in spice-server
> >always looked strange to me. Same with the logging code. Maybe this
> >should be moved?
> but the migration destination server needs the surfaces. The source
> server can't help with this (unless we use the client to  transfer
> the surfaces, but then we also could do seemless migration :))

My suggestion was to move the logic to the server, it would still need to
be put in vmstate by qemu/hw/qxl.

> >
> >>
> >>>                  break;
> >>>              default:
> >>>                  red_printf("unhandled loadvm command type (%d)", ext.cmd.type);
> >>
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel


More information about the Spice-devel mailing list