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

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(a)redhat.com&gt;

> + 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.

> + > +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.