[libvirt] [PATCH] util: Don't delete the original file for truncation

[Marc Hartmayer]

Truncate means that if a file exists it's length will be truncated to
0, but the mode and the owner shall be unchanged. The current behavior
is that the original file is deleted and a new file is created. Let's
fix this by using O_TRUNC.

The function virRotatingFileWriterDelete is now unused but may be used
in the future and is therefore still defined.

Signed-off-by: Marc Hartmayer <mhartmay at linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy at linux.ibm.com>
---
Note:

This change has the (potentially unwanted) security effect that the
owner/group of the log file does not change. Before this patch the old
log file was deleted and the newly created log file was owned by the
virtlogd user. Now, if a user has created the log file before, he can
read the logs. If we don't wanna have this effect we can either
adjust/add a virtlogd API or do a chown within the calling driver
(e.g. QEMU driver).

Side note: the original behavior has changed with the patch series
"qemu: use FD passing for chardev UNIX sockets".

Example where it the behavior has changed:

 <console type='file'>
      <source path='/tmp/console.log'/>
      <target type='serial'/>
 </console>
---
 src/util/virrotatingfile.c | 49 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 34 insertions(+), 15 deletions(-)

diff --git a/src/util/virrotatingfile.c b/src/util/virrotatingfile.c
index ca62a8e02641..d6ab3780aae3 100644
--- a/src/util/virrotatingfile.c
+++ b/src/util/virrotatingfile.c
@@ -98,17 +98,25 @@ virRotatingFileReaderEntryFree(virRotatingFileReaderEntryPtr entry)
 
 static virRotatingFileWriterEntryPtr
 virRotatingFileWriterEntryNew(const char *path,
-                              mode_t mode)
+                              mode_t mode,
+                              bool truncate)
 {
     virRotatingFileWriterEntryPtr entry;
     struct stat sb;
+    /* O_APPEND is also useful in combination with O_TRUNC since it
+     * guarantees the atomicity of a write operation (at least for
+     * POSIX systems) */
+    int oflag = O_CREAT|O_APPEND|O_WRONLY|O_CLOEXEC;
 
     VIR_DEBUG("Opening %s mode=0%02o", path, mode);
 
     if (VIR_ALLOC(entry) < 0)
         return NULL;
 
-    if ((entry->fd = open(path, O_CREAT|O_APPEND|O_WRONLY|O_CLOEXEC, mode)) < 0) {
+    if (truncate)
+        oflag |= O_TRUNC;
+
+    if ((entry->fd = open(path, oflag, mode)) < 0) {
         virReportSystemError(errno,
                              _("Unable to open file: %s"), path);
         goto error;
@@ -182,18 +190,10 @@ virRotatingFileReaderEntryNew(const char *path)
 
 
 static int
-virRotatingFileWriterDelete(virRotatingFileWriterPtr file)
+virRotatingFileWriterDeleteBackup(virRotatingFileWriterPtr file)
 {
     size_t i;
 
-    if (unlink(file->basepath) < 0 &&
-        errno != ENOENT) {
-        virReportSystemError(errno,
-                             _("Unable to delete file %s"),
-                             file->basepath);
-        return -1;
-    }
-
     for (i = 0; i < file->maxbackup; i++) {
         char *oldpath;
         if (virAsprintf(&oldpath, "%s.%zu", file->basepath, i) < 0)
@@ -214,6 +214,24 @@ virRotatingFileWriterDelete(virRotatingFileWriterPtr file)
 }
 
 
+static int ATTRIBUTE_UNUSED
+virRotatingFileWriterDelete(virRotatingFileWriterPtr file)
+{
+    if (unlink(file->basepath) < 0 &&
+        errno != ENOENT) {
+        virReportSystemError(errno,
+                             _("Unable to delete file %s"),
+                             file->basepath);
+        return -1;
+    }
+
+    if (virRotatingFileWriterDeleteBackup(file) < 0)
+        return -1;
+
+    return 0;
+}
+
+
 /**
  * virRotatingFileWriterNew
  * @path: the base path for files
@@ -257,12 +275,12 @@ virRotatingFileWriterNew(const char *path,
     file->maxbackup = maxbackup;
     file->maxlen = maxlen;
 
-    if (trunc &&
-        virRotatingFileWriterDelete(file) < 0)
+    if (trunc && virRotatingFileWriterDeleteBackup(file) < 0)
         goto error;
 
     if (!(file->entry = virRotatingFileWriterEntryNew(file->basepath,
-                                                      mode)))
+                                                      mode,
+                                                      trunc)))
         goto error;
 
     return file;
@@ -491,7 +509,8 @@ virRotatingFileWriterAppend(virRotatingFileWriterPtr file,
                 return -1;
 
             if (!(tmp = virRotatingFileWriterEntryNew(file->basepath,
-                                                      file->mode)))
+                                                      file->mode,
+                                                      false)))
                 return -1;
 
             virRotatingFileWriterEntryFree(file->entry);
-- 
2.13.4