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

[Alon Levy]

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?

> 
> >                  break;
> >              default:
> >                  red_printf("unhandled loadvm command type (%d)", ext.cmd.type);
>