[libvirt] [PATCH 1/5] util: add API for writing to rotating files

[Daniel P. Berrange]

On Wed, Nov 04, 2015 at 07:56:37AM +0100, Peter Krempa wrote:
> On Tue, Nov 03, 2015 at 16:04:20 +0000, Daniel Berrange wrote:
> > Add a virRotatingFile object which allows writing to a file
> > 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 at redhat.com>

> > +    if (VIR_ALLOC(file) < 0)
> > +        goto error;
> > +
> > +    if (VIR_STRDUP(file->path, path) < 0)
> > +        goto error;
> > +
> > +    file->mode = mode;
> > +    file->maxbackup = maxbackup;
> 
> If maxbackup is set to 0 the rollover code below will become a no-op,
> wich is fine, but in virRotatingFileWrite you close the fd, open the
> same file in apend mode seek to the end and find out that is already
> full.

Hmm, yeah, good point. I had thought about requiring maxbackup > 0
but I guess it is nicer to just fix that edge case.


> > +
> > +ssize_t virRotatingFileWrite(virRotatingFilePtr file,
> > +                             const char *buf,
> > +                             size_t len)
> > +{
> > +    ssize_t ret = 0;
> > +    while (len) {
> > +        size_t towrite = len;
> > +
> > +        if ((file->curlen + towrite) > file->maxlen)
> > +            towrite = file->maxlen - file->curlen;
> 
> So the boundary on which the file will be broken is strictly decided on
> the number of bytes. Generaly this is fine but for logging it might
> break messages in half which isn't entirely nice.
> 
> I'd suggest that either this function (or via a flag or a different
> function) will have logic that will peek into the last eg. 100
> characters (perhaps from the end) of the logged string to see whether
> there's a newline and if there is, it will rollover the log files
> earlier.
> 
> This will slightly decrease the logging capacity but in turn we'll get
> nicely broken error messages.

Yeah, I think I'll just add logic that always looks for presence of
a \n in the last 128 bytes before the size limit, and rollover early
on that if found.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|