On Mon, 2015-08-17 at 04:45 -0400, Frediano Ziglio wrote: > > > > On Thu, 2015-08-13 at 16:25 +0100, Frediano Ziglio wrote: > > > From: Alon Levy <alon@xxxxxxxxx> > > > > > > usage: replay <cmdfile> <port> <client command line> > > > > This usage doesn't really match the implementation below. It should be > > something more like: > > > > replay -p <port> -c <client command line> <cmdfile> > > > > While we're on the subject, though, can we use a more specific name? > > e.g. spice-replay > > > > Already taken :) > > http://cgit.freedesktop.org/~fziglio/spice-replay/ > Sorry, It seems that maybe you misunderstood my suggestion. I wasn't referring to the branch name (although that's fine as well). I was actually talking about the executable name for this replay utility. I think it would a bit nicer if it was like spice-replay -p <port> .... This executable is not actually installed, so it's not a huge problem, but I still feel like it should use a less-generic name. > > > > > > will run the commands from cmdfile ignoring timestamps, right after a > > > connection is established from the client, and will SIGINT the client > > > on end of cmdfile, and exit itself after waiting for the client. > > > > > > spicy-stats from spice-gtk is useful for testing, it prints the summary > > > of the traffic on each channel. > > > > > > You can also run with no client by doing: > > > replay <cmdfile> > > > > > > For example, the 300 MB file (compressed to 4 MB with xz -9) available > > > at [1] produces the following output: > > > > > > spicy-stats total bytes read: > > > total bytes read: > > > inputs: 214 > > > display: 1968983 > > > cursor: 390 > > > main: 256373 > > > > > > You could run it directly like so: > > > curl http://annarchy.freedesktop.org/~alon/win7_boot_shutdown.cmd.xz | > > > xzcat | server/tests/replay - 12345 `which spicy-stats` -h localhost -p > > > 12345 > > > > Again, command above doesn't seem to match the implementation. > > > > How to handle such changes? > Should we just change the commit and add the signed-off-by for authorship? > > > > > > > Known Problems: > > > * Implementation is wrong. Should do a single device->host conversion > > > (i.e. get_virt), and then marshall/demarshall that (i.e. RedDrawable). > > > * segfault on file read done resulting in the above spicy-stats not > > > being reproducable (well, up to 1% yes). > > > > > > [1] http://annarchy.freedesktop.org/~alon/win7_boot_shutdown.cmd.xz > > > > > > Now based on glib including using an asyncqueue for reading the playback > > > file, and proper freeing of the allocated commands, with --slow, > > > --compression and a progress timer, and doesn't use more then nsurfaces.3 > > > > This last paragraph seems to be addressing differences to a previous > > version of the patch. We can probably incorporate this into the above > > commit log or just drop it. > > > > > > > --- > > > server/red_replay_qxl.c | 7 +- > > > server/red_replay_qxl.h | 17 +++ > > > server/tests/Makefile.am | 9 ++ > > > server/tests/replay.c | 346 > > > +++++++++++++++++++++++++++++++++++++++++++++++ > > > 4 files changed, 377 insertions(+), 2 deletions(-) > > > create mode 100644 server/tests/replay.c > > > > > > diff --git a/server/red_replay_qxl.c b/server/red_replay_qxl.c > > > index 663c6ed..e52ee9c 100644 > > > --- a/server/red_replay_qxl.c > > > +++ b/server/red_replay_qxl.c > > > @@ -186,13 +186,15 @@ static replay_t read_binary(SpiceReplay *replay, > > > const char *prefix, size_t *siz > > > **buf, size_t base_size) > > > { > > > char template[1024]; > > > - int with_zlib; > > > + int with_zlib = -1; > > > int zlib_size; > > > uint8_t *zlib_buffer; > > > z_stream strm; > > > > > > snprintf(template, sizeof(template), "binary %%d %s %%ld:", prefix); > > > - replay_fscanf(replay, template, &with_zlib, size); > > > + if (replay_fscanf(replay, template, &with_zlib, size) == REPLAY_EOF) > > > + return REPLAY_EOF; > > > + > > > if (*buf == NULL) { > > > *buf = malloc(*size + base_size); > > > if (*buf == NULL) { > > > @@ -207,6 +209,7 @@ static replay_t read_binary(SpiceReplay *replay, const > > > char *prefix, size_t *siz > > > hexdump(*buf + base_size, *size); > > > } > > > #else > > > + spice_return_val_if_fail(with_zlib != -1, REPLAY_EOF); > > > if (with_zlib) { > > > int ret; > > > > > > diff --git a/server/red_replay_qxl.h b/server/red_replay_qxl.h > > > index 77a37e0..a89b3d4 100644 > > > --- a/server/red_replay_qxl.h > > > +++ b/server/red_replay_qxl.h > > > @@ -1,3 +1,20 @@ > > > +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */ > > > +/* > > > + Copyright (C) 2009-2015 Red Hat, Inc. > > > + > > > + This library is free software; you can redistribute it and/or > > > + modify it under the terms of the GNU Lesser General Public > > > + License as published by the Free Software Foundation; either > > > + version 2.1 of the License, or (at your option) any later version. > > > + > > > + This library is distributed in the hope that it will be useful, > > > + but WITHOUT ANY WARRANTY; without even the implied warranty of > > > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > > > + Lesser General Public License for more details. > > > + > > > + You should have received a copy of the GNU Lesser General Public > > > + License along with this library; if not, see > > > <http://www.gnu.org/licenses/>. > > > +*/ > > > #ifndef RED_REPLAY_QXL_H > > > #define RED_REPLAY_QXL_H > > > > > > diff --git a/server/tests/Makefile.am b/server/tests/Makefile.am > > > index 32c97ce..4fa03fe 100644 > > > --- a/server/tests/Makefile.am > > > +++ b/server/tests/Makefile.am > > > @@ -40,6 +40,7 @@ noinst_PROGRAMS = \ > > > test_two_servers \ > > > test_vdagent \ > > > test_display_width_stride \ > > > + replay \ > > > $(NULL) > > > > > > test_vdagent_SOURCES = \ > > > @@ -101,3 +102,11 @@ test_display_width_stride_SOURCES = \ > > > test_display_base.h \ > > > test_display_width_stride.c \ > > > $(NULL) > > > + > > > +replay_SOURCES = \ > > > + replay.c \ > > > + test_display_base.h \ > > > + basic_event_loop.c \ > > > + basic_event_loop.h \ > > > + test_util.h \ > > > + $(NULL) > > > diff --git a/server/tests/replay.c b/server/tests/replay.c > > > new file mode 100644 > > > index 0000000..f1f1098 > > > --- /dev/null > > > +++ b/server/tests/replay.c > > > @@ -0,0 +1,346 @@ > > > +/* Replay a previously recorded file (via SPICE_WORKER_RECORD_FILENAME) > > > + */ > > > + > > > +#include <string.h> > > > +#include <stdlib.h> > > > +#include <stdio.h> > > > +#include <strings.h> > > > +#include <sys/types.h> > > > +#include <signal.h> > > > +#include <unistd.h> > > > +#include <pthread.h> > > > +#include <sys/wait.h> > > > +#include <fcntl.h> > > > +#include <glib.h> > > > + > > > +#include <spice/macros.h> > > > +#include "red_replay_qxl.h" > > > +#include "test_display_base.h" > > > +#include "common/log.h" > > > + > > > +static SpiceCoreInterface *core; > > > +static SpiceServer *server; > > > +static SpiceReplay *replay; > > > +static QXLWorker *qxl_worker = NULL; > > > +static gboolean started = FALSE; > > > +static QXLInstance display_sin = { 0, }; > > > +static gint slow = 0; > > > +static pid_t client_pid; > > > +static GMainLoop *loop = NULL; > > > +static GAsyncQueue *aqueue = NULL; > > > +static long total_size; > > > + > > > +static GMutex mutex; > > > +static guint fill_source_id = 0; > > > + > > > + > > > +#define MEM_SLOT_GROUP_ID 0 > > > + > > > +/* Parts cribbed from spice-display.h/.c/qxl.c */ > > > + > > > +static QXLDevMemSlot slot = { > > > +.slot_group_id = MEM_SLOT_GROUP_ID, > > > +.slot_id = 0, > > > +.generation = 0, > > > +.virt_start = 0, > > > +.virt_end = ~0, > > > +.addr_delta = 0, > > > +.qxl_ram_size = ~0, > > > +}; > > > + > > > +static void attach_worker(QXLInstance *qin, QXLWorker *_qxl_worker) > > > +{ > > > + static int count = 0; > > > + if (++count > 1) { > > > + printf("%s ignored\n", __func__); > > > > g_warning()? > > > > > + return; > > > + } > > > + printf("%s\n", __func__); > > > > Is this necessary? perhaps use g_debug() so it doesn't print by default. > > > > > + qxl_worker = _qxl_worker; > > > + spice_qxl_add_memslot(qin, &slot); > > > + spice_server_vm_start(server); > > > +} > > > + > > > +static void set_compression_level(QXLInstance *qin, int level) > > > +{ > > > + printf("%s\n", __func__); > > > > g_debug()? > > > > > +} > > > + > > > +static void set_mm_time(QXLInstance *qin, uint32_t mm_time) > > > +{ > > > +} > > > + > > > +// same as qemu/ui/spice-display.h > > > +#define MAX_SURFACE_NUM 1024 > > > + > > > +static void get_init_info(QXLInstance *qin, QXLDevInitInfo *info) > > > +{ > > > + bzero(info, sizeof(*info)); > > > + info->num_memslots = 1; > > > + info->num_memslots_groups = 1; > > > + info->memslot_id_bits = 1; > > > + info->memslot_gen_bits = 1; > > > + info->n_surfaces = MAX_SURFACE_NUM; > > > +} > > > + > > > +static gboolean fill_queue_idle(gpointer user_data) > > > +{ > > > + gboolean keep = FALSE; > > > + > > > + while (g_async_queue_length(aqueue) < 50) { > > > + QXLCommandExt *cmd = spice_replay_next_cmd(replay, qxl_worker); > > > + if (!cmd) { > > > + g_async_queue_push(aqueue, GINT_TO_POINTER(-1)); > > > + goto end; > > > + } > > > + > > > + if (slow) > > > + g_usleep(slow); > > > + > > > + g_async_queue_push(aqueue, cmd); > > > + } > > > + > > > +end: > > > + if (!keep) { > > > + g_mutex_lock(&mutex); > > > + fill_source_id = 0; > > > + g_mutex_unlock(&mutex); > > > + } > > > + spice_qxl_wakeup(&display_sin); > > > + > > > + return keep; > > > +} > > > + > > > +static void fill_queue(void) > > > +{ > > > + g_mutex_lock(&mutex); > > > + > > > + if (!started) > > > + goto end; > > > + > > > + if (fill_source_id != 0) > > > + goto end; > > > + > > > + fill_source_id = g_idle_add(fill_queue_idle, NULL); > > > + > > > +end: > > > + g_mutex_unlock(&mutex); > > > +} > > > + > > > + > > > +// called from spice_server thread (i.e. red_worker thread) > > > +static int get_command(QXLInstance *qin, QXLCommandExt *ext) > > > +{ > > > + QXLCommandExt *cmd; > > > + > > > + if (g_async_queue_length(aqueue) == 0) { > > > + /* could use a gcondition ? */ > > > + fill_queue(); > > > + return FALSE; > > > + } > > > + > > > + cmd = g_async_queue_try_pop(aqueue); > > > + if (GPOINTER_TO_INT(cmd) == -1) { > > > + g_main_loop_quit(loop); > > > + return FALSE; > > > + } > > > + > > > + *ext = *cmd; > > > + > > > + return TRUE; > > > +} > > > + > > > +static int req_cmd_notification(QXLInstance *qin) > > > +{ > > > + if (!started) > > > + return TRUE; > > > + > > > + spice_printerr("id: %d, queue length: %d", > > > + fill_source_id, g_async_queue_length(aqueue)); > > > > I'd personally prefer to use glib functions in new code: -> > > g_printerr()? > > > > > + > > > + return TRUE; > > > +} > > > + > > > +static void end_replay(void) > > > +{ > > > + int child_status; > > > + > > > + /* FIXME: wait threads and end cleanly */ > > > + spice_replay_free(replay); > > > + > > > + if (client_pid) { > > > + g_debug("kill %d", client_pid); > > > + kill(client_pid, SIGINT); > > > + waitpid(client_pid, &child_status, 0); > > > + } > > > + exit(0); > > > +} > > > + > > > +static void release_resource(QXLInstance *qin, struct QXLReleaseInfoExt > > > release_info) > > > +{ > > > + spice_replay_free_cmd(replay, (QXLCommandExt *)release_info.info->id); > > > +} > > > + > > > +static int get_cursor_command(QXLInstance *qin, struct QXLCommandExt *ext) > > > +{ > > > + return FALSE; > > > +} > > > + > > > +static int req_cursor_notification(QXLInstance *qin) > > > +{ > > > + return TRUE; > > > +} > > > + > > > +static void notify_update(QXLInstance *qin, uint32_t update_id) > > > +{ > > > +} > > > + > > > +static int flush_resources(QXLInstance *qin) > > > +{ > > > + return TRUE; > > > +} > > > + > > > +static QXLInterface display_sif = { > > > + .base = { > > > + .type = SPICE_INTERFACE_QXL, > > > + .description = "replay", > > > + .major_version = SPICE_INTERFACE_QXL_MAJOR, > > > + .minor_version = SPICE_INTERFACE_QXL_MINOR > > > + }, > > > + .attache_worker = attach_worker, > > > + .set_compression_level = set_compression_level, > > > + .set_mm_time = set_mm_time, > > > + .get_init_info = get_init_info, > > > + .get_command = get_command, > > > + .req_cmd_notification = req_cmd_notification, > > > + .release_resource = release_resource, > > > + .get_cursor_command = get_cursor_command, > > > + .req_cursor_notification = req_cursor_notification, > > > + .notify_update = notify_update, > > > + .flush_resources = flush_resources, > > > +}; > > > + > > > +static void replay_channel_event(int event, SpiceChannelEventInfo *info) > > > +{ > > > + spice_printerr(""); > > > > g_printerr() > > > > > + > > > + if (info->type == SPICE_CHANNEL_DISPLAY && > > > + event == SPICE_CHANNEL_EVENT_INITIALIZED) { > > > + started = TRUE; > > > + } > > > +} > > > + > > > +static gboolean start_client(gchar *cmd, GError **error) > > > +{ > > > + gboolean retval; > > > + gint argc; > > > + gchar **argv = NULL; > > > + > > > + > > > + if (!g_shell_parse_argv(cmd, &argc, &argv, error)) > > > + return FALSE; > > > + > > > + retval = g_spawn_async(NULL, argv, NULL, G_SPAWN_SEARCH_PATH, > > > + NULL, NULL, &client_pid, error); > > > + g_strfreev(argv); > > > + > > > + return retval; > > > +} > > > + > > > +static gboolean progress_timer(gpointer user_data) > > > +{ > > > + FILE *fd = user_data; > > > + /* it seems somehow thread safe, move to worker thread? */ > > > + double pos = (double)ftell(fd); > > > + > > > + g_debug("%.2f%%", pos/total_size * 100); > > > + return TRUE; > > > +} > > > + > > > +int main(int argc, char **argv) > > > +{ > > > + GError *error = NULL; > > > + GOptionContext *context = NULL; > > > + gchar *client = NULL, **file = NULL; > > > + gint port = 5000, compression = SPICE_IMAGE_COMPRESSION_AUTO_GLZ; > > > + gboolean wait = FALSE; > > > + FILE *fd; > > > + > > > + GOptionEntry entries[] = { > > > + { "client", 'c', 0, G_OPTION_ARG_STRING, &client, "Client", "CMD" > > > }, > > > + { "compression", 'C', 0, G_OPTION_ARG_INT, &compression, > > > "Compression (default 2)", "INT" }, > > > + { "port", 'p', 0, G_OPTION_ARG_INT, &port, "Server port (default > > > 5000)", "PORT" }, > > > + { "wait", 'w', 0, G_OPTION_ARG_NONE, &wait, "Wait for client", > > > NULL }, > > > + { "slow", 's', 0, G_OPTION_ARG_INT, &slow, "Slow down replay", > > > NULL }, > > > + { G_OPTION_REMAINING, 0, 0, G_OPTION_ARG_FILENAME_ARRAY, &file, > > > "replay file", "FILE" }, > > > > This last option specifies an array of files, but the code below only > > opens the first one. > > > > > + { NULL } > > > + }; > > > + > > > + context = g_option_context_new("- replay spice server recording"); > > > > This is a common use of this "parameter_string" argument (and it's even > > mentioned as an alternative in the API documentation), but I really > > dislike it and find it confusing. I would prefer to use > > g_option_context_set_summary() for the description. > > > > > + g_option_context_add_main_entries(context, entries, NULL); > > > + if (!g_option_context_parse(context, &argc, &argv, &error)) { > > > + g_printerr("Option parsing failed: %s\n", error->message); > > > + exit(1); > > > + } > > > + if (!file) { > > > + g_printerr(g_option_context_get_help(context, TRUE, NULL)); > > > + exit(1); > > > + } > > > + > > > + if (strncmp(file[0], "-", 1) == 0) { > > > + fd = stdin; > > > + } else { > > > + fd = fopen(file[0], "r"); > > > + } > > > + if (fd == NULL) { > > > + g_printerr("error opening %s\n", argv[1]); > > > + return 1; > > > + } > > > + if (fcntl(fileno(fd), FD_CLOEXEC) < 0) { > > > + perror("fcntl failed"); > > > + exit(1); > > > + } > > > + fseek(fd, 0L, SEEK_END); > > > + total_size = ftell(fd); > > > + fseek(fd, 0L, SEEK_SET); > > > + if (total_size > 0) > > > + g_timeout_add_seconds(1, progress_timer, fd); > > > + replay = spice_replay_new(fd, MAX_SURFACE_NUM); > > > + > > > + aqueue = g_async_queue_new(); > > > + core = basic_event_loop_init(); > > > + core->channel_event = replay_channel_event; > > > + > > > + SpiceServer* server = spice_server_new(); > > > + spice_server_set_image_compression(server, compression); > > > + spice_server_set_port(server, port); > > > + spice_server_set_noauth(server); > > > + > > > + g_print("listening on port %d (unsecure)\n", port); > > > > "unsecure" should be "insecure" > > > > > + spice_server_init(server, core); > > > + > > > + display_sin.base.sif = &display_sif.base; > > > + spice_server_add_interface(server, &display_sin.base); > > > + > > > + if (client) { > > > + start_client(client, &error); > > > + wait = TRUE; > > > + } > > > + > > > + if (!wait) { > > > + started = TRUE; > > > + fill_queue(); > > > + } > > > + > > > + loop = g_main_loop_new(NULL, FALSE); > > > + g_main_loop_run(loop); > > > + > > > + end_replay(); > > > + g_async_queue_unref(aqueue); > > > + > > > + /* FIXME: there should be a way to join server threads before: > > > + * g_main_loop_unref(loop); > > > + */ > > > + > > > + return 0; > > > +} > > > > > > > > Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel