| Message ID | 1308561383-18964-1-git-send-email-cyliu@novell.com |
|---|---|
| State | New |
| Headers | show |
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 <cyliu@novell.com> > --- > 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
On Mon, 2011-06-20 at 19:42 +0100, Stefano Stabellini wrote: > > @@ -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 surely getenv() isn't the correct interface anyway? What guarantees that the environment running qemu will have the /etc/sysconfig/xencommons derived settings in it? It's not usually the case under xl unless the user happens to have sources the file (which would be strange). Also XENCONSOLED_TRACE is clearly a setting aimed at xenconsoled -- not qemu. In fact it is really a setting for the xencommons initscript which turns it into the appropriate xenconsoled command line option. Perhaps we should be plumbing the console to xenconsoled (like we do for serial = pty), that'll get us this logging for free and be consistent to Xen users compared with PV guests, HVM serial=pty and similar... The other option would be to properly plumb this stuff through the qemu command line and make use of it from the Xen toolstack as necessary. Ian.
On Thu, 23 Jun 2011, Ian Campbell wrote: > On Mon, 2011-06-20 at 19:42 +0100, Stefano Stabellini wrote: > > > @@ -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 > > surely getenv() isn't the correct interface anyway? > > What guarantees that the environment running qemu will have > the /etc/sysconfig/xencommons derived settings in it? It's not usually > the case under xl unless the user happens to have sources the file > (which would be strange). > > Also XENCONSOLED_TRACE is clearly a setting aimed at xenconsoled -- not > qemu. In fact it is really a setting for the xencommons initscript which > turns it into the appropriate xenconsoled command line option. Considering that xenconsoled and qemu consoles are equivalent and interchangeable, it is a good idea to support XENCONSOLED_TRACE is qemu as well. How we do that is another matter. It is certainly more coherent from the qemu point of view if we pass it as a command line option, like Anthony suggested. > Perhaps we should be plumbing the console to xenconsoled (like we do for > serial = pty), that'll get us this logging for free and be consistent to > Xen users compared with PV guests, HVM serial=pty and similar... xenconsoled doesn't support any output other than a pty and doesn't support multiple PV consoles, that are going to be required at least for linux stubdoms. > The other option would be to properly plumb this stuff through the qemu > command line and make use of it from the Xen toolstack as necessary. That would be good, maybe cleaner.
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; +} + 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)); + } + 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; +} + /* -------------------------------------------------------------------- */ 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; } @@ -230,6 +284,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 +302,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. Log info will be saved in /var/log/xen/console/guest-domUname.log. Signed-off-by: Chunyan Liu <cyliu@novell.com> --- hw/xen_console.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 63 insertions(+), 0 deletions(-)