[Spice-devel] [PATCH v5 0/6] Implement record/replay

[Jonathon Jongsma]

ACK series with a couple minor comments on commit messages (patch #5 and
#6)

(sorry, one of my comments was on a v4 patch, I wasn't paying close
enough attention)



On Fri, 2015-08-21 at 09:46 +0100, Frediano Ziglio wrote:
> With this series of patches is possible to record what happens to
> spice-server and replay it.
> The main purpose is debugging.
> These are part of a long series of patches.
> 
> Started playing with it quite a lot and are very useful for profiling
> and debugging.
> Actually valgrind shows some leaks due to a missing proper
> spice_server_destroy function (actually does not much). This is
> not however causing problems using with Qemu/Xspice.
> 
> Many formatting strings use signed (%d) numbers however this does not
> make sense for some data (like binary lengths!), IMHO should be converted
> to unsigned (%u).
> 
> Changes from v4:
> - fix positive stripe surfaces;
> - removed server/make_recorder.sh.
> 
> Changes from v3:
> - added a missing patch causing problems, removed my former fix,
>   tested again.
> 
> Changes from v2:
> - more strong tests (using with client) revealed that loop code
>   is not working as expected as there were 2 loops, one for commands
>   and another for spice event however only commands one was running
>   leading to client not connecting or working. Added a new patch but I
>   think to squash into second one and remove comment for glib loop;
> - renamed replay executable to spice-server-replay (still not
>   installed);
> - renamed dispatch handler;
> - add a patch to support filtering the output. This avoid to add code
>   to compress the output as compression can be done with a filter
>   (setting SPICE_WORKER_RECORD_FILTER environmen to "gzip" just works
>   for me compression ratio is about 9% so quite good). This make it
>   automatically use more cpu cores and support multiple compressors.
>   I would remove code to support zlib compression if this patch is
>   accepted. Also we should update link to xz file then.
>   Probably would be better to define a 
>      gboolean red_record_open_file(FILE **file_out)
>   function instead which manage even the SPICE_WORKER_RECORD_FILENAME
>   environment and reduce red_worker.c changes;
> - remove useless check for record_fd;
> - fix recording images if stripe is negative.
> 
> Changes from v1:
> - merged some later pacthes by Marc-André Lureau;
> - removed first patch that added an argument to normal callbacks;
> - removed record_clock_id, not use anymore;
> - fixed start of record file;
> - register callback only if we need to register something;
> - moved exported symbols to new version, removed old ones;
> - some typos;
> - fixed some comments for program arguments;
> - use glib functions for debug/warnings.
> 
> 
> Alon Levy (4):
>   server/dispatcher: add extra_dispatcher, hack for red_record
>   server/red_{record, replay}.[ch]: introduce
>   server/red_worker: record to SPICE_WORKER_RECORD_FILENAME
>   server/tests/replay: introduce
> 
> Frediano Ziglio (1):
>   Allow to specify a filter for record output
> 
> Marc-André Lureau (1):
>   tests: use glib main loop
> 
>  server/Makefile.am              |    2 +
>  server/dispatcher.c             |   10 +
>  server/dispatcher.h             |   12 +
>  server/red_record_qxl.c         |  877 +++++++++++++++++++++++++++
>  server/red_record_qxl.h         |   36 ++
>  server/red_replay_qxl.c         | 1243 +++++++++++++++++++++++++++++++++++++++
>  server/red_replay_qxl.h         |   34 ++
>  server/red_worker.c             |   40 +-
>  server/spice-server.syms        |    4 +
>  server/tests/Makefile.am        |   11 +
>  server/tests/basic_event_loop.c |  285 +++------
>  server/tests/replay.c           |  346 +++++++++++
>  12 files changed, 2707 insertions(+), 193 deletions(-)
>  create mode 100644 server/red_record_qxl.c
>  create mode 100644 server/red_record_qxl.h
>  create mode 100644 server/red_replay_qxl.c
>  create mode 100644 server/red_replay_qxl.h
>  create mode 100644 server/tests/replay.c
>