[Spice-devel] [PATCH v5 6/6] Allow to specify a filter for record output

[Frediano Ziglio]

> From: "Jonathon Jongsma" <jjongsma at redhat.com>
> 
> So I thought you were going to remove this functionality after talking
> with marc-andre? Or did the alternate method end up not working?
> 


Pushed the series beside this patch. No sense to change original patches
more and more.

About this patch after some more testing looks like that using shell
to make some trick does not work in all situations.
I know that replacing something like

   executable

with

   ENV=value executable

or 

   executable | filter

has some drawback (for instance now the original executable process is
not child so it's harder to send signals) so I used exec like

  ENV=value exec executable

and a special syntax for filter

  OUTPUT=>(exec gzip > compress_file) exec executable

this lead to executable as direct child and gzip child of executable
but for some reasons still does not work (I can see gzip is compressing
an empty file so seems to work but perhaps does not receive output, qemu,
the executable, looks stuck). As bash is not used interactive in this way
it should not be a problem with tty/sid or similar.

So still helpful (I can record without worrying to fill my disk!) and
works always. Not counting that at the and I can use more compressors
with different options and record code is smaller (at the expense of
another process).

About the commit message, yes, I should do something about it.

Frediano

> 
> On Fri, 2015-08-21 at 09:46 +0100, Frediano Ziglio wrote:
> > This allows compressions using external programs or any type
> > of filters.
> 
> Would be useful to specify how to do this in the commit message. At
> least mention the name of the environment variable.
> 
> 
> > 
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> >  server/red_record_qxl.c | 50
> >  +++++++++++++++++++++++++++++++++++++++++++++++++
> >  server/red_record_qxl.h |  2 ++
> >  server/red_worker.c     |  2 +-
> >  3 files changed, 53 insertions(+), 1 deletion(-)
> > 
> > diff --git a/server/red_record_qxl.c b/server/red_record_qxl.c
> > index d96fb79..6a0b8fd 100644
> > --- a/server/red_record_qxl.c
> > +++ b/server/red_record_qxl.c
> > @@ -21,6 +21,8 @@
> >  
> >  #include <stdbool.h>
> >  #include <inttypes.h>
> > +#include <fcntl.h>
> > +#include <glib.h>
> >  #include "red_worker.h"
> >  #include "red_common.h"
> >  #include "red_memslots.h"
> > @@ -825,3 +827,51 @@ void red_record_qxl_command(FILE *fd, RedMemSlotInfo
> > *slots,
> >          break;
> >      }
> >  }
> > +
> > +static void out_setup(gpointer user_data)
> > +{
> > +    int fd = GPOINTER_TO_INT(user_data);
> > +
> > +    dup2(fd, 1);
> > +    close(fd);
> > +    fcntl(1, F_SETFD, 0);
> > +}
> > +
> > +FILE *red_record_open_file(const char *record_filename)
> > +{
> > +    const char *filter;
> > +    FILE *f;
> > +
> > +    f = fopen(record_filename, "w+");
> > +    if (!f)
> > +        return NULL;
> > +
> > +    filter = getenv("SPICE_WORKER_RECORD_FILTER");
> > +    if (filter) {
> > +        gint argc;
> > +        gchar **argv = NULL;
> > +        GError *error = NULL;
> > +        GPid child_pid;
> > +        gboolean ret;
> > +        gint fd_in;
> > +
> > +        if (!g_shell_parse_argv(filter, &argc, &argv, &error)) {
> > +            fclose(f);
> > +            return NULL;
> > +        }
> > +
> > +        ret = g_spawn_async_with_pipes(NULL, argv, NULL,
> > G_SPAWN_SEARCH_PATH, out_setup,
> > +                                       GINT_TO_POINTER(fileno(f)),
> > &child_pid,
> > +                                       &fd_in, NULL, NULL, &error);
> > +
> > +        g_strfreev(argv);
> > +        if (!ret) {
> > +            g_error_free(error);
> > +            fclose(f);
> > +            return NULL;
> > +        }
> > +        dup2(fd_in, fileno(f));
> > +        close(fd_in);
> > +    }
> > +    return f;
> > +}
> > diff --git a/server/red_record_qxl.h b/server/red_record_qxl.h
> > index b737db8..0b74dc4 100644
> > --- a/server/red_record_qxl.h
> > +++ b/server/red_record_qxl.h
> > @@ -31,4 +31,6 @@ void red_record_event(FILE *fd, int what, uint32_t type,
> > unsigned long ts);
> >  void red_record_qxl_command(FILE *fd, RedMemSlotInfo *slots,
> >                              QXLCommandExt ext_cmd, unsigned long ts);
> >  
> > +FILE *red_record_open_file(const char *record_filename);
> > +
> >  #endif
> > diff --git a/server/red_worker.c b/server/red_worker.c
> > index 7d7858e..a80dc93 100644
> > --- a/server/red_worker.c
> > +++ b/server/red_worker.c
> > @@ -12107,7 +12107,7 @@ static void red_init(RedWorker *worker,
> > WorkerInitData *init_data)
> >      if (record_filename) {
> >          static const char header[] = "SPICE_REPLAY 1\n";
> >  
> > -        worker->record_fd = fopen(record_filename, "w+");
> > +        worker->record_fd = red_record_open_file(record_filename);
> >          if (worker->record_fd == NULL) {
> >              spice_error("failed to open recording file %s\n",
> >              record_filename);
> >          }
> 
> 
>