Re: [Qemu-devel] [PATCH] Support logging xen-guest console

[Stefano Stabellini]

On Mon, 20 Jun 2011, Chunyan Liu wrote:
> Add code to support logging xen-domU console, as what xenconsoled does. Log 
> info
> will be saved in /var/log/xen/console/guest-domUname.log.
> 
> Signed-off-by: Chunyan Liu <address@hidden>
> ---
>  hw/xen_console.c |   63 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 63 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/xen_console.c b/hw/xen_console.c
> index c6c8163..ac3208d 100644
> --- a/hw/xen_console.c
> +++ b/hw/xen_console.c
> @@ -36,6 +36,8 @@
>  #include "qemu-char.h"
>  #include "xen_backend.h"
>  
> +static int log_guest = 0;
> +
>  struct buffer {
>      uint8_t *data;
>      size_t consumed;
> @@ -52,8 +54,24 @@ struct XenConsole {
>      void              *sring;
>      CharDriverState   *chr;
>      int               backlog;
> +    int               log_fd;
>  };
>  
> +static int write_all(int fd, const char* buf, size_t len)
> +{
> +    while (len) {
> +        ssize_t ret = write(fd, buf, len);
> +        if (ret == -1 && errno == EINTR)
> +            continue;
> +        if (ret <= 0)
> +            return -1;
> +        len -= ret;
> +        buf += ret;
> +    }
> +
> +    return 0;
> +}
> +

If I am not mistaken ret == 0 doesn't always mean an error on write.


>  static void buffer_append(struct XenConsole *con)
>  {
>      struct buffer *buffer = &con->buffer;
> @@ -81,6 +99,14 @@ static void buffer_append(struct XenConsole *con)
>      intf->out_cons = cons;
>      xen_be_send_notify(&con->xendev);
>  
> +    if (con->log_fd != -1) {
> +        int logret;
> +        logret = write_all(con->log_fd, buffer->data + buffer->size - size, 
> size);
> +        if (logret < 0)
> +            xen_be_printf(&con->xendev, 1, "Write to log failed on domain 
> %d: %d (%s)\n",
> +                      con->xendev.dom, errno, strerror(errno));
> +     }

code style: you needs brackets around the xen_be_printf statement


>      if (buffer->max_capacity &&
>       buffer->size > buffer->max_capacity) {
>       /* Discard the middle of the data. */
> @@ -174,12 +200,36 @@ static void xencons_send(struct XenConsole *con)
>      }
>  }
>  
> +static int create_domain_log(struct XenConsole *con)
> +{
> +    char *logfile;
> +    char *path, *domname;
> +    int fd;
> +
> +    path = xs_get_domain_path(xenstore, con->xendev.dom);
> +    domname = xenstore_read_str(path, "name");
> +    free(path);
> +    if (!domname)
> +        return -1;
> +
> +    asprintf(&logfile, "/var/log/xen/console/guest-%s.log", domname);
> +    qemu_free(domname);
> +
> +    fd = open(logfile, O_WRONLY|O_CREAT|O_APPEND, 0644);
> +    free(logfile);
> +    if (fd == -1)
> +        xen_be_printf(&con->xendev, 1,  "Failed to open log %s: %d (%s)", 
> logfile, errno, strerror(errno));
> +
> +    return fd;
> +}
> +

What if the "console" subdirectory is missing? Maybe we should create
the directory automatically here.


>  /* -------------------------------------------------------------------- */
>  
>  static int con_init(struct XenDevice *xendev)
>  {
>      struct XenConsole *con = container_of(xendev, struct XenConsole, xendev);
>      char *type, *dom;
> +    char *logenv = NULL;
>  
>      /* setup */
>      dom = xs_get_domain_path(xenstore, con->xendev.dom);
> @@ -198,6 +248,10 @@ static int con_init(struct XenDevice *xendev)
>      else
>          con->chr = serial_hds[con->xendev.dev];
>  
> +    logenv = getenv("XENCONSOLED_TRACE");
> +    if (logenv != NULL && !strcmp(logenv, "guest")) {
> +        log_guest = 1;
> +    }
>      return 0;
>  }

please check the length of logenv before using strcmp on it