On 11/12/2015 12:18 PM, Daniel P. Berrange wrote:
> Add virRotatingFileReader and virRotatingFileWriter objects
> which allow reading & writing from/to files with automation
> rotation to N backup files when a size limit is reached. This
> is useful for guest logging when a guaranteed finite size
> limit is required. Use of external tools like logrotate is
> inadequate since it leaves the possibility for guest to DOS
> the host in between invokations of logrotate.
>
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---
> po/POTFILES.in | 1 +
> src/Makefile.am | 1 +
> src/libvirt_private.syms | 13 +
> src/util/virrotatingfile.c | 608 ++++++++++++++++++++++++++++++++++++++
> src/util/virrotatingfile.h | 62 ++++
> tests/Makefile.am | 6 +
> tests/virrotatingfiletest.c | 698 ++++++++++++++++++++++++++++++++++++++++++++
> 7 files changed, 1389 insertions(+)
> create mode 100644 src/util/virrotatingfile.c
> create mode 100644 src/util/virrotatingfile.h
> create mode 100644 tests/virrotatingfiletest.c
>
[...]
> --- /dev/null
> +++ b/src/util/virrotatingfile.c
> @@ -0,0 +1,608 @@
In the grand scheme of things a nit - lately there seems to be an effort
to use two lines for functions, such as:
[static] int
virFunctionName(param1Type param1,
param2Type param2)
This module has some that are and some that aren't. I don't have heart
break one way or another, but someone may ;-)
> +/*
> + * virrotatingfile.c: file I/O with size rotation
> + *
> + * Copyright (C) 2015 Red Hat, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library 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
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library. If not, see
> + * <http://www.gnu.org/licenses/>.
> + *
> + */
> +
[...]
> +
> +
> +static virRotatingFileReaderEntryPtr
> +virRotatingFileReaderEntryNew(const char *path)
> +{
> + virRotatingFileReaderEntryPtr entry;
> + struct stat sb;
> +
> + VIR_DEBUG("Opening %s", path);
> +
> + if (VIR_ALLOC(entry) < 0)
> + return NULL;
> +
> + if (VIR_STRDUP(entry->path, path) < 0)
> + goto error;
Should the open occur first; otherwise, entry->fd == 0 and the *Free
routine will VIR_FORCE_CLOSE(0);
> +
> + if ((entry->fd = open(path, O_RDONLY)) < 0) {
> + if (errno != ENOENT) {
> + virReportSystemError(errno,
> + _("Unable to open file: %s"), path);
> + goto error;
> + }
> + }
> +
> + if (entry->fd != -1) {
> + if (fstat(entry->fd, &sb) < 0) {
> + virReportSystemError(errno,
> + _("Unable to determine current file inode: %s"),
> + path);
> + goto error;
> + }
> +
> + entry->inode = sb.st_ino;
> + }
> +
> + return entry;
> +
> + error:
> + virRotatingFileReaderEntryFree(entry);
> + return NULL;
> +}
> +
[...]
> +/**
> + * virRotatingFileWriterNew
> + * @path: the base path for files
> + * @maxlen: the maximum number of bytes to write before rollover
> + * @maxbackup: number of backup files to keep when rolloing over
rolling over
Although I don't see it being documented/used later in/for some config
file (and the max value I see used in code is 2)... Should there be some
sort of "sanity check" that maxbackup doesn't exceed some number - only
so many files can be open per process, right? Then of course there's
the silly test of someone passing -1...
Also, as a config value it seems if someone changes the maxbackup value
(higher to lower), then some algorithms may miss files... If then going
from lower to higher, then files that may not have been deleted might be
found.
ACK -
John
> + * @truncate: whether to truncate the current files when opening
> + * @mode: the file mode to use for creating new files
> + *
> + * Create a new object for writing data to a file with
> + * automatic rollover. If @maxbackup is zero, no backup
> + * files will be created. The primary file will just get
> + * truncated and reopened.
> + *
> + * The files will never exceed @maxlen bytes in size,
> + * but may be rolled over before they reach this size
> + * in order to avoid splitting lines
> + */
> +virRotatingFileWriterPtr virRotatingFileWriterNew(const char *path,
> + off_t maxlen,
> + size_t maxbackup,
> + bool truncate,
> + mode_t mode)
> +{
> + virRotatingFileWriterPtr file;
> +
> + if (VIR_ALLOC(file) < 0)
> + goto error;
> +
> + if (VIR_STRDUP(file->basepath, path) < 0)
> + goto error;
> +
> + file->mode = mode;
> + file->maxbackup = maxbackup;
> + file->maxlen = maxlen;
> +
> + if (truncate &&
> + virRotatingFileWriterDelete(file) < 0)
> + goto error;
> +
> + if (!(file->entry = virRotatingFileWriterEntryNew(file->basepath,
> + mode)))
> + goto error;
> +
> + return file;
> +
> + error:
> + virRotatingFileWriterFree(file);
> + return NULL;
> +}
> +
> +
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list