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

[Frediano Ziglio]

> 
> Hi
> 
> ----- Original Message -----> > ----- Original Message -----
> > > > This allows compressions using external programs or any type
> > > > of filters.
> > > > 
> > > 
> > > The shell worked well for that already. Why do you need it?
> > > 
> > 
> > How to use the shell in this case ?
> > Make a fifo and pass it ? Who is going to delete the fifo?
> > I did it as the same reason why there is the zlib code; I think compressing
> > whole file is better.
> 
> Good question, I remember I did us xz for compression, and you can
> see in further commit I disabled zlib. But not sure how now.
> 
> Fifo is a solution, I guess I was simply doing
> SPICE_WORKER_RECORD_FILENAME=/dev/stdout | xz
> 
> Assuming that nothing else would go in stdout, imho that is a reasonable
> assumption for a server code. debugging/log should go in stderr.
> 
> Imho, it's not necessary to add this.
> 

No, stdout is quite risky. Unfortunately using printf as a debugging is too used.

I found a bash solution (at least supported in Linux):

SPICE_WORKER_RECORD_FILENAME=>(gzip > /tmp/spice-record.$$.gz) exec /usr/bin/qemu-system-x86_64.bin "$@"

yes... quite strange syntax but works and don't create a fifo but a pipe. I would ask how portable it is.
Looks like one of the feature that is not supported by all bash. At least we should check if supported by
all bash-es we support (RHEL5/6/7).

Well... found some time to try with RHEL 5.11, RHEL 6... the bash syntax works! So now that I master
this new feature I can ditch my patch!
I can change emulator from virsh and use a script to change the environment in this way.

Yes, zlib is disabled with #if, I would remove it entirely.

Frediano

> > 
> > Frediano
> > 
> > > > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > > > ---
> > > >  server/red_record_qxl.c | 49
> > > >  +++++++++++++++++++++++++++++++++++++++++++++++++
> > > >  server/red_record_qxl.h |  2 ++
> > > >  server/red_worker.c     |  2 +-
> > > >  3 files changed, 52 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/server/red_record_qxl.c b/server/red_record_qxl.c
> > > > index d96fb79..d6b837c 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,50 @@ 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) {
> > > > +            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);
> > > >          }
> > > > --
> > > > 2.4.3
> > > > 
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> > 
>