* [PATCH 1/6] xenconsoled: introduce log file abstraction
2016-06-06 15:59 [PATCH 0/6] xenconsoled: rotating log file abstration Wei Liu
@ 2016-06-06 15:59 ` Wei Liu
2016-07-08 16:50 ` Ian Jackson
2016-06-06 15:59 ` [PATCH 2/6] xenconsoled: switch hypervisor log to use logfile abstraction Wei Liu
` (7 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Wei Liu @ 2016-06-06 15:59 UTC (permalink / raw)
To: Xen-devel; +Cc: Wei Liu, Ian Jackson
It will be used to handle hypervsior log and guest console log.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
tools/console/daemon/logfile.c | 229 +++++++++++++++++++++++++++++++++++++++++
tools/console/daemon/logfile.h | 57 ++++++++++
2 files changed, 286 insertions(+)
create mode 100644 tools/console/daemon/logfile.c
create mode 100644 tools/console/daemon/logfile.h
diff --git a/tools/console/daemon/logfile.c b/tools/console/daemon/logfile.c
new file mode 100644
index 0000000..ad73f8e
--- /dev/null
+++ b/tools/console/daemon/logfile.c
@@ -0,0 +1,229 @@
+/*
+ * Copyright (C) Citrix 2016
+ * Author(s): Wei Liu <wei.liu2@citrix.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; under version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; If not, see <http://www.gnu.org/licenses/>.
+ */
+#define _GNU_SOURCE
+
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <stdlib.h>
+#include <string.h>
+#include <stdio.h>
+
+#include "utils.h"
+#include "logfile.h"
+
+static void logfile_entry_free(struct logfile_entry *entry)
+{
+ if (!entry) return;
+
+ if (entry->fd >= 0) close(entry->fd);
+ free(entry);
+}
+
+void logfile_free(struct logfile *f)
+{
+ if (!f) return;
+
+ logfile_entry_free(f->entry);
+
+ free(f->basepath);
+ free(f);
+}
+
+static struct logfile_entry *logfile_entry_new(const char *path, mode_t mode)
+{
+ struct logfile_entry *entry;
+ struct stat sb;
+
+ entry = calloc(1, sizeof(*entry));
+ if (!entry) goto err;
+
+ entry->fd = open(path, O_CREAT|O_APPEND|O_WRONLY|O_CLOEXEC, mode);
+ if (entry->fd < 0) {
+ dolog(LOG_ERR, "Failed to open log file %s", path);
+ goto err;
+ }
+
+ entry->pos = lseek(entry->fd, 0, SEEK_END);
+ if (entry->pos == (off_t)-1) {
+ dolog(LOG_ERR, "Unable to determine current file offset in %s",
+ path);
+ goto err;
+ }
+
+ if (fstat(entry->fd, &sb) < 0) {
+ dolog(LOG_ERR, "Unable to fstat current file %s", path);
+ goto err;
+ }
+
+ entry->len = sb.st_size;
+ return entry;
+
+err:
+ logfile_entry_free(entry);
+ return NULL;
+}
+
+struct logfile *logfile_new(const char *path, mode_t mode)
+{
+ struct logfile *logfile;
+
+ logfile = calloc(1, sizeof(*logfile));
+ if (!logfile) goto err;
+
+ logfile->basepath = strdup(path);
+ if (!logfile->basepath) {
+ dolog(LOG_ERR, "Failed to strdup %s", path);
+ goto err;
+ }
+
+ logfile->mode = mode;
+ logfile->maxlen = LOGFILE_MAX_SIZE;
+
+
+ logfile->entry = logfile_entry_new(logfile->basepath, mode);
+
+ if (!logfile->entry) goto err;
+
+ return logfile;
+err:
+ logfile_free(logfile);
+ return NULL;
+}
+
+static int logfile_rollover(struct logfile *file)
+{
+ struct logfile_entry *old = file->entry, *new = NULL;
+ char *this = NULL;
+ char *next = NULL;
+ unsigned i;
+
+ if (asprintf(&next, "%s.%u", file->basepath,
+ LOGFILE_MAX_BACKUP_NUM) < 0) {
+ dolog(LOG_ERR, "Failed to asprintf %s.%u",
+ file->basepath, LOGFILE_MAX_BACKUP_NUM);
+ goto err;
+ }
+
+ for (i = LOGFILE_MAX_BACKUP_NUM; i > 0; i--) {
+ if (i == 1) {
+ this = strdup(file->basepath);
+ if (!this) {
+ dolog(LOG_ERR, "Failed to strdup %s",
+ file->basepath);
+ goto err;
+ }
+ } else {
+ if (asprintf(&this, "%s.%u", file->basepath, i-1) < 0) {
+ dolog(LOG_ERR, "Failed to asprintf %s.%u",
+ file->basepath, i-1);
+ goto err;
+ }
+ }
+
+ if (rename(this, next) < 0 && errno != ENOENT) {
+ dolog(LOG_ERR, "Failed to rename %s to %s",
+ this, next);
+ goto err;
+ }
+
+ free(next);
+ next = this;
+ this = NULL;
+ }
+
+ new = logfile_entry_new(file->basepath, file->mode);
+ if (!new) goto err;
+
+ file->entry = new;
+ logfile_entry_free(old);
+
+ return 0;
+err:
+ free(this);
+ free(next);
+ return -1;
+}
+
+static ssize_t write_all(int fd, const char* buf, size_t len)
+{
+ ssize_t written = 0;
+ while (len) {
+ ssize_t r = write(fd, buf, len);
+ if (r < 0 && errno == EINTR)
+ continue;
+ if (r < 0)
+ return r;
+ len -= r;
+ buf += r;
+ written += r;
+ }
+
+ return written;
+}
+
+ssize_t logfile_append(struct logfile *file, const char *buf,
+ size_t len)
+{
+ ssize_t ret = 0;
+
+ while (len) {
+ struct logfile_entry *entry = file->entry;
+ size_t towrite = len;
+ bool rollover = false;
+
+ if (entry->pos > file->maxlen) {
+ rollover = true;
+ towrite = 0;
+ } else if (entry->pos + towrite > file->maxlen) {
+ towrite = file->maxlen - entry->pos;
+ }
+
+ if (towrite) {
+ if (write_all(entry->fd, buf, towrite) != towrite) {
+ dolog(LOG_ERR, "Unable to write file %s",
+ file->basepath);
+ return -1;
+ }
+
+ len -= towrite;
+ buf += towrite;
+ ret += towrite;
+ entry->pos += towrite;
+ entry->len += towrite;
+ }
+
+ if ((entry->pos == file->maxlen && len) || rollover) {
+ dolog(LOG_DEBUG, "Roll over %s (maxlen %zu)",
+ file->basepath, file->maxlen);
+ if (logfile_rollover(file))
+ return -1;
+ }
+ }
+
+ return ret;
+}
+
+
+/*
+ * Local variables:
+ * c-file-style: "linux"
+ * indent-tabs-mode: t
+ * c-indent-level: 8
+ * c-basic-offset: 8
+ * tab-width: 8
+ * End:
+ */
diff --git a/tools/console/daemon/logfile.h b/tools/console/daemon/logfile.h
new file mode 100644
index 0000000..87f3c51
--- /dev/null
+++ b/tools/console/daemon/logfile.h
@@ -0,0 +1,57 @@
+/*
+ * Copyright (C) Citrix 2016
+ * Author(s): Wei Liu <wei.liu2@citrix.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; under version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef XENCONSOLED_LOGFILE_H
+#define XENCONSOLED_LOGFILE_H
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+#define LOGFILE_MAX_BACKUP_NUM 5
+#define LOGFILE_MAX_SIZE (1024*128)
+
+/* Append only abstraction for log file */
+struct logfile_entry {
+ int fd;
+ off_t pos;
+ off_t len;
+};
+
+struct logfile {
+ char *basepath;
+ mode_t mode;
+ off_t maxlen;
+ struct logfile_entry *entry;
+};
+
+struct logfile *logfile_new(const char *path, mode_t mode);
+void logfile_free(struct logfile *f);
+ssize_t logfile_append(struct logfile *file, const char *buf,
+ size_t len);
+
+#endif /* XENCONSOLED_LOGFILE_H */
+
+/*
+ * Local variables:
+ * c-file-style: "linux"
+ * indent-tabs-mode: t
+ * c-indent-level: 8
+ * c-basic-offset: 8
+ * tab-width: 8
+ * End:
+ */
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 1/6] xenconsoled: introduce log file abstraction
2016-06-06 15:59 ` [PATCH 1/6] xenconsoled: introduce log file abstraction Wei Liu
@ 2016-07-08 16:50 ` Ian Jackson
0 siblings, 0 replies; 25+ messages in thread
From: Ian Jackson @ 2016-07-08 16:50 UTC (permalink / raw)
To: Wei Liu; +Cc: Xen-devel
Wei Liu writes ("[PATCH 1/6] xenconsoled: introduce log file abstraction"):
> It will be used to handle hypervsior log and guest console log.
Thanks.
> diff --git a/tools/console/daemon/logfile.h b/tools/console/daemon/logfile.h
> new file mode 100644
> index 0000000..87f3c51
...
> +#define LOGFILE_MAX_BACKUP_NUM 5
> +#define LOGFILE_MAX_SIZE (1024*128)
These should surely be configurable by the end-user.
> +/* Append only abstraction for log file */
> +struct logfile_entry {
> + int fd;
> + off_t pos;
> + off_t len;
> +};
AFAICT these types could be defined inside logfile.c, and `struct
logfile' declared (but not defined) here in the header file.
That would make the abstraction clearer.
> diff --git a/tools/console/daemon/logfile.c b/tools/console/daemon/logfile.c
> new file mode 100644
> index 0000000..ad73f8e
> --- /dev/null
> +++ b/tools/console/daemon/logfile.c
...
> +static void logfile_entry_free(struct logfile_entry *entry)
> +{
> + if (!entry) return;
> +
> + if (entry->fd >= 0) close(entry->fd);
> + free(entry);
> +}
I do wonder whether it's actually worth having a whole separate struct
for an `entry'. Each `struct logfile' has zero or one `struct
logfile_entry's but most of the time exactly one.
Eliminating the separate struct logfile_entry and folding it into
struct logfile would save some memory allocation (and associated error
handling).
> + entry = calloc(1, sizeof(*entry));
> + if (!entry) goto err;
This pattern calls close(0) on error! Either declare that fd==0 means
"none here" (which would be sane IMO) or initialise fd=-1.
> + entry->fd = open(path, O_CREAT|O_APPEND|O_WRONLY|O_CLOEXEC, mode);
I think we had concluded that O_CLOEXEC wasn't sufficiently portable ?
It's not needed here because xenconsoled does not exec.
> + entry->pos = lseek(entry->fd, 0, SEEK_END);
> + if (entry->pos == (off_t)-1) {
> + dolog(LOG_ERR, "Unable to determine current file offset in %s",
> + path);
> + goto err;
> + }
> +
> + if (fstat(entry->fd, &sb) < 0) {
> + dolog(LOG_ERR, "Unable to fstat current file %s", path);
> + goto err;
> + }
I'm not sure why you need to keep track of `len' and `pos'
separately. AFAICT len is never used.
> +struct logfile *logfile_new(const char *path, mode_t mode)
> +{
Why does logfile_new take a mode for which all callers pass 644 ?
Do we anticipate logs with different permissions ?
> + if (rename(this, next) < 0 && errno != ENOENT) {
> + dolog(LOG_ERR, "Failed to rename %s to %s",
> + this, next);
> + goto err;
> + }
If LOGFILE_MAX_BACKUP_NUM becomes configurable, this loop will have to
become more sophisticated. I think it may have to count up and then
down again.
> +static ssize_t write_all(int fd, const char* buf, size_t len)
> +{
...
This is a copy of an identical function in io.c.
> +ssize_t logfile_append(struct logfile *file, const char *buf,
> + size_t len)
> +{
> + ssize_t ret = 0;
> +
> + while (len) {
> + struct logfile_entry *entry = file->entry;
> + size_t towrite = len;
> + bool rollover = false;
> +
> + if (entry->pos > file->maxlen) {
> + rollover = true;
> + towrite = 0;
> + } else if (entry->pos + towrite > file->maxlen) {
> + towrite = file->maxlen - entry->pos;
> + }
...
[and later]
> + if ((entry->pos == file->maxlen && len) || rollover) {
This logic is rather tangled. Why not
maxwrite = file->maxlen - file->pos;
if (towrite > maxwrite) {
rollover = 1;
towrite = maxwrite;
if (towrite < 0)
towrite = 0;
}
and then later simply
if (rollover) {
?
> +
> + if (towrite) {
> + if (write_all(entry->fd, buf, towrite) != towrite) {
> + dolog(LOG_ERR, "Unable to write file %s",
> + file->basepath);
> + return -1;
> + }
> +
> + len -= towrite;
> + buf += towrite;
> + ret += towrite;
I'm not a huge fan of this approach where several control variables
need to be kept in step. You could save the beginning and end of the
buffer, and then you could do away with ret and no longer need to
modify len.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 2/6] xenconsoled: switch hypervisor log to use logfile abstraction
2016-06-06 15:59 [PATCH 0/6] xenconsoled: rotating log file abstration Wei Liu
2016-06-06 15:59 ` [PATCH 1/6] xenconsoled: introduce log file abstraction Wei Liu
@ 2016-06-06 15:59 ` Wei Liu
2016-07-08 16:58 ` Ian Jackson
2016-06-06 15:59 ` [PATCH 3/6] xenconsoled: switch guest " Wei Liu
` (6 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Wei Liu @ 2016-06-06 15:59 UTC (permalink / raw)
To: Xen-devel; +Cc: Wei Liu, Ian Jackson
To minimise code churn, copy and paste a some existing functions and
adapt them to write to logfile. The functions to deal with fd directly
will go away eventually.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
tools/console/daemon/io.c | 95 +++++++++++++++++++++++++++++++++++------------
1 file changed, 71 insertions(+), 24 deletions(-)
diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
index 7e6a886..228c4af 100644
--- a/tools/console/daemon/io.c
+++ b/tools/console/daemon/io.c
@@ -21,6 +21,7 @@
#include "utils.h"
#include "io.h"
+#include "logfile.h"
#include <xenevtchn.h>
#include <xengnttab.h>
#include <xenstore.h>
@@ -71,7 +72,7 @@ extern int discard_overflowed_data;
static int log_time_hv_needts = 1;
static int log_time_guest_needts = 1;
-static int log_hv_fd = -1;
+static struct logfile *log_hv_file;
static xengnttab_handle *xgt_handle = NULL;
@@ -127,6 +128,15 @@ static int write_all(int fd, const char* buf, size_t len)
return 0;
}
+static int write_logfile(struct logfile *logfile, const char *buf,
+ size_t len)
+{
+ ssize_t r = logfile_append(logfile, buf, len);
+
+ if (r < 0) return -1;
+ return 0;
+}
+
static int write_with_timestamp(int fd, const char *data, size_t sz,
int *needts)
{
@@ -158,6 +168,38 @@ static int write_with_timestamp(int fd, const char *data, size_t sz,
return 0;
}
+static int write_logfile_with_timestamp(struct logfile *logfile,
+ const char *data, size_t sz,
+ int *needts)
+{
+ char ts[32];
+ time_t now = time(NULL);
+ const struct tm *tmnow = localtime(&now);
+ size_t tslen = strftime(ts, sizeof(ts), "[%Y-%m-%d %H:%M:%S] ", tmnow);
+ const char *last_byte = data + sz - 1;
+
+ while (data <= last_byte) {
+ const char *nl = memchr(data, '\n', last_byte + 1 - data);
+ int found_nl = (nl != NULL);
+ if (!found_nl)
+ nl = last_byte;
+
+ if ((*needts && logfile_append(logfile, ts, tslen))
+ || logfile_append(logfile, data, nl + 1 - data))
+ return -1;
+
+ *needts = found_nl;
+ data = nl + 1;
+ if (found_nl) {
+ // If we printed a newline, strip all \r following it
+ while (data <= last_byte && *data == '\r')
+ data++;
+ }
+ }
+
+ return 0;
+}
+
static void buffer_append(struct domain *dom)
{
struct buffer *buffer = &dom->buffer;
@@ -265,29 +307,33 @@ static bool domain_is_valid(int domid)
return ret;
}
-static int create_hv_log(void)
+static struct logfile *create_hv_log(void)
{
char logfile[PATH_MAX];
- int fd;
+ struct logfile *tmp;
+
snprintf(logfile, PATH_MAX-1, "%s/hypervisor.log", log_dir);
logfile[PATH_MAX-1] = '\0';
- fd = open(logfile, O_WRONLY|O_CREAT|O_APPEND, 0644);
- if (fd == -1)
+ tmp = logfile_new(logfile, 0644);
+
+ if (!tmp)
dolog(LOG_ERR, "Failed to open log %s: %d (%s)",
logfile, errno, strerror(errno));
- if (fd != -1 && log_time_hv) {
- if (write_with_timestamp(fd, "Logfile Opened\n",
- strlen("Logfile Opened\n"),
- &log_time_hv_needts) < 0) {
+
+ if (tmp && log_time_hv) {
+ if (write_logfile_with_timestamp(tmp, "Logfile Opened\n",
+ strlen("Logfile Opened\n"),
+ &log_time_hv_needts) < 0) {
dolog(LOG_ERR, "Failed to log opening timestamp "
- "in %s: %d (%s)", logfile, errno,
+ "in %s: %d (%s)", tmp->basepath, errno,
strerror(errno));
- close(fd);
- return -1;
+ logfile_free(tmp);
+ return NULL;
}
}
- return fd;
+
+ return tmp;
}
static int create_domain_log(struct domain *dom)
@@ -929,10 +975,11 @@ static void handle_hv_logs(xenevtchn_handle *xce_handle, bool force)
break;
if (log_time_hv)
- logret = write_with_timestamp(log_hv_fd, buffer, size,
- &log_time_hv_needts);
+ logret = write_logfile_with_timestamp(log_hv_file,
+ buffer, size,
+ &log_time_hv_needts);
else
- logret = write_all(log_hv_fd, buffer, size);
+ logret = write_logfile(log_hv_file, buffer, size);
if (logret < 0)
dolog(LOG_ERR, "Failed to write hypervisor log: "
@@ -955,9 +1002,9 @@ static void handle_log_reload(void)
}
if (log_hv) {
- if (log_hv_fd != -1)
- close(log_hv_fd);
- log_hv_fd = create_hv_log();
+ if (log_hv_file)
+ logfile_free(log_hv_file);
+ log_hv_file = create_hv_log();
}
}
@@ -1017,8 +1064,8 @@ void handle_io(void)
errno, strerror(errno));
goto out;
}
- log_hv_fd = create_hv_log();
- if (log_hv_fd == -1)
+ log_hv_file = create_hv_log();
+ if (!log_hv_file)
goto out;
log_hv_evtchn = xenevtchn_bind_virq(xce_handle, VIRQ_CON_RING);
if (log_hv_evtchn == -1) {
@@ -1199,9 +1246,9 @@ void handle_io(void)
current_array_size = 0;
out:
- if (log_hv_fd != -1) {
- close(log_hv_fd);
- log_hv_fd = -1;
+ if (log_hv_file) {
+ logfile_free(log_hv_file);
+ log_hv_file = NULL;
}
if (xce_handle != NULL) {
xenevtchn_close(xce_handle);
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 2/6] xenconsoled: switch hypervisor log to use logfile abstraction
2016-06-06 15:59 ` [PATCH 2/6] xenconsoled: switch hypervisor log to use logfile abstraction Wei Liu
@ 2016-07-08 16:58 ` Ian Jackson
0 siblings, 0 replies; 25+ messages in thread
From: Ian Jackson @ 2016-07-08 16:58 UTC (permalink / raw)
To: Wei Liu; +Cc: Xen-devel, Ian Jackson
Wei Liu writes ("[PATCH 2/6] xenconsoled: switch hypervisor log to use logfile abstraction"):
> To minimise code churn, copy and paste a some existing functions and
> adapt them to write to logfile. The functions to deal with fd directly
> will go away eventually.
...
> +static int write_logfile(struct logfile *logfile, const char *buf,
> + size_t len)
> +{
> + ssize_t r = logfile_append(logfile, buf, len);
> +
> + if (r < 0) return -1;
> + return 0;
> +}
That this is necessary suggests that logfile_append has an
inconvenient calling convention.
> +static int write_logfile_with_timestamp(struct logfile *logfile,
> + const char *data, size_t sz,
> + int *needts)
> +{
This should be in logfile.c. Furthermore, this is at the wrong level
with respect to log rotation:
> + if ((*needts && logfile_append(logfile, ts, tslen))
This could result in a timestamp being split across two different
logfiles which is undesirable. So I think this needs to be done at
the layer below.
> + data = nl + 1;
> + if (found_nl) {
> + // If we printed a newline, strip all \r following it
> + while (data <= last_byte && *data == '\r')
> + data++;
> + }
I would prefer that we only apply a lossless transformation to
logfiles, even if that means that our lotfiles contain CRs. Also,
this is rather odd. Surely it is more usual for the CR to precede the
LF.
I suggest the transformation `after every LF, preface the next byte
with the timestamp of that next byte'.
> - if (fd != -1 && log_time_hv) {
> - if (write_with_timestamp(fd, "Logfile Opened\n",
> - strlen("Logfile Opened\n"),
> - &log_time_hv_needts) < 0) {
> +
> + if (tmp && log_time_hv) {
> + if (write_logfile_with_timestamp(tmp, "Logfile Opened\n",
> + strlen("Logfile Opened\n"),
> + &log_time_hv_needts) < 0) {
I think that if timestamps are turned off, writing the "Logfile
Opened" (with no preceding timestamp) is probably a good idea. So
this can be made simpler.
> if (log_hv) {
> - if (log_hv_fd != -1)
> - close(log_hv_fd);
> - log_hv_fd = create_hv_log();
> + if (log_hv_file)
> + logfile_free(log_hv_file);
> + log_hv_file = create_hv_log();
You could separate out the nonfunctional changes if you did it in
stages:
* provide logfile_valid as a dummy macro that tests against -1
* replace `log_hv_fd' with `log_hv' throughout; replace
comparisons with -1 with logfile_valid(log_hv); likewise
client logfiles - no functional change
* replace log opening code with calls to logfile_open; change
types; change implementation of logfile_valid
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 3/6] xenconsoled: switch guest log to use logfile abstraction
2016-06-06 15:59 [PATCH 0/6] xenconsoled: rotating log file abstration Wei Liu
2016-06-06 15:59 ` [PATCH 1/6] xenconsoled: introduce log file abstraction Wei Liu
2016-06-06 15:59 ` [PATCH 2/6] xenconsoled: switch hypervisor log to use logfile abstraction Wei Liu
@ 2016-06-06 15:59 ` Wei Liu
2016-07-08 17:01 ` Ian Jackson
2016-06-06 15:59 ` [PATCH 4/6] xenconsoled: delete two now unused functions Wei Liu
` (5 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Wei Liu @ 2016-06-06 15:59 UTC (permalink / raw)
To: Xen-devel; +Cc: Wei Liu, Ian Jackson
Note that this causes write_with_timestamp to have no caller. Mark
it as unused for now to minimise code churn.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
tools/console/daemon/io.c | 67 +++++++++++++++++++++++++----------------------
1 file changed, 35 insertions(+), 32 deletions(-)
diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
index 228c4af..c2f63e6 100644
--- a/tools/console/daemon/io.c
+++ b/tools/console/daemon/io.c
@@ -95,7 +95,7 @@ struct domain {
int master_fd;
int master_pollfd_idx;
int slave_fd;
- int log_fd;
+ struct logfile *log;
bool is_dead;
unsigned last_seen;
struct buffer buffer;
@@ -137,8 +137,9 @@ static int write_logfile(struct logfile *logfile, const char *buf,
return 0;
}
-static int write_with_timestamp(int fd, const char *data, size_t sz,
- int *needts)
+static __attribute__((unused))
+int write_with_timestamp(int fd, const char *data, size_t sz,
+ int *needts)
{
char ts[32];
time_t now = time(NULL);
@@ -235,16 +236,16 @@ static void buffer_append(struct domain *dom)
* no one is listening on the console pty then it will fill up
* and handle_tty_write will stop being called.
*/
- if (dom->log_fd != -1) {
+ if (dom->log) {
int logret;
if (log_time_guest) {
- logret = write_with_timestamp(
- dom->log_fd,
+ logret = write_logfile_with_timestamp(
+ dom->log,
buffer->data + buffer->size - size,
size, &log_time_guest_needts);
} else {
- logret = write_all(
- dom->log_fd,
+ logret = write_logfile(
+ dom->log,
buffer->data + buffer->size - size,
size);
}
@@ -336,50 +337,53 @@ static struct logfile *create_hv_log(void)
return tmp;
}
-static int create_domain_log(struct domain *dom)
+static struct logfile *create_domain_log(struct domain *dom)
{
char logfile[PATH_MAX];
char *namepath, *data, *s;
- int fd;
unsigned int len;
+ struct logfile *tmp;
namepath = xs_get_domain_path(xs, dom->domid);
s = realloc(namepath, strlen(namepath) + 6);
if (s == NULL) {
free(namepath);
- return -1;
+ return NULL;
}
namepath = s;
strcat(namepath, "/name");
data = xs_read(xs, XBT_NULL, namepath, &len);
free(namepath);
if (!data)
- return -1;
+ return NULL;
if (!len) {
free(data);
- return -1;
+ return NULL;
}
snprintf(logfile, PATH_MAX-1, "%s/guest-%s.log", log_dir, data);
free(data);
logfile[PATH_MAX-1] = '\0';
- fd = open(logfile, O_WRONLY|O_CREAT|O_APPEND, 0644);
- if (fd == -1)
+ tmp = logfile_new(logfile, 0644);
+
+ if (!tmp)
dolog(LOG_ERR, "Failed to open log %s: %d (%s)",
logfile, errno, strerror(errno));
- if (fd != -1 && log_time_guest) {
- if (write_with_timestamp(fd, "Logfile Opened\n",
- strlen("Logfile Opened\n"),
- &log_time_guest_needts) < 0) {
+
+ if (tmp && log_time_guest) {
+ if (write_logfile_with_timestamp(tmp, "Logfile Opened\n",
+ strlen("Logfile Opened\n"),
+ &log_time_guest_needts) < 0) {
dolog(LOG_ERR, "Failed to log opening timestamp "
- "in %s: %d (%s)", logfile, errno,
+ "in %s: %d (%s)", tmp->basepath, errno,
strerror(errno));
- close(fd);
- return -1;
+ logfile_free(tmp);
+ return NULL;
}
}
- return fd;
+
+ return tmp;
}
static void domain_close_tty(struct domain *dom)
@@ -665,8 +669,8 @@ static int domain_create_ring(struct domain *dom)
}
}
- if (log_guest && (dom->log_fd == -1))
- dom->log_fd = create_domain_log(dom);
+ if (log_guest && !dom->log)
+ dom->log = create_domain_log(dom);
out:
return err;
@@ -724,7 +728,6 @@ static struct domain *create_domain(int domid)
dom->master_fd = -1;
dom->master_pollfd_idx = -1;
dom->slave_fd = -1;
- dom->log_fd = -1;
dom->xce_pollfd_idx = -1;
dom->next_period = ((long long)ts.tv_sec * 1000) + (ts.tv_nsec / 1000000) + RATE_LIMIT_PERIOD;
@@ -777,9 +780,9 @@ static void cleanup_domain(struct domain *d)
{
domain_close_tty(d);
- if (d->log_fd != -1) {
- close(d->log_fd);
- d->log_fd = -1;
+ if (d->log) {
+ logfile_free(d->log);
+ d->log = NULL;
}
free(d->buffer.data);
@@ -995,9 +998,9 @@ static void handle_log_reload(void)
if (log_guest) {
struct domain *d;
for (d = dom_head; d; d = d->next) {
- if (d->log_fd != -1)
- close(d->log_fd);
- d->log_fd = create_domain_log(d);
+ if (d->log)
+ logfile_free(d->log);
+ d->log = create_domain_log(d);
}
}
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 3/6] xenconsoled: switch guest log to use logfile abstraction
2016-06-06 15:59 ` [PATCH 3/6] xenconsoled: switch guest " Wei Liu
@ 2016-07-08 17:01 ` Ian Jackson
0 siblings, 0 replies; 25+ messages in thread
From: Ian Jackson @ 2016-07-08 17:01 UTC (permalink / raw)
To: Wei Liu; +Cc: Xen-devel
Wei Liu writes ("[PATCH 3/6] xenconsoled: switch guest log to use logfile abstraction"):
> Note that this causes write_with_timestamp to have no caller. Mark
> it as unused for now to minimise code churn.
This is plausible, although I think I would have structured this
series differently. (1. provide new api to old code; 2. switch
everyone to new api; 3. replace implementation.)
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 4/6] xenconsoled: delete two now unused functions
2016-06-06 15:59 [PATCH 0/6] xenconsoled: rotating log file abstration Wei Liu
` (2 preceding siblings ...)
2016-06-06 15:59 ` [PATCH 3/6] xenconsoled: switch guest " Wei Liu
@ 2016-06-06 15:59 ` Wei Liu
2016-07-08 17:01 ` Ian Jackson
2016-06-06 15:59 ` [PATCH 5/6] xenconsoled: options to control log rotation Wei Liu
` (4 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Wei Liu @ 2016-06-06 15:59 UTC (permalink / raw)
To: Xen-devel; +Cc: Wei Liu, Ian Jackson
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
tools/console/daemon/io.c | 47 -----------------------------------------------
1 file changed, 47 deletions(-)
diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
index c2f63e6..7c38232 100644
--- a/tools/console/daemon/io.c
+++ b/tools/console/daemon/io.c
@@ -113,21 +113,6 @@ struct domain {
static struct domain *dom_head;
-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 int write_logfile(struct logfile *logfile, const char *buf,
size_t len)
{
@@ -137,38 +122,6 @@ static int write_logfile(struct logfile *logfile, const char *buf,
return 0;
}
-static __attribute__((unused))
-int write_with_timestamp(int fd, const char *data, size_t sz,
- int *needts)
-{
- char ts[32];
- time_t now = time(NULL);
- const struct tm *tmnow = localtime(&now);
- size_t tslen = strftime(ts, sizeof(ts), "[%Y-%m-%d %H:%M:%S] ", tmnow);
- const char *last_byte = data + sz - 1;
-
- while (data <= last_byte) {
- const char *nl = memchr(data, '\n', last_byte + 1 - data);
- int found_nl = (nl != NULL);
- if (!found_nl)
- nl = last_byte;
-
- if ((*needts && write_all(fd, ts, tslen))
- || write_all(fd, data, nl + 1 - data))
- return -1;
-
- *needts = found_nl;
- data = nl + 1;
- if (found_nl) {
- // If we printed a newline, strip all \r following it
- while (data <= last_byte && *data == '\r')
- data++;
- }
- }
-
- return 0;
-}
-
static int write_logfile_with_timestamp(struct logfile *logfile,
const char *data, size_t sz,
int *needts)
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 5/6] xenconsoled: options to control log rotation
2016-06-06 15:59 [PATCH 0/6] xenconsoled: rotating log file abstration Wei Liu
` (3 preceding siblings ...)
2016-06-06 15:59 ` [PATCH 4/6] xenconsoled: delete two now unused functions Wei Liu
@ 2016-06-06 15:59 ` Wei Liu
2016-07-08 17:02 ` Ian Jackson
2016-06-06 15:59 ` [PATCH 6/6] xenconsoled: handle --log-backups 0 in logfile_rollover Wei Liu
` (3 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Wei Liu @ 2016-06-06 15:59 UTC (permalink / raw)
To: Xen-devel; +Cc: Wei Liu, Ian Jackson
Provide two options: one to control the maximum number of copies kept
and the other to control the maximum length of each log file.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
tools/console/daemon/logfile.c | 11 +++++++----
tools/console/daemon/main.c | 13 ++++++++++++-
2 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/tools/console/daemon/logfile.c b/tools/console/daemon/logfile.c
index ad73f8e..3b95f84 100644
--- a/tools/console/daemon/logfile.c
+++ b/tools/console/daemon/logfile.c
@@ -25,6 +25,9 @@
#include "utils.h"
#include "logfile.h"
+extern unsigned int log_backups;
+extern unsigned int log_max_len;
+
static void logfile_entry_free(struct logfile_entry *entry)
{
if (!entry) return;
@@ -91,7 +94,7 @@ struct logfile *logfile_new(const char *path, mode_t mode)
}
logfile->mode = mode;
- logfile->maxlen = LOGFILE_MAX_SIZE;
+ logfile->maxlen = log_max_len;
logfile->entry = logfile_entry_new(logfile->basepath, mode);
@@ -112,13 +115,13 @@ static int logfile_rollover(struct logfile *file)
unsigned i;
if (asprintf(&next, "%s.%u", file->basepath,
- LOGFILE_MAX_BACKUP_NUM) < 0) {
+ log_backups) < 0) {
dolog(LOG_ERR, "Failed to asprintf %s.%u",
- file->basepath, LOGFILE_MAX_BACKUP_NUM);
+ file->basepath, log_backups);
goto err;
}
- for (i = LOGFILE_MAX_BACKUP_NUM; i > 0; i--) {
+ for (i = log_backups; i > 0; i--) {
if (i == 1) {
this = strdup(file->basepath);
if (!this) {
diff --git a/tools/console/daemon/main.c b/tools/console/daemon/main.c
index 23860d3..dd338ae 100644
--- a/tools/console/daemon/main.c
+++ b/tools/console/daemon/main.c
@@ -31,6 +31,7 @@
#include "utils.h"
#include "io.h"
+#include "logfile.h"
int log_reload = 0;
int log_guest = 0;
@@ -39,6 +40,8 @@ int log_time_hv = 0;
int log_time_guest = 0;
char *log_dir = NULL;
int discard_overflowed_data = 1;
+unsigned int log_backups = LOGFILE_MAX_BACKUP_NUM;
+unsigned int log_max_len = LOGFILE_MAX_SIZE;
static void handle_hup(int sig)
{
@@ -47,7 +50,7 @@ static void handle_hup(int sig)
static void usage(char *name)
{
- printf("Usage: %s [-h] [-V] [-v] [-i] [--log=none|guest|hv|all] [--log-dir=DIR] [--pid-file=PATH] [-t, --timestamp=none|guest|hv|all] [-o, --overflow-data=discard|keep]\n", name);
+ printf("Usage: %s [-h] [-V] [-v] [-i] [--log=none|guest|hv|all] [--log-dir=DIR] [--pid-file=PATH] [--log-backups=NUM] [--log-max-len=NUM] [-t, --timestamp=none|guest|hv|all] [-o, --overflow-data=discard|keep]\n", name);
}
static void version(char *name)
@@ -100,6 +103,8 @@ int main(int argc, char **argv)
{ "interactive", 0, 0, 'i' },
{ "log", 1, 0, 'l' },
{ "log-dir", 1, 0, 'r' },
+ { "log-backups", 1, 0, 'n' },
+ { "log-max-len", 1, 0, 'm' },
{ "pid-file", 1, 0, 'p' },
{ "timestamp", 1, 0, 't' },
{ "overflow-data", 1, 0, 'o'},
@@ -144,6 +149,12 @@ int main(int argc, char **argv)
case 'r':
log_dir = strdup(optarg);
break;
+ case 'n':
+ log_backups = strtoul(optarg, 0, 0);
+ break;
+ case 'm':
+ log_max_len = strtoul(optarg, 0, 0);
+ break;
case 'p':
pidfile = strdup(optarg);
break;
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 6/6] xenconsoled: handle --log-backups 0 in logfile_rollover
2016-06-06 15:59 [PATCH 0/6] xenconsoled: rotating log file abstration Wei Liu
` (4 preceding siblings ...)
2016-06-06 15:59 ` [PATCH 5/6] xenconsoled: options to control log rotation Wei Liu
@ 2016-06-06 15:59 ` Wei Liu
2016-07-08 17:03 ` Ian Jackson
2016-06-06 20:40 ` [PATCH 0/6] xenconsoled: rotating log file abstration Doug Goldstein
` (2 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Wei Liu @ 2016-06-06 15:59 UTC (permalink / raw)
To: Xen-devel; +Cc: Wei Liu, Ian Jackson
We now allow user to configure the number of backups to keep. We need to
handle when the number is set to 0.
Check if number of backup is 0. If so, just unlink the file. Move the
rotation to `else' branch so that we skip it altogether.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
tools/console/daemon/logfile.c | 63 +++++++++++++++++++++++++-----------------
1 file changed, 37 insertions(+), 26 deletions(-)
diff --git a/tools/console/daemon/logfile.c b/tools/console/daemon/logfile.c
index 3b95f84..fd29ba5 100644
--- a/tools/console/daemon/logfile.c
+++ b/tools/console/daemon/logfile.c
@@ -114,38 +114,49 @@ static int logfile_rollover(struct logfile *file)
char *next = NULL;
unsigned i;
- if (asprintf(&next, "%s.%u", file->basepath,
- log_backups) < 0) {
- dolog(LOG_ERR, "Failed to asprintf %s.%u",
- file->basepath, log_backups);
- goto err;
- }
+ if (log_backups == 0) {
+ if (unlink(file->basepath) < 0 &&
+ errno != ENOENT) {
+ dolog(LOG_ERR, "Failed to unlink %s",
+ file->basepath);
+ goto err;
+ }
+ } else {
+ if (asprintf(&next, "%s.%u", file->basepath,
+ log_backups) < 0) {
+ dolog(LOG_ERR, "Failed to asprintf %s.%u",
+ file->basepath, log_backups);
+ goto err;
+ }
- for (i = log_backups; i > 0; i--) {
- if (i == 1) {
- this = strdup(file->basepath);
- if (!this) {
- dolog(LOG_ERR, "Failed to strdup %s",
- file->basepath);
- goto err;
+ for (i = log_backups; i > 0; i--) {
+ if (i == 1) {
+ this = strdup(file->basepath);
+ if (!this) {
+ dolog(LOG_ERR, "Failed to strdup %s",
+ file->basepath);
+ goto err;
+ }
+ } else {
+ if (asprintf(&this, "%s.%u", file->basepath,
+ i-1) < 0) {
+ dolog(LOG_ERR,
+ "Failed to asprintf %s.%u",
+ file->basepath, i-1);
+ goto err;
+ }
}
- } else {
- if (asprintf(&this, "%s.%u", file->basepath, i-1) < 0) {
- dolog(LOG_ERR, "Failed to asprintf %s.%u",
- file->basepath, i-1);
+
+ if (rename(this, next) < 0 && errno != ENOENT) {
+ dolog(LOG_ERR, "Failed to rename %s to %s",
+ this, next);
goto err;
}
- }
- if (rename(this, next) < 0 && errno != ENOENT) {
- dolog(LOG_ERR, "Failed to rename %s to %s",
- this, next);
- goto err;
+ free(next);
+ next = this;
+ this = NULL;
}
-
- free(next);
- next = this;
- this = NULL;
}
new = logfile_entry_new(file->basepath, file->mode);
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 0/6] xenconsoled: rotating log file abstration
2016-06-06 15:59 [PATCH 0/6] xenconsoled: rotating log file abstration Wei Liu
` (5 preceding siblings ...)
2016-06-06 15:59 ` [PATCH 6/6] xenconsoled: handle --log-backups 0 in logfile_rollover Wei Liu
@ 2016-06-06 20:40 ` Doug Goldstein
2016-06-07 8:07 ` Wei Liu
2016-06-07 9:44 ` David Vrabel
2016-07-02 10:24 ` Wei Liu
8 siblings, 1 reply; 25+ messages in thread
From: Doug Goldstein @ 2016-06-06 20:40 UTC (permalink / raw)
To: Wei Liu, Xen-devel; +Cc: Ian Jackson
[-- Attachment #1.1.1: Type: text/plain, Size: 1122 bytes --]
On 6/6/16 10:59 AM, Wei Liu wrote:
> Wei Liu (6):
> xenconsoled: introduce log file abstraction
> xenconsoled: switch hypervisor log to use logfile abstraction
> xenconsoled: switch guest log to use logfile abstraction
> xenconsoled: delete two now unused functions
> xenconsoled: options to control log rotation
> xenconsoled: handle --log-backups 0 in logfile_rollover
>
> tools/console/daemon/io.c | 139 +++++++++++------------
> tools/console/daemon/logfile.c | 243 +++++++++++++++++++++++++++++++++++++++++
> tools/console/daemon/logfile.h | 57 ++++++++++
> tools/console/daemon/main.c | 13 ++-
> 4 files changed, 383 insertions(+), 69 deletions(-)
> create mode 100644 tools/console/daemon/logfile.c
> create mode 100644 tools/console/daemon/logfile.h
>
Is there a mechanism for me to turn this off entirely? Due to some very
real possibilities like logging overwhelming xenconsoled and causing
qemu to block this is just not something that I will be able to roll out
in any of my deployments. We would rather lose logs than block qemu.
--
Doug Goldstein
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 0/6] xenconsoled: rotating log file abstration
2016-06-06 20:40 ` [PATCH 0/6] xenconsoled: rotating log file abstration Doug Goldstein
@ 2016-06-07 8:07 ` Wei Liu
0 siblings, 0 replies; 25+ messages in thread
From: Wei Liu @ 2016-06-07 8:07 UTC (permalink / raw)
To: Doug Goldstein; +Cc: Xen-devel, Wei Liu, Ian Jackson
On Mon, Jun 06, 2016 at 03:40:33PM -0500, Doug Goldstein wrote:
> On 6/6/16 10:59 AM, Wei Liu wrote:
> > Wei Liu (6):
> > xenconsoled: introduce log file abstraction
> > xenconsoled: switch hypervisor log to use logfile abstraction
> > xenconsoled: switch guest log to use logfile abstraction
> > xenconsoled: delete two now unused functions
> > xenconsoled: options to control log rotation
> > xenconsoled: handle --log-backups 0 in logfile_rollover
> >
> > tools/console/daemon/io.c | 139 +++++++++++------------
> > tools/console/daemon/logfile.c | 243 +++++++++++++++++++++++++++++++++++++++++
> > tools/console/daemon/logfile.h | 57 ++++++++++
> > tools/console/daemon/main.c | 13 ++-
> > 4 files changed, 383 insertions(+), 69 deletions(-)
> > create mode 100644 tools/console/daemon/logfile.c
> > create mode 100644 tools/console/daemon/logfile.h
> >
>
> Is there a mechanism for me to turn this off entirely? Due to some very
> real possibilities like logging overwhelming xenconsoled and causing
> qemu to block this is just not something that I will be able to roll out
> in any of my deployments. We would rather lose logs than block qemu.
>
This is far from handling log for QEMU.
That needs further discussion.
Wei.
> --
> Doug Goldstein
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 0/6] xenconsoled: rotating log file abstration
2016-06-06 15:59 [PATCH 0/6] xenconsoled: rotating log file abstration Wei Liu
` (6 preceding siblings ...)
2016-06-06 20:40 ` [PATCH 0/6] xenconsoled: rotating log file abstration Doug Goldstein
@ 2016-06-07 9:44 ` David Vrabel
2016-06-07 9:55 ` Wei Liu
2016-07-02 10:24 ` Wei Liu
8 siblings, 1 reply; 25+ messages in thread
From: David Vrabel @ 2016-06-07 9:44 UTC (permalink / raw)
To: Wei Liu, Xen-devel; +Cc: Ian Jackson
On 06/06/16 16:59, Wei Liu wrote:
> Wei Liu (6):
> xenconsoled: introduce log file abstraction
> xenconsoled: switch hypervisor log to use logfile abstraction
> xenconsoled: switch guest log to use logfile abstraction
> xenconsoled: delete two now unused functions
> xenconsoled: options to control log rotation
> xenconsoled: handle --log-backups 0 in logfile_rollover
What not use syslog and/or logrotate?
David
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 0/6] xenconsoled: rotating log file abstration
2016-06-07 9:44 ` David Vrabel
@ 2016-06-07 9:55 ` Wei Liu
2016-06-07 10:17 ` David Vrabel
0 siblings, 1 reply; 25+ messages in thread
From: Wei Liu @ 2016-06-07 9:55 UTC (permalink / raw)
To: David Vrabel; +Cc: Xen-devel, Wei Liu, Ian Jackson
On Tue, Jun 07, 2016 at 10:44:50AM +0100, David Vrabel wrote:
> On 06/06/16 16:59, Wei Liu wrote:
> > Wei Liu (6):
> > xenconsoled: introduce log file abstraction
> > xenconsoled: switch hypervisor log to use logfile abstraction
> > xenconsoled: switch guest log to use logfile abstraction
> > xenconsoled: delete two now unused functions
> > xenconsoled: options to control log rotation
> > xenconsoled: handle --log-backups 0 in logfile_rollover
>
> What not use syslog and/or logrotate?
>
It's cumbersome to setup per guest facility for syslog. We don't want
console log from different guests mix together in one file.
Logrorate runs periodically. It doesn't actively limit the disk space
consumed.
The combination of syslog and logrotate can't prevent a guest from
filling up Dom0 disk as far as I can tell. I'm happy to know if there is
some sort of simple configuration we can ship with open source Xen.
Wei.
> David
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 0/6] xenconsoled: rotating log file abstration
2016-06-07 9:55 ` Wei Liu
@ 2016-06-07 10:17 ` David Vrabel
2016-06-07 10:21 ` Wei Liu
0 siblings, 1 reply; 25+ messages in thread
From: David Vrabel @ 2016-06-07 10:17 UTC (permalink / raw)
To: Wei Liu, David Vrabel; +Cc: Xen-devel, Ian Jackson
On 07/06/16 10:55, Wei Liu wrote:
> On Tue, Jun 07, 2016 at 10:44:50AM +0100, David Vrabel wrote:
>> On 06/06/16 16:59, Wei Liu wrote:
>>> Wei Liu (6):
>>> xenconsoled: introduce log file abstraction
>>> xenconsoled: switch hypervisor log to use logfile abstraction
>>> xenconsoled: switch guest log to use logfile abstraction
>>> xenconsoled: delete two now unused functions
>>> xenconsoled: options to control log rotation
>>> xenconsoled: handle --log-backups 0 in logfile_rollover
>>
>> What not use syslog and/or logrotate?
>>
>
> It's cumbersome to setup per guest facility for syslog. We don't want
> console log from different guests mix together in one file.
>
> Logrorate runs periodically. It doesn't actively limit the disk space
> consumed.
>
> The combination of syslog and logrotate can't prevent a guest from
> filling up Dom0 disk as far as I can tell. I'm happy to know if there is
> some sort of simple configuration we can ship with open source Xen.
Ok. Found the other thread now.
We should just use virtlogd instead of re-inventing the wheel.
David
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 0/6] xenconsoled: rotating log file abstration
2016-06-07 10:17 ` David Vrabel
@ 2016-06-07 10:21 ` Wei Liu
2016-06-07 10:29 ` David Vrabel
0 siblings, 1 reply; 25+ messages in thread
From: Wei Liu @ 2016-06-07 10:21 UTC (permalink / raw)
To: David Vrabel; +Cc: Xen-devel, Wei Liu, Ian Jackson
On Tue, Jun 07, 2016 at 11:17:07AM +0100, David Vrabel wrote:
> On 07/06/16 10:55, Wei Liu wrote:
> > On Tue, Jun 07, 2016 at 10:44:50AM +0100, David Vrabel wrote:
> >> On 06/06/16 16:59, Wei Liu wrote:
> >>> Wei Liu (6):
> >>> xenconsoled: introduce log file abstraction
> >>> xenconsoled: switch hypervisor log to use logfile abstraction
> >>> xenconsoled: switch guest log to use logfile abstraction
> >>> xenconsoled: delete two now unused functions
> >>> xenconsoled: options to control log rotation
> >>> xenconsoled: handle --log-backups 0 in logfile_rollover
> >>
> >> What not use syslog and/or logrotate?
> >>
> >
> > It's cumbersome to setup per guest facility for syslog. We don't want
> > console log from different guests mix together in one file.
> >
> > Logrorate runs periodically. It doesn't actively limit the disk space
> > consumed.
> >
> > The combination of syslog and logrotate can't prevent a guest from
> > filling up Dom0 disk as far as I can tell. I'm happy to know if there is
> > some sort of simple configuration we can ship with open source Xen.
>
> Ok. Found the other thread now.
>
> We should just use virtlogd instead of re-inventing the wheel.
>
As in importing that to our own tree (potentially a fork)? Provide
interfaces to let system administrator configure that (need to at least
check if it provides stable interfaces)?
Not to mention to use that we need to add dependency to some sort of RPC
library in our toolstack.
Wei.
> David
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 0/6] xenconsoled: rotating log file abstration
2016-06-07 10:21 ` Wei Liu
@ 2016-06-07 10:29 ` David Vrabel
2016-06-07 10:34 ` Wei Liu
0 siblings, 1 reply; 25+ messages in thread
From: David Vrabel @ 2016-06-07 10:29 UTC (permalink / raw)
To: Wei Liu, David Vrabel; +Cc: Xen-devel, Ian Jackson
On 07/06/16 11:21, Wei Liu wrote:
> On Tue, Jun 07, 2016 at 11:17:07AM +0100, David Vrabel wrote:
>> On 07/06/16 10:55, Wei Liu wrote:
>>> On Tue, Jun 07, 2016 at 10:44:50AM +0100, David Vrabel wrote:
>>>> On 06/06/16 16:59, Wei Liu wrote:
>>>>> Wei Liu (6):
>>>>> xenconsoled: introduce log file abstraction
>>>>> xenconsoled: switch hypervisor log to use logfile abstraction
>>>>> xenconsoled: switch guest log to use logfile abstraction
>>>>> xenconsoled: delete two now unused functions
>>>>> xenconsoled: options to control log rotation
>>>>> xenconsoled: handle --log-backups 0 in logfile_rollover
>>>>
>>>> What not use syslog and/or logrotate?
>>>>
>>>
>>> It's cumbersome to setup per guest facility for syslog. We don't want
>>> console log from different guests mix together in one file.
>>>
>>> Logrorate runs periodically. It doesn't actively limit the disk space
>>> consumed.
>>>
>>> The combination of syslog and logrotate can't prevent a guest from
>>> filling up Dom0 disk as far as I can tell. I'm happy to know if there is
>>> some sort of simple configuration we can ship with open source Xen.
>>
>> Ok. Found the other thread now.
>>
>> We should just use virtlogd instead of re-inventing the wheel.
>>
>
> As in importing that to our own tree (potentially a fork)? Provide
> interfaces to let system administrator configure that (need to at least
> check if it provides stable interfaces)?
Of course not! It would simply be an external dependency.
> Not to mention to use that we need to add dependency to some sort of RPC
> library in our toolstack.
So?
David
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 0/6] xenconsoled: rotating log file abstration
2016-06-07 10:29 ` David Vrabel
@ 2016-06-07 10:34 ` Wei Liu
2016-06-07 10:35 ` David Vrabel
0 siblings, 1 reply; 25+ messages in thread
From: Wei Liu @ 2016-06-07 10:34 UTC (permalink / raw)
To: David Vrabel; +Cc: Xen-devel, Wei Liu, Ian Jackson
On Tue, Jun 07, 2016 at 11:29:31AM +0100, David Vrabel wrote:
> On 07/06/16 11:21, Wei Liu wrote:
> > On Tue, Jun 07, 2016 at 11:17:07AM +0100, David Vrabel wrote:
> >> On 07/06/16 10:55, Wei Liu wrote:
> >>> On Tue, Jun 07, 2016 at 10:44:50AM +0100, David Vrabel wrote:
> >>>> On 06/06/16 16:59, Wei Liu wrote:
> >>>>> Wei Liu (6):
> >>>>> xenconsoled: introduce log file abstraction
> >>>>> xenconsoled: switch hypervisor log to use logfile abstraction
> >>>>> xenconsoled: switch guest log to use logfile abstraction
> >>>>> xenconsoled: delete two now unused functions
> >>>>> xenconsoled: options to control log rotation
> >>>>> xenconsoled: handle --log-backups 0 in logfile_rollover
> >>>>
> >>>> What not use syslog and/or logrotate?
> >>>>
> >>>
> >>> It's cumbersome to setup per guest facility for syslog. We don't want
> >>> console log from different guests mix together in one file.
> >>>
> >>> Logrorate runs periodically. It doesn't actively limit the disk space
> >>> consumed.
> >>>
> >>> The combination of syslog and logrotate can't prevent a guest from
> >>> filling up Dom0 disk as far as I can tell. I'm happy to know if there is
> >>> some sort of simple configuration we can ship with open source Xen.
> >>
> >> Ok. Found the other thread now.
> >>
> >> We should just use virtlogd instead of re-inventing the wheel.
> >>
> >
> > As in importing that to our own tree (potentially a fork)? Provide
> > interfaces to let system administrator configure that (need to at least
> > check if it provides stable interfaces)?
>
> Of course not! It would simply be an external dependency.
>
> > Not to mention to use that we need to add dependency to some sort of RPC
> > library in our toolstack.
>
> So?
>
That's something I would like to avoid if possible. This is just for
guest console logging. It's definitely not a good idea to add that to
xenconsoled.
Please don't mix this thread with the other on-going discussion about
QEMU logging.
Wei.
> David
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 0/6] xenconsoled: rotating log file abstration
2016-06-07 10:34 ` Wei Liu
@ 2016-06-07 10:35 ` David Vrabel
2016-06-07 10:43 ` Wei Liu
0 siblings, 1 reply; 25+ messages in thread
From: David Vrabel @ 2016-06-07 10:35 UTC (permalink / raw)
To: Wei Liu; +Cc: Xen-devel, Ian Jackson
On 07/06/16 11:34, Wei Liu wrote:
> On Tue, Jun 07, 2016 at 11:29:31AM +0100, David Vrabel wrote:
>> On 07/06/16 11:21, Wei Liu wrote:
>>> On Tue, Jun 07, 2016 at 11:17:07AM +0100, David Vrabel wrote:
>>>> On 07/06/16 10:55, Wei Liu wrote:
>>>>> On Tue, Jun 07, 2016 at 10:44:50AM +0100, David Vrabel wrote:
>>>>>> On 06/06/16 16:59, Wei Liu wrote:
>>>>>>> Wei Liu (6):
>>>>>>> xenconsoled: introduce log file abstraction
>>>>>>> xenconsoled: switch hypervisor log to use logfile abstraction
>>>>>>> xenconsoled: switch guest log to use logfile abstraction
>>>>>>> xenconsoled: delete two now unused functions
>>>>>>> xenconsoled: options to control log rotation
>>>>>>> xenconsoled: handle --log-backups 0 in logfile_rollover
>>>>>>
>>>>>> What not use syslog and/or logrotate?
>>>>>>
>>>>>
>>>>> It's cumbersome to setup per guest facility for syslog. We don't want
>>>>> console log from different guests mix together in one file.
>>>>>
>>>>> Logrorate runs periodically. It doesn't actively limit the disk space
>>>>> consumed.
>>>>>
>>>>> The combination of syslog and logrotate can't prevent a guest from
>>>>> filling up Dom0 disk as far as I can tell. I'm happy to know if there is
>>>>> some sort of simple configuration we can ship with open source Xen.
>>>>
>>>> Ok. Found the other thread now.
>>>>
>>>> We should just use virtlogd instead of re-inventing the wheel.
>>>>
>>>
>>> As in importing that to our own tree (potentially a fork)? Provide
>>> interfaces to let system administrator configure that (need to at least
>>> check if it provides stable interfaces)?
>>
>> Of course not! It would simply be an external dependency.
>>
>>> Not to mention to use that we need to add dependency to some sort of RPC
>>> library in our toolstack.
>>
>> So?
>>
>
> That's something I would like to avoid if possible. This is just for
> guest console logging. It's definitely not a good idea to add that to
> xenconsoled.
Why is it a bad idea to make use of an existing solution?
David
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 0/6] xenconsoled: rotating log file abstration
2016-06-07 10:35 ` David Vrabel
@ 2016-06-07 10:43 ` Wei Liu
0 siblings, 0 replies; 25+ messages in thread
From: Wei Liu @ 2016-06-07 10:43 UTC (permalink / raw)
To: David Vrabel; +Cc: Xen-devel, Wei Liu, Ian Jackson
On Tue, Jun 07, 2016 at 11:35:41AM +0100, David Vrabel wrote:
> On 07/06/16 11:34, Wei Liu wrote:
> > On Tue, Jun 07, 2016 at 11:29:31AM +0100, David Vrabel wrote:
> >> On 07/06/16 11:21, Wei Liu wrote:
> >>> On Tue, Jun 07, 2016 at 11:17:07AM +0100, David Vrabel wrote:
> >>>> On 07/06/16 10:55, Wei Liu wrote:
> >>>>> On Tue, Jun 07, 2016 at 10:44:50AM +0100, David Vrabel wrote:
> >>>>>> On 06/06/16 16:59, Wei Liu wrote:
> >>>>>>> Wei Liu (6):
> >>>>>>> xenconsoled: introduce log file abstraction
> >>>>>>> xenconsoled: switch hypervisor log to use logfile abstraction
> >>>>>>> xenconsoled: switch guest log to use logfile abstraction
> >>>>>>> xenconsoled: delete two now unused functions
> >>>>>>> xenconsoled: options to control log rotation
> >>>>>>> xenconsoled: handle --log-backups 0 in logfile_rollover
> >>>>>>
> >>>>>> What not use syslog and/or logrotate?
> >>>>>>
> >>>>>
> >>>>> It's cumbersome to setup per guest facility for syslog. We don't want
> >>>>> console log from different guests mix together in one file.
> >>>>>
> >>>>> Logrorate runs periodically. It doesn't actively limit the disk space
> >>>>> consumed.
> >>>>>
> >>>>> The combination of syslog and logrotate can't prevent a guest from
> >>>>> filling up Dom0 disk as far as I can tell. I'm happy to know if there is
> >>>>> some sort of simple configuration we can ship with open source Xen.
> >>>>
> >>>> Ok. Found the other thread now.
> >>>>
> >>>> We should just use virtlogd instead of re-inventing the wheel.
> >>>>
> >>>
> >>> As in importing that to our own tree (potentially a fork)? Provide
> >>> interfaces to let system administrator configure that (need to at least
> >>> check if it provides stable interfaces)?
> >>
> >> Of course not! It would simply be an external dependency.
> >>
> >>> Not to mention to use that we need to add dependency to some sort of RPC
> >>> library in our toolstack.
> >>
> >> So?
> >>
> >
> > That's something I would like to avoid if possible. This is just for
> > guest console logging. It's definitely not a good idea to add that to
> > xenconsoled.
>
> Why is it a bad idea to make use of an existing solution?
>
The burden of maintaining the interface to an external dependency is
larger then writing our own code in this particular instance.
Wei.
> David
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 0/6] xenconsoled: rotating log file abstration
2016-06-06 15:59 [PATCH 0/6] xenconsoled: rotating log file abstration Wei Liu
` (7 preceding siblings ...)
2016-06-07 9:44 ` David Vrabel
@ 2016-07-02 10:24 ` Wei Liu
2016-07-04 17:07 ` Ian Jackson
8 siblings, 1 reply; 25+ messages in thread
From: Wei Liu @ 2016-07-02 10:24 UTC (permalink / raw)
To: Xen-devel; +Cc: Wei Liu, Ian Jackson
Ian, do you have any opinion on this series?
Despite the pending discussion of how to proceed on the logging issue, I
think this functionality is useful to have in xenconsoled.
Wei.
On Mon, Jun 06, 2016 at 04:59:35PM +0100, Wei Liu wrote:
> Wei Liu (6):
> xenconsoled: introduce log file abstraction
> xenconsoled: switch hypervisor log to use logfile abstraction
> xenconsoled: switch guest log to use logfile abstraction
> xenconsoled: delete two now unused functions
> xenconsoled: options to control log rotation
> xenconsoled: handle --log-backups 0 in logfile_rollover
>
> tools/console/daemon/io.c | 139 +++++++++++------------
> tools/console/daemon/logfile.c | 243 +++++++++++++++++++++++++++++++++++++++++
> tools/console/daemon/logfile.h | 57 ++++++++++
> tools/console/daemon/main.c | 13 ++-
> 4 files changed, 383 insertions(+), 69 deletions(-)
> create mode 100644 tools/console/daemon/logfile.c
> create mode 100644 tools/console/daemon/logfile.h
>
> --
> 2.1.4
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 0/6] xenconsoled: rotating log file abstration
2016-07-02 10:24 ` Wei Liu
@ 2016-07-04 17:07 ` Ian Jackson
0 siblings, 0 replies; 25+ messages in thread
From: Ian Jackson @ 2016-07-04 17:07 UTC (permalink / raw)
To: Wei Liu; +Cc: Xen-devel
Wei Liu writes ("Re: [PATCH 0/6] xenconsoled: rotating log file abstration"):
> Ian, do you have any opinion on this series?
>
> Despite the pending discussion of how to proceed on the logging issue, I
> think this functionality is useful to have in xenconsoled.
I was waiting for a decision on whether to go forward with xenconsoled
for qemu logging. But looking at your summary I see that actually
this is probably useful anyway, so I will review the patches.
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread