| Message ID | 1309417722-11424-1-git-send-email-cyliu@novell.com |
|---|---|
| State | New |
| Headers | show |
On 30.06.2011, at 09:08, Chunyan Liu wrote: > Add code to support logging xen-domU console, as what xenconsoled does. To > enable logging, set environment variable XENCONSOLED_TRACE=guest and > XENCONSOLED_LOGDIR=<specified directory>, log file will be saved in <specified > directory>. So far, qemu doesn't rely on environment variables and I don't see a compelling reason to introduce us relying on them here. Keep in mind that with Xenner, this code is also used without Xen as the host, so it's just like any other device driver in qemu. Please change it to command line options. Also, please run checkpatch.pl on your patch before sending it :). Alex
On 30.06.2011, at 09:08, Chunyan Liu wrote: > Add code to support logging xen-domU console, as what xenconsoled does. To > enable logging, set environment variable XENCONSOLED_TRACE=guest and > XENCONSOLED_LOGDIR=<specified directory>, log file will be saved in <specified > directory>. In fact, this whole thing looks as if you're merely trying to reimplement "tee" on the xenconsole output. Wouldn't it make more sense to do this in the char layer? So we could do: -xenconsole tee:stdio,file:/tmp/xen.log or similar? That's probably a lot more useful than a random Xen specific hack. Alex
On Thursday, June 30, 2011 03:58:57 PM Alexander Graf wrote: > On 30.06.2011, at 09:08, Chunyan Liu wrote: > > Add code to support logging xen-domU console, as what xenconsoled does. > > To enable logging, set environment variable XENCONSOLED_TRACE=guest and > > XENCONSOLED_LOGDIR=<specified directory>, log file will be saved in > > <specified directory>. > > In fact, this whole thing looks as if you're merely trying to reimplement > "tee" on the xenconsole output. Wouldn't it make more sense to do this in > the char layer? So we could do: > > -xenconsole tee:stdio,file:/tmp/xen.log > > or similar? That's probably a lot more useful than a random Xen specific > hack. > Thanks, Alex. It IS something like "tee". But IMO, change in xen_console.c and change in char layer are just different time points, do not have essential difference. Change in xen_console.c is trying to backup output data into log file before sending to char device, change in char device is trying to dupicate data from char device to log file. Correct me if I'm wrong. Thanks, Chunyan
On 06/30/2011 11:39 AM, Chun Yan Liu wrote: > On Thursday, June 30, 2011 03:58:57 PM Alexander Graf wrote: >> On 30.06.2011, at 09:08, Chunyan Liu wrote: >>> Add code to support logging xen-domU console, as what xenconsoled does. >>> To enable logging, set environment variable XENCONSOLED_TRACE=guest and >>> XENCONSOLED_LOGDIR=<specified directory>, log file will be saved in >>> <specified directory>. >> In fact, this whole thing looks as if you're merely trying to reimplement >> "tee" on the xenconsole output. Wouldn't it make more sense to do this in >> the char layer? So we could do: >> >> -xenconsole tee:stdio,file:/tmp/xen.log >> >> or similar? That's probably a lot more useful than a random Xen specific >> hack. >> > Thanks, Alex. It IS something like "tee". But IMO, change in xen_console.c > and change in char layer are just different time points, do not have essential > difference. Change in xen_console.c is trying to backup output data into log > file before sending to char device, change in char device is trying to > dupicate data from char device to log file. Correct me if I'm wrong. Sure, the outcome is the same though, no? We get the output data in both a file and the char backend. Alex
On Thursday, June 30, 2011 07:18:54 PM you wrote: > On 06/30/2011 11:39 AM, Chun Yan Liu wrote: > > On Thursday, June 30, 2011 03:58:57 PM Alexander Graf wrote: > >> On 30.06.2011, at 09:08, Chunyan Liu wrote: > >>> Add code to support logging xen-domU console, as what xenconsoled does. > >>> To enable logging, set environment variable XENCONSOLED_TRACE=guest and > >>> XENCONSOLED_LOGDIR=<specified directory>, log file will be saved in > >>> <specified directory>. > >> > >> In fact, this whole thing looks as if you're merely trying to > >> reimplement "tee" on the xenconsole output. Wouldn't it make more sense > >> to do this in > >> > >> the char layer? So we could do: > >> -xenconsole tee:stdio,file:/tmp/xen.log > >> > >> or similar? That's probably a lot more useful than a random Xen specific > >> hack. > > > > Thanks, Alex. It IS something like "tee". But IMO, change in > > xen_console.c and change in char layer are just different time points, > > do not have essential difference. Change in xen_console.c is trying to > > backup output data into log file before sending to char device, change > > in char device is trying to dupicate data from char device to log file. > > Correct me if I'm wrong. > > Sure, the outcome is the same though, no? We get the output data in both > a file and the char backend. > Char device in qemu is not only used by console. Compared with the benefits that brings, I still doubt if it is proper to adding this functionality to char layer. Stefano, how do you think? Thanks, Chunyan
On Fri, 1 Jul 2011, Chun Yan Liu wrote: > On Thursday, June 30, 2011 07:18:54 PM you wrote: > > On 06/30/2011 11:39 AM, Chun Yan Liu wrote: > > > On Thursday, June 30, 2011 03:58:57 PM Alexander Graf wrote: > > >> On 30.06.2011, at 09:08, Chunyan Liu wrote: > > >>> Add code to support logging xen-domU console, as what xenconsoled does. > > >>> To enable logging, set environment variable XENCONSOLED_TRACE=guest and > > >>> XENCONSOLED_LOGDIR=<specified directory>, log file will be saved in > > >>> <specified directory>. > > >> > > >> In fact, this whole thing looks as if you're merely trying to > > >> reimplement "tee" on the xenconsole output. Wouldn't it make more sense > > >> to do this in > > >> > > >> the char layer? So we could do: > > >> -xenconsole tee:stdio,file:/tmp/xen.log > > >> > > >> or similar? That's probably a lot more useful than a random Xen specific > > >> hack. > > > > > > Thanks, Alex. It IS something like "tee". But IMO, change in > > > xen_console.c and change in char layer are just different time points, > > > do not have essential difference. Change in xen_console.c is trying to > > > backup output data into log file before sending to char device, change > > > in char device is trying to dupicate data from char device to log file. > > > Correct me if I'm wrong. > > > > Sure, the outcome is the same though, no? We get the output data in both > > a file and the char backend. > > > Char device in qemu is not only used by console. Compared with the benefits > that brings, I still doubt if it is proper to adding this functionality to > char layer. > Stefano, how do you think? I agree with Alexander that Qemu should take a command line argument to enable xen_console logging, rather than reading an environmental variable. However considering that xen_console allocates char drivers dynamically, the command line syntax proposed by Alexander is not a good fit. We need a global command line parameter that enables logging for all xen_console ptys, for example: -xencon_logs /path/to/logdir Regarding the particular way to implement logging, I think this patch is OK. Of course if the char layer had a "tee" option we could use it to log the pty output, for example prepending "tee:/log/to/file," to the filename parameter that we are going to pass to qemu_chr_open.
diff --git a/hw/xen_console.c b/hw/xen_console.c index c6c8163..51e2ba2 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; +} + static void buffer_append(struct XenConsole *con) { struct buffer *buffer = &con->buffer; @@ -81,6 +99,17 @@ 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)); + } + } + if (buffer->max_capacity && buffer->size > buffer->max_capacity) { /* Discard the middle of the data. */ @@ -174,12 +203,51 @@ static void xencons_send(struct XenConsole *con) } } +static int create_domain_log(struct XenConsole *con) +{ + char *logfile; + char *path, *domname; + int fd; + char *logdir; + + path = xs_get_domain_path(xenstore, con->xendev.dom); + domname = xenstore_read_str(path, "name"); + free(path); + if (!domname) + return -1; + + logdir = getenv("XENCONSOLED_LOGDIR"); + if (!logdir) + return -1; + if (mkdir(logdir, 0755) && errno != EEXIST) + { + xen_be_printf(&con->xendev, 1, + "Directory %s does not exist and fail to create it!", + logdir); + return -1; + } + + asprintf(&logfile, "%s/guest-%s.log", logdir, 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; +} + /* -------------------------------------------------------------------- */ 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 +266,11 @@ static int con_init(struct XenDevice *xendev) else con->chr = serial_hds[con->xendev.dev]; + logenv = getenv("XENCONSOLED_TRACE"); + if (logenv != NULL && strlen(logenv) == strlen("guest") + && !strcmp(logenv, "guest")) { + log_guest = 1; + } return 0; } @@ -230,6 +303,9 @@ static int con_connect(struct XenDevice *xendev) con->xendev.remote_port, con->xendev.local_port, con->buffer.max_capacity); + con->log_fd = -1; + if (log_guest) + con->log_fd = create_domain_log(con); return 0; } @@ -245,6 +321,12 @@ static void con_disconnect(struct XenDevice *xendev) munmap(con->sring, XC_PAGE_SIZE); con->sring = NULL; } + + if (con->log_fd != -1) { + close(con->log_fd); + con->log_fd = -1; + } + } static void con_event(struct XenDevice *xendev)
Add code to support logging xen-domU console, as what xenconsoled does. To enable logging, set environment variable XENCONSOLED_TRACE=guest and XENCONSOLED_LOGDIR=<specified directory>, log file will be saved in <specified directory>. Signed-off-by: Chunyan Liu <cyliu@novell.com> --- hw/xen_console.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 82 insertions(+), 0 deletions(-)