Re: [Qemu-devel] [PATCH V2] Add "tee" option to qemu char device

[Anthony Liguori <anthony@codemonkey.ws>]

On 07/14/2011 03:58 AM, Chunyan Liu wrote:
> Add "tee" backend to char device. It could be used as follows:
>      -serial tee:filepath,pty
>      -chardev tee,tee_fpath=path,tee_backend=pty,,path=path,,[mux=on|off]
> With "tee" option, "pty" output would be duplicated to filepath.
> Related thread:
> http://lists.nongnu.org/archive/html/qemu-devel/2011-07/msg00105.html

I loathe adding even more complexity to the the char layer.  Can't you 
do this just as well with socat?

Regards,

Anthony Liguori

>
> V2 changes:
>      -implement "tee" as a new backend. V1 implemented "tee" as a option.
>      -add documentation in qemu-options.hx.
>
> Please review. Thanks.
>
> ---
>   qemu-char.c     |  168 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>   qemu-config.c   |    6 ++
>   qemu-options.hx |   25 ++++++++-
>   3 files changed, 197 insertions(+), 2 deletions(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index fb13b28..99e49a9 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -228,6 +228,156 @@ static CharDriverState *qemu_chr_open_null(QemuOpts *opts)
>       return chr;
>   }
>
> +/* Tee driver */
> +typedef struct {
> +    CharDriverState *basechr; /* base io*/
> +    CharDriverState *filechr; /* duplicate output to file */
> +} TeeDriver;
> +
> +static void tee_init(CharDriverState *chr)
> +{
> +    TeeDriver *s = chr->opaque;
> +    if (s->basechr->init) {
> +        s->basechr->init(s->basechr);
> +    }
> +    if (s->filechr->init) {
> +        s->filechr->init(s->filechr);
> +    }
> +}
> +
> +static void tee_chr_update_read_handler(CharDriverState *chr)
> +{
> +    TeeDriver *s = chr->opaque;
> +    qemu_chr_add_handlers(s->basechr, chr->chr_can_read, chr->chr_read,
> +                              chr->chr_event, chr->handler_opaque);
> +}
> +
> +/* tee_chr_write will return the write result of basechr, write result to file
> + * will be ignored. FIX ME. */
> +static int tee_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
> +{
> +    TeeDriver *s = chr->opaque;
> +    if (s->filechr->chr_write) {
> +        s->filechr->chr_write(s->filechr, buf, len);
> +    }
> +    if (s->basechr->chr_write) {
> +        return s->basechr->chr_write(s->basechr, buf, len);
> +    }
> +    return 0;
> +}
> +
> +static void tee_chr_close(CharDriverState *chr)
> +{
> +    TeeDriver *s = chr->opaque;
> +    if (s->basechr->chr_close) {
> +        s->basechr->chr_close(s->basechr);
> +    }
> +    if (s->filechr->chr_close) {
> +        s->filechr->chr_close(s->filechr);
> +    }
> +    qemu_free(s);
> +}
> +
> +static int tee_chr_ioctl(CharDriverState *chr, int cmd, void *arg)
> +{
> +    TeeDriver *s = chr->opaque;
> +    if (s->basechr->chr_ioctl) {
> +        return s->basechr->chr_ioctl(s->basechr, cmd, arg);
> +    }
> +    return 0;
> +}
> +
> +static int tee_get_msgfd(CharDriverState *chr)
> +{
> +    TeeDriver *s = chr->opaque;
> +    if (s->basechr->get_msgfd) {
> +        return s->basechr->get_msgfd(s->basechr);
> +    }
> +    return -1;
> +}
> +
> +static void tee_chr_send_event(CharDriverState *chr, int event)
> +{
> +    TeeDriver *s = chr->opaque;
> +    if (s->basechr->chr_send_event) {
> +        s->basechr->chr_send_event(s->basechr, event);
> +    }
> +}
> +
> +static void tee_chr_accept_input(CharDriverState *chr)
> +{
> +    TeeDriver *s = chr->opaque;
> +    if (s->basechr->chr_accept_input) {
> +        s->basechr->chr_accept_input(s->basechr);
> +    }
> +}
> +static void tee_chr_set_echo(CharDriverState *chr, bool echo)
> +{
> +    TeeDriver *s = chr->opaque;
> +    if (s->basechr->chr_set_echo) {
> +        s->basechr->chr_set_echo(s->basechr, echo);
> +    }
> +}
> +static void tee_chr_guest_open(CharDriverState *chr)
> +{
> +    TeeDriver *s = chr->opaque;
> +    if (s->basechr->chr_guest_open) {
> +        s->basechr->chr_guest_open(s->basechr);
> +    }
> +}
> +static void tee_chr_guest_close(CharDriverState *chr)
> +{
> +    TeeDriver *s = chr->opaque;
> +    if (s->basechr->chr_guest_close) {
> +        s->basechr->chr_guest_close(s->basechr);
> +    }
> +}
> +
> +static CharDriverState *qemu_chr_open_tee(QemuOpts *opts)
> +{
> +    CharDriverState *chr;
> +    TeeDriver *d;
> +    CharDriverState *basechr;
> +    CharDriverState *filechr;
> +    const char *label = qemu_opts_id(opts);
> +    const char *tee_fpath = qemu_opt_get(opts, "tee_fpath");
> +    const char *tee_backend = qemu_opt_get(opts, "tee_backend");
> +    char *new_label, *new_filename;
> +    int sz;
> +
> +    chr = qemu_mallocz(sizeof(CharDriverState));
> +    d = qemu_mallocz(sizeof(TeeDriver));
> +
> +    sz = strlen(label)+3;
> +    new_label = qemu_malloc(sz);
> +    snprintf(new_label, sz, "%s-0", label);
> +    basechr = qemu_chr_open(new_label, tee_backend, NULL);
> +
> +    snprintf(new_label, sz, "%s-1", label);
> +    sz = strlen(tee_fpath)+6;
> +    new_filename = qemu_malloc(sz);
> +    snprintf(new_filename, sz, "file:%s", tee_fpath);
> +    filechr = qemu_chr_open(new_label, new_filename, NULL);
> +    qemu_free(new_label);
> +    qemu_free(new_filename);
> +
> +    d->basechr = basechr;
> +    d->filechr = filechr;
> +    chr->opaque = d;
> +    chr->init = tee_init;
> +    chr->chr_write = tee_chr_write;
> +    chr->chr_close = tee_chr_close;
> +    chr->chr_update_read_handler = tee_chr_update_read_handler;
> +    chr->chr_ioctl = tee_chr_ioctl;
> +    chr->get_msgfd = tee_get_msgfd;
> +    chr->chr_send_event = tee_chr_send_event;
> +    chr->chr_accept_input = tee_chr_accept_input;
> +    chr->chr_set_echo = tee_chr_set_echo;
> +    chr->chr_guest_open = tee_chr_guest_open;
> +    chr->chr_guest_close = tee_chr_guest_close;
> +
> +    return chr;
> +}
>   /* MUX driver for serial I/O splitting */
>   #define MAX_MUX 4
>   #define MUX_BUFFER_SIZE 32	/* Must be a power of 2.  */
> @@ -2356,6 +2506,16 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename)
>           qemu_opt_set(opts, "mux", "on");
>       }
>
> +    if (strstart(filename, "tee:",&p)) {
> +        char tee_fpath[1024];
> +        p = get_opt_value(tee_fpath, sizeof(tee_fpath), p);
> +        filename = p+1;
> +        qemu_opt_set(opts, "tee_backend", filename);
> +        qemu_opt_set(opts, "tee_fpath", tee_fpath);
> +        qemu_opt_set(opts, "backend", "tee");
> +        return opts;
> +   }
> +
>       if (strcmp(filename, "null")    == 0 ||
>           strcmp(filename, "pty")     == 0 ||
>           strcmp(filename, "msmouse") == 0 ||
> @@ -2468,6 +2628,7 @@ static const struct {
>       const char *name;
>       CharDriverState *(*open)(QemuOpts *opts);
>   } backend_table[] = {
> +    { .name = "tee",       .open = qemu_chr_open_tee },
>       { .name = "null",      .open = qemu_chr_open_null },
>       { .name = "socket",    .open = qemu_chr_open_socket },
>       { .name = "udp",       .open = qemu_chr_open_udp },
> @@ -2536,7 +2697,12 @@ CharDriverState *qemu_chr_open_opts(QemuOpts *opts,
>
>       if (!chr->filename)
>           chr->filename = qemu_strdup(qemu_opt_get(opts, "backend"));
> -    chr->init = init;
> +    if (strcmp(qemu_opt_get(opts, "backend"), "tee") == 0) {
> +        TeeDriver *s = chr->opaque;
> +        s->basechr->init = init;
> +    } else {
> +        chr->init = init;
> +    }
>       QTAILQ_INSERT_TAIL(&chardevs, chr, next);
>
>       if (qemu_opt_get_bool(opts, "mux", 0)) {
> diff --git a/qemu-config.c b/qemu-config.c
> index c63741c..b82516f 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -151,6 +151,12 @@ static QemuOptsList qemu_chardev_opts = {
>           },{
>               .name = "debug",
>               .type = QEMU_OPT_NUMBER,
> +        },{
> +            .name = "tee_backend",
> +            .type = QEMU_OPT_STRING,
> +        },{
> +            .name = "tee_fpath",
> +            .type = QEMU_OPT_STRING,
>           },
>           { /* end of list */ }
>       },
> diff --git a/qemu-options.hx b/qemu-options.hx
> index e6d7adc..1496f34 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1428,6 +1428,7 @@ DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
>   #if defined(CONFIG_SPICE)
>       "-chardev spicevmc,id=id,name=name[,debug=debug]\n"
>   #endif
> +    "-chardev tee,id=id,tee_backend=dev_string,tee_fpath=path\n"
>       , QEMU_ARCH_ALL
>   )
>
> @@ -1453,7 +1454,8 @@ Backend is one of:
>   @option{braille},
>   @option{tty},
>   @option{parport},
> -@option{spicevmc}.
> +@option{spicevmc},
> +@option{tee}.
>   The specific backend will determine the applicable options.
>
>   All devices must have an id, which can be any string up to 127 characters long.
> @@ -1639,6 +1641,18 @@ required.
>   Connect to a spice virtual machine channel, such as vdiport.
>   #endif
>
> +@item -chardev tee ,id=@var{id} ,tee_backend=@var{dev_string} ,tee_fpath=@var{path}
> +
> +@option{tee_backend} any kind of char device specified above. Double each comma
> +in device string to ensure the whole device string is the option value, "id" in
> +device string could be ignored. For example:
> +tee_backend=stdio,,mux=on,,signal=on
> +
> +@option{tee_fpath} file path where char device output content is duplicated to.
> +
> +Connect to a device specified by dev_string, and duplicate output content of
> +that device to a file.
> +
>   @end table
>   ETEXI
>
> @@ -1878,6 +1892,15 @@ A unix domain socket is used instead of a tcp socket.  The option works the
>   same as if you had specified @code{-serial tcp} except the unix domain socket
>   @var{path} is used for connections.
>
> +@item tee:@var{path},@var{dev_string}
> +Tee can duplicate output content of a serial device to a file.
> +@var{path} is the file path where output content is duplicated to.
> +@var{dev_string} should be any one of the serial devices specified
> +above. An example would be:
> +@table @code
> +@item -serial tee:/var/log/test.log,pty
> +@end table
> +
>   @item mon:@var{dev_string}
>   This is a special option to allow the monitor to be multiplexed onto
>   another serial port.  The monitor is accessed with key sequence of