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

[Yonit Halperin]

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 :))
>
>>
>>>                   break;
>>>               default:
>>>                   red_printf("unhandled loadvm command type (%d)", ext.cmd.type);
>>