You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by jp...@apache.org on 2014/01/08 18:45:49 UTC

git commit: TS-2471: records.config becomes empty when the disk full

Updated Branches:
  refs/heads/master c78f95423 -> 7fd40eba2


TS-2471: records.config becomes empty when the disk full

In lib/records/RecLocal.cc and lib/records/RecProcess.cc, records.config
may become empty when the disk is full because we write records.config
directly. We should write to a temporary file, then rename it to
records.config.


Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/7fd40eba
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/7fd40eba
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/7fd40eba

Branch: refs/heads/master
Commit: 7fd40eba2157a3d709a9f254cad156ab44fbb465
Parents: c78f954
Author: Yu Qing <zh...@taobao.com>
Authored: Tue Jan 7 18:35:54 2014 +0800
Committer: James Peach <jp...@apache.org>
Committed: Wed Jan 8 09:44:46 2014 -0800

----------------------------------------------------------------------
 CHANGES                   |  3 ++
 lib/records/P_RecCore.cc  | 79 ++++++++++++++++++++++++++++++++++++++++++
 lib/records/P_RecCore.h   |  1 +
 lib/records/P_RecFile.h   |  3 +-
 lib/records/RecFile.cc    | 13 +++++--
 lib/records/RecLocal.cc   | 12 ++-----
 lib/records/RecMessage.cc |  1 +
 lib/records/RecProcess.cc |  6 +---
 mgmt/Rollback.cc          | 43 +++++++++++++----------
 mgmt/Rollback.h           |  2 +-
 10 files changed, 125 insertions(+), 38 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7fd40eba/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index 3f6442d..2835c24 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,9 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache Traffic Server 4.2.0
 
+  *) [TS-2471] Writing records.config can fail when the disk is full.
+   Author: Yu Qing <zh...@taobao.com>
+
   *) [TS-2479] Don't output orphan log after failover sucessfully.
 
   *) [TS-2370] SSL proxy.config.ssl.server.honor_cipher_order is backwards.

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7fd40eba/lib/records/P_RecCore.cc
----------------------------------------------------------------------
diff --git a/lib/records/P_RecCore.cc b/lib/records/P_RecCore.cc
index 6f02a20..f176192 100644
--- a/lib/records/P_RecCore.cc
+++ b/lib/records/P_RecCore.cc
@@ -925,3 +925,82 @@ RecSetSyncRequired(char *name, bool lock)
 
   return err;
 }
+
+int RecWriteConfigFile(textBuffer *tb)
+{
+#define TMP_FILENAME_EXT_STR ".tmp"
+#define TMP_FILENAME_EXT_LEN (sizeof(TMP_FILENAME_EXT_STR) - 1)
+
+  int nbytes;
+  int filename_len;
+  int tmp_filename_len;
+  int result;
+  char buff[1024];
+  char *tmp_filename;
+
+  filename_len = strlen(g_rec_config_fpath);
+  tmp_filename_len = filename_len + TMP_FILENAME_EXT_LEN;
+  if (tmp_filename_len < (int)sizeof(buff)) {
+    tmp_filename = buff;
+  } else {
+    tmp_filename = (char *)ats_malloc(tmp_filename_len + 1);
+  }
+  sprintf(tmp_filename, "%s%s", g_rec_config_fpath, TMP_FILENAME_EXT_STR);
+
+  RecDebug(DL_Note, "Writing '%s'", g_rec_config_fpath);
+
+  RecHandle h_file = RecFileOpenW(tmp_filename);
+  do {
+    if (h_file == REC_HANDLE_INVALID) {
+      RecLog(DL_Warning, "open file: %s to write fail, errno: %d, error info: %s",
+          tmp_filename, errno, strerror(errno));
+      result = REC_ERR_FAIL;
+      break;
+    }
+
+    if (RecFileWrite(h_file, tb->bufPtr(), tb->spaceUsed(), &nbytes) != REC_ERR_OKAY) {
+      RecLog(DL_Warning, "write to file: %s fail, errno: %d, error info: %s",
+          tmp_filename, errno, strerror(errno));
+      result = REC_ERR_FAIL;
+      break;
+    }
+
+    if (nbytes != tb->spaceUsed()) {
+      RecLog(DL_Warning, "write to file: %s fail, disk maybe full", tmp_filename);
+      result = REC_ERR_FAIL;
+      break;
+    }
+
+    if (RecFileSync(h_file) != REC_ERR_OKAY) {
+      RecLog(DL_Warning, "fsync file: %s fail, errno: %d, error info: %s",
+          tmp_filename, errno, strerror(errno));
+      result = REC_ERR_FAIL;
+      break;
+    }
+    if (RecFileClose(h_file) != REC_ERR_OKAY) {
+      RecLog(DL_Warning, "close file: %s fail, errno: %d, error info: %s",
+          tmp_filename, errno, strerror(errno));
+      result = REC_ERR_FAIL;
+      break;
+    }
+    h_file = REC_HANDLE_INVALID;
+
+    if (rename(tmp_filename, g_rec_config_fpath) != 0) {
+      RecLog(DL_Warning, "rename file %s to %s fail, errno: %d, error info: %s",
+          tmp_filename, g_rec_config_fpath, errno, strerror(errno));
+      result = REC_ERR_FAIL;
+      break;
+    }
+
+    result = REC_ERR_OKAY;
+  } while (0);
+
+  if (h_file != REC_HANDLE_INVALID) {
+    RecFileClose(h_file);
+  }
+  if (tmp_filename != buff) {
+    ats_free(tmp_filename);
+  }
+  return result;
+}
+

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7fd40eba/lib/records/P_RecCore.h
----------------------------------------------------------------------
diff --git a/lib/records/P_RecCore.h b/lib/records/P_RecCore.h
index 375f526..f0335b9 100644
--- a/lib/records/P_RecCore.h
+++ b/lib/records/P_RecCore.h
@@ -87,6 +87,7 @@ int RecGetRecord_Xmalloc(const char *name, RecDataT data_type, RecData * data, b
 int RecReadStatsFile();
 int RecSyncStatsFile();
 int RecReadConfigFile(bool inc_version);
+int RecWriteConfigFile(textBuffer *tb);
 int RecSyncConfigToTB(textBuffer * tb, bool *inc_version = NULL);
 
 //-------------------------------------------------------------------------

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7fd40eba/lib/records/P_RecFile.h
----------------------------------------------------------------------
diff --git a/lib/records/P_RecFile.h b/lib/records/P_RecFile.h
index 48ce70d..25b0a49 100644
--- a/lib/records/P_RecFile.h
+++ b/lib/records/P_RecFile.h
@@ -37,11 +37,12 @@ typedef int RecHandle;
 
 RecHandle RecFileOpenR(const char *file);
 RecHandle RecFileOpenW(const char *file);
-void RecFileClose(RecHandle h_file);
+int RecFileClose(RecHandle h_file);
 int RecFileRead(RecHandle h_file, char *buf, int size, int *bytes_read);
 int RecFileWrite(RecHandle h_file, char *buf, int size, int *bytes_written);
 int RecFileGetSize(RecHandle h_file);
 int RecFileExists(const char *file);
+int RecFileSync(RecHandle h_file);
 
 //-------------------------------------------------------------------------
 // RecPipe

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7fd40eba/lib/records/RecFile.cc
----------------------------------------------------------------------
diff --git a/lib/records/RecFile.cc b/lib/records/RecFile.cc
index 2a0b52f..1bb9750 100644
--- a/lib/records/RecFile.cc
+++ b/lib/records/RecFile.cc
@@ -54,13 +54,22 @@ RecFileOpenW(const char *file)
 }
 
 //-------------------------------------------------------------------------
+// RecFileSync
+//-------------------------------------------------------------------------
+
+int RecFileSync(RecHandle h_file)
+{
+  return (fsync(h_file) == 0) ? REC_ERR_OKAY : REC_ERR_FAIL;
+}
+
+//-------------------------------------------------------------------------
 // RecFileClose
 //-------------------------------------------------------------------------
 
-void
+int
 RecFileClose(RecHandle h_file)
 {
-  close(h_file);
+  return (close(h_file) == 0) ? REC_ERR_OKAY : REC_ERR_FAIL;
 }
 
 //-------------------------------------------------------------------------

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7fd40eba/lib/records/RecLocal.cc
----------------------------------------------------------------------
diff --git a/lib/records/RecLocal.cc b/lib/records/RecLocal.cc
index 9502bb9..87622f4 100644
--- a/lib/records/RecLocal.cc
+++ b/lib/records/RecLocal.cc
@@ -72,11 +72,7 @@ sync_thr(void *data)
     send_push_message();
     RecSyncStatsFile();
     if (RecSyncConfigToTB(&tb) == REC_ERR_OKAY) {
-      int nbytes;
-      RecDebug(DL_Note, "Writing '%s'", g_rec_config_fpath);
-      RecHandle h_file = RecFileOpenW(g_rec_config_fpath);
-      RecFileWrite(h_file, tb.bufPtr(), tb.spaceUsed(), &nbytes);
-      RecFileClose(h_file);
+      RecWriteConfigFile(&tb);
     }
     usleep(REC_REMOTE_SYNC_INTERVAL_MS * 1000);
   }
@@ -124,11 +120,7 @@ sync_thr(void *data)
         rb = NULL;
       }
       if (!written) {
-        int nbytes;
-        RecDebug(DL_Note, "Writing '%s'", g_rec_config_fpath);
-        RecHandle h_file = RecFileOpenW(g_rec_config_fpath);
-        RecFileWrite(h_file, tb->bufPtr(), tb->spaceUsed(), &nbytes);
-        RecFileClose(h_file);
+        RecWriteConfigFile(tb);
         if (rb != NULL) {
           rb->setLastModifiedTime();
         }

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7fd40eba/lib/records/RecMessage.cc
----------------------------------------------------------------------
diff --git a/lib/records/RecMessage.cc b/lib/records/RecMessage.cc
index bec22be..fd25d4d 100644
--- a/lib/records/RecMessage.cc
+++ b/lib/records/RecMessage.cc
@@ -552,6 +552,7 @@ RecMessageWriteToDisk(RecMessage *msg, const char *fpath)
   msg_size = sizeof(RecMessageHdr) + (msg->o_write - msg->o_start);
   if ((h_file = RecFileOpenW(fpath)) != REC_HANDLE_INVALID) {
     if (RecFileWrite(h_file, (char *) msg, msg_size, &bytes_written) == REC_ERR_FAIL) {
+      RecFileClose(h_file);
       return REC_ERR_FAIL;
     }
     RecFileClose(h_file);

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7fd40eba/lib/records/RecProcess.cc
----------------------------------------------------------------------
diff --git a/lib/records/RecProcess.cc b/lib/records/RecProcess.cc
index 75b5829..5dc7392 100644
--- a/lib/records/RecProcess.cc
+++ b/lib/records/RecProcess.cc
@@ -392,11 +392,7 @@ struct sync_cont: public Continuation
     send_push_message();
     RecSyncStatsFile();
     if (RecSyncConfigToTB(m_tb) == REC_ERR_OKAY) {
-      int nbytes;
-      RecDebug(DL_Note, "Writing '%s'", g_rec_config_fpath);
-      RecHandle h_file = RecFileOpenW(g_rec_config_fpath);
-      RecFileWrite(h_file, m_tb->bufPtr(), m_tb->spaceUsed(), &nbytes);
-      RecFileClose(h_file);
+        RecWriteConfigFile(m_tb);
     }
     Debug("statsproc", "sync_cont() processed");
 

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7fd40eba/mgmt/Rollback.cc
----------------------------------------------------------------------
diff --git a/mgmt/Rollback.cc b/mgmt/Rollback.cc
index 6c219e3..4489a18 100644
--- a/mgmt/Rollback.cc
+++ b/mgmt/Rollback.cc
@@ -140,7 +140,7 @@ Rollback::Rollback(const char *baseFileName, bool root_access_needed_)
           mgmt_log(stderr, "[RollBack::Rollback] %s\n", alarmMsg);
           lmgmt->alarm_keeper->signalAlarm(MGMT_ALARM_CONFIG_UPDATE_FAILED, alarmMsg);
           ats_free(alarmMsg);
-          closeFile(fd);
+          closeFile(fd, true);
         } else {
           mgmt_fatal(stderr, 0,
                      "[RollBack::Rollback] Unable to find configuration file %s.\n\tCreation of a placeholder failed : %s\n",
@@ -201,11 +201,11 @@ Rollback::Rollback(const char *baseFileName, bool root_access_needed_)
       snprintf(alarmMsg, 2048, "Config file is read-only");
       mgmt_log(stderr, "[Rollback::Rollback] %s : %s\n", alarmMsg, fileName);
       lmgmt->alarm_keeper->signalAlarm(MGMT_ALARM_CONFIG_UPDATE_FAILED, alarmMsg);
-      closeFile(testFD);
+      closeFile(testFD, false);
     }
     ats_free(alarmMsg);
   } else {
-    closeFile(testFD);
+    closeFile(testFD, true);
   }
 }
 
@@ -338,8 +338,9 @@ Rollback::openFile(version_t version, int oflags, int *errnoPtr)
     }
     mgmt_log(stderr, "[Rollback::openFile] Open of %s failed: %s\n", fileName, strerror(errno));
   }
-
-  fcntl(fd, F_SETFD, 1);
+  else {
+    fcntl(fd, F_SETFD, 1);
+  }
 
   delete[]filePath;
 
@@ -347,18 +348,21 @@ Rollback::openFile(version_t version, int oflags, int *errnoPtr)
 }
 
 int
-Rollback::closeFile(int fd)
+Rollback::closeFile(int fd, bool callSync)
 {
-  if (fsync(fd) == -1) {
-    // INKqa11574: SGI fsync will return EBADF error if the file was
-    // open w/o write access (e.g. read-only).  Do not print an error
-    // if we get this errno back -- just in case this fd was open
-    // O_RDONLY.
-    if (errno != EBADF) {
-      mgmt_log(stderr, "[Rollback::closeFile] fsync failed for file '%s' (%d: %s)\n", fileName, errno, strerror(errno));
-    }
+  int result = 0;
+  if (callSync && fsync(fd) < 0) {
+    result = -1;
+    mgmt_log(stderr, "[Rollback::closeFile] fsync failed for file '%s' (%d: %s)\n", fileName, errno, strerror(errno));
   }
-  return (close(fd));
+
+  if (result == 0) {
+    result = close(fd);
+  }
+  else {
+    close(fd);
+  }
+  return result;
 }
 
 
@@ -422,6 +426,7 @@ Rollback::internalUpdate(textBuffer * buf, version_t newVersion, bool notifyChan
   char *nextVersion;
   int writeBytes;
   int diskFD;
+  int ret;
   versionInfo *toRemove;
   versionInfo *newBak;
   bool failedLink = false;
@@ -465,9 +470,9 @@ Rollback::internalUpdate(textBuffer * buf, version_t newVersion, bool notifyChan
   }
   // Write the buffer into the new configuration file
   writeBytes = write(diskFD, buf->bufPtr(), buf->spaceUsed());
-  closeFile(diskFD);
-  if (writeBytes != buf->spaceUsed()) {
-    mgmt_log(stderr, "[Rollback::intrernalUpdate] Unable to write new version of %s : %s\n", fileName, strerror(errno));
+  ret = closeFile(diskFD, true);
+  if ((ret < 0) || (writeBytes != buf->spaceUsed())) {
+    mgmt_log(stderr, "[Rollback::internalUpdate] Unable to write new version of %s : %s\n", fileName, strerror(errno));
     returnCode = SYS_CALL_ERROR_ROLLBACK;
     goto UPDATE_CLEANUP;
   }
@@ -632,7 +637,7 @@ Rollback::getVersion_ml(version_t version, textBuffer ** buffer)
 GET_CLEANUP:
 
   if (diskFD >= 0) {
-    closeFile(diskFD);
+    closeFile(diskFD, false);
   }
 
   return returnCode;

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7fd40eba/mgmt/Rollback.h
----------------------------------------------------------------------
diff --git a/mgmt/Rollback.h b/mgmt/Rollback.h
index e3025a6..7b877a0 100644
--- a/mgmt/Rollback.h
+++ b/mgmt/Rollback.h
@@ -203,7 +203,7 @@ public:
 private:
   Rollback(const Rollback &);
   int openFile(version_t version, int oflags, int *errnoPtr = NULL);
-  int closeFile(int fd);
+  int closeFile(int fd, bool callSync);
   int statFile(version_t version, struct stat *buf);
   char *createPathStr(version_t version);
   RollBackCodes internalUpdate(textBuffer * buf, version_t newVersion, bool notifyChange = true, bool incVersion = true);


Re: git commit: TS-2471: records.config becomes empty when the disk full

Posted by Igor Galić <i....@brainsware.org>.

----- Original Message -----
> Updated Branches:
>   refs/heads/master c78f95423 -> 7fd40eba2
> 
> 
> TS-2471: records.config becomes empty when the disk full
> 
> In lib/records/RecLocal.cc and lib/records/RecProcess.cc, records.config
> may become empty when the disk is full because we write records.config
> directly. We should write to a temporary file, then rename it to
> records.config.
> 
> 
> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
> Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/7fd40eba
> Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/7fd40eba
> Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/7fd40eba
> 
> Branch: refs/heads/master
> Commit: 7fd40eba2157a3d709a9f254cad156ab44fbb465
> Parents: c78f954
> Author: Yu Qing <zh...@taobao.com>
> Authored: Tue Jan 7 18:35:54 2014 +0800
> Committer: James Peach <jp...@apache.org>
> Committed: Wed Jan 8 09:44:46 2014 -0800
> 
> ----------------------------------------------------------------------
>  CHANGES                   |  3 ++
>  lib/records/P_RecCore.cc  | 79 ++++++++++++++++++++++++++++++++++++++++++
>  lib/records/P_RecCore.h   |  1 +
>  lib/records/P_RecFile.h   |  3 +-
>  lib/records/RecFile.cc    | 13 +++++--
>  lib/records/RecLocal.cc   | 12 ++-----
>  lib/records/RecMessage.cc |  1 +
>  lib/records/RecProcess.cc |  6 +---
>  mgmt/Rollback.cc          | 43 +++++++++++++----------
>  mgmt/Rollback.h           |  2 +-
>  10 files changed, 125 insertions(+), 38 deletions(-)
> ----------------------------------------------------------------------
> 
> 
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7fd40eba/CHANGES
> ----------------------------------------------------------------------
> diff --git a/CHANGES b/CHANGES
> index 3f6442d..2835c24 100644
> --- a/CHANGES
> +++ b/CHANGES
> @@ -1,6 +1,9 @@
>                                                           -*- coding: utf-8
>                                                           -*-
>  Changes with Apache Traffic Server 4.2.0
>  
> +  *) [TS-2471] Writing records.config can fail when the disk is full.
> +   Author: Yu Qing <zh...@taobao.com>
> +
>    *) [TS-2479] Don't output orphan log after failover sucessfully.
>  
>    *) [TS-2370] SSL proxy.config.ssl.server.honor_cipher_order is backwards.
> 
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7fd40eba/lib/records/P_RecCore.cc
> ----------------------------------------------------------------------
> diff --git a/lib/records/P_RecCore.cc b/lib/records/P_RecCore.cc
> index 6f02a20..f176192 100644
> --- a/lib/records/P_RecCore.cc
> +++ b/lib/records/P_RecCore.cc
> @@ -925,3 +925,82 @@ RecSetSyncRequired(char *name, bool lock)
>  
>    return err;
>  }
> +
> +int RecWriteConfigFile(textBuffer *tb)
> +{
> +#define TMP_FILENAME_EXT_STR ".tmp"
> +#define TMP_FILENAME_EXT_LEN (sizeof(TMP_FILENAME_EXT_STR) - 1)

Why not use mkstemp(3) here?

> +
> +  int nbytes;
> +  int filename_len;
> +  int tmp_filename_len;
> +  int result;
> +  char buff[1024];
> +  char *tmp_filename;
> +
> +  filename_len = strlen(g_rec_config_fpath);
> +  tmp_filename_len = filename_len + TMP_FILENAME_EXT_LEN;
> +  if (tmp_filename_len < (int)sizeof(buff)) {
> +    tmp_filename = buff;
> +  } else {
> +    tmp_filename = (char *)ats_malloc(tmp_filename_len + 1);
> +  }
> +  sprintf(tmp_filename, "%s%s", g_rec_config_fpath, TMP_FILENAME_EXT_STR);
> +
> +  RecDebug(DL_Note, "Writing '%s'", g_rec_config_fpath);
> +
> +  RecHandle h_file = RecFileOpenW(tmp_filename);
> +  do {
> +    if (h_file == REC_HANDLE_INVALID) {
> +      RecLog(DL_Warning, "open file: %s to write fail, errno: %d, error
> info: %s",
> +          tmp_filename, errno, strerror(errno));
> +      result = REC_ERR_FAIL;
> +      break;
> +    }
> +
> +    if (RecFileWrite(h_file, tb->bufPtr(), tb->spaceUsed(), &nbytes) !=
> REC_ERR_OKAY) {
> +      RecLog(DL_Warning, "write to file: %s fail, errno: %d, error info:
> %s",
> +          tmp_filename, errno, strerror(errno));
> +      result = REC_ERR_FAIL;
> +      break;
> +    }
> +
> +    if (nbytes != tb->spaceUsed()) {
> +      RecLog(DL_Warning, "write to file: %s fail, disk maybe full",
> tmp_filename);
> +      result = REC_ERR_FAIL;
> +      break;
> +    }
> +
> +    if (RecFileSync(h_file) != REC_ERR_OKAY) {
> +      RecLog(DL_Warning, "fsync file: %s fail, errno: %d, error info: %s",
> +          tmp_filename, errno, strerror(errno));
> +      result = REC_ERR_FAIL;
> +      break;
> +    }
> +    if (RecFileClose(h_file) != REC_ERR_OKAY) {
> +      RecLog(DL_Warning, "close file: %s fail, errno: %d, error info: %s",
> +          tmp_filename, errno, strerror(errno));
> +      result = REC_ERR_FAIL;
> +      break;
> +    }
> +    h_file = REC_HANDLE_INVALID;
> +
> +    if (rename(tmp_filename, g_rec_config_fpath) != 0) {
> +      RecLog(DL_Warning, "rename file %s to %s fail, errno: %d, error info:
> %s",
> +          tmp_filename, g_rec_config_fpath, errno, strerror(errno));
> +      result = REC_ERR_FAIL;
> +      break;
> +    }
> +
> +    result = REC_ERR_OKAY;
> +  } while (0);
> +
> +  if (h_file != REC_HANDLE_INVALID) {
> +    RecFileClose(h_file);
> +  }
> +  if (tmp_filename != buff) {
> +    ats_free(tmp_filename);
> +  }
> +  return result;
> +}
> +
> 
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7fd40eba/lib/records/P_RecCore.h
> ----------------------------------------------------------------------
> diff --git a/lib/records/P_RecCore.h b/lib/records/P_RecCore.h
> index 375f526..f0335b9 100644
> --- a/lib/records/P_RecCore.h
> +++ b/lib/records/P_RecCore.h
> @@ -87,6 +87,7 @@ int RecGetRecord_Xmalloc(const char *name, RecDataT
> data_type, RecData * data, b
>  int RecReadStatsFile();
>  int RecSyncStatsFile();
>  int RecReadConfigFile(bool inc_version);
> +int RecWriteConfigFile(textBuffer *tb);
>  int RecSyncConfigToTB(textBuffer * tb, bool *inc_version = NULL);
>  
>  //-------------------------------------------------------------------------
> 
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7fd40eba/lib/records/P_RecFile.h
> ----------------------------------------------------------------------
> diff --git a/lib/records/P_RecFile.h b/lib/records/P_RecFile.h
> index 48ce70d..25b0a49 100644
> --- a/lib/records/P_RecFile.h
> +++ b/lib/records/P_RecFile.h
> @@ -37,11 +37,12 @@ typedef int RecHandle;
>  
>  RecHandle RecFileOpenR(const char *file);
>  RecHandle RecFileOpenW(const char *file);
> -void RecFileClose(RecHandle h_file);
> +int RecFileClose(RecHandle h_file);
>  int RecFileRead(RecHandle h_file, char *buf, int size, int *bytes_read);
>  int RecFileWrite(RecHandle h_file, char *buf, int size, int *bytes_written);
>  int RecFileGetSize(RecHandle h_file);
>  int RecFileExists(const char *file);
> +int RecFileSync(RecHandle h_file);
>  
>  //-------------------------------------------------------------------------
>  // RecPipe
> 
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7fd40eba/lib/records/RecFile.cc
> ----------------------------------------------------------------------
> diff --git a/lib/records/RecFile.cc b/lib/records/RecFile.cc
> index 2a0b52f..1bb9750 100644
> --- a/lib/records/RecFile.cc
> +++ b/lib/records/RecFile.cc
> @@ -54,13 +54,22 @@ RecFileOpenW(const char *file)
>  }
>  
>  //-------------------------------------------------------------------------
> +// RecFileSync
> +//-------------------------------------------------------------------------
> +
> +int RecFileSync(RecHandle h_file)
> +{
> +  return (fsync(h_file) == 0) ? REC_ERR_OKAY : REC_ERR_FAIL;
> +}
> +
> +//-------------------------------------------------------------------------
>  // RecFileClose
>  //-------------------------------------------------------------------------
>  
> -void
> +int
>  RecFileClose(RecHandle h_file)
>  {
> -  close(h_file);
> +  return (close(h_file) == 0) ? REC_ERR_OKAY : REC_ERR_FAIL;
>  }
>  
>  //-------------------------------------------------------------------------
> 
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7fd40eba/lib/records/RecLocal.cc
> ----------------------------------------------------------------------
> diff --git a/lib/records/RecLocal.cc b/lib/records/RecLocal.cc
> index 9502bb9..87622f4 100644
> --- a/lib/records/RecLocal.cc
> +++ b/lib/records/RecLocal.cc
> @@ -72,11 +72,7 @@ sync_thr(void *data)
>      send_push_message();
>      RecSyncStatsFile();
>      if (RecSyncConfigToTB(&tb) == REC_ERR_OKAY) {
> -      int nbytes;
> -      RecDebug(DL_Note, "Writing '%s'", g_rec_config_fpath);
> -      RecHandle h_file = RecFileOpenW(g_rec_config_fpath);
> -      RecFileWrite(h_file, tb.bufPtr(), tb.spaceUsed(), &nbytes);
> -      RecFileClose(h_file);
> +      RecWriteConfigFile(&tb);
>      }
>      usleep(REC_REMOTE_SYNC_INTERVAL_MS * 1000);
>    }
> @@ -124,11 +120,7 @@ sync_thr(void *data)
>          rb = NULL;
>        }
>        if (!written) {
> -        int nbytes;
> -        RecDebug(DL_Note, "Writing '%s'", g_rec_config_fpath);
> -        RecHandle h_file = RecFileOpenW(g_rec_config_fpath);
> -        RecFileWrite(h_file, tb->bufPtr(), tb->spaceUsed(), &nbytes);
> -        RecFileClose(h_file);
> +        RecWriteConfigFile(tb);
>          if (rb != NULL) {
>            rb->setLastModifiedTime();
>          }
> 
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7fd40eba/lib/records/RecMessage.cc
> ----------------------------------------------------------------------
> diff --git a/lib/records/RecMessage.cc b/lib/records/RecMessage.cc
> index bec22be..fd25d4d 100644
> --- a/lib/records/RecMessage.cc
> +++ b/lib/records/RecMessage.cc
> @@ -552,6 +552,7 @@ RecMessageWriteToDisk(RecMessage *msg, const char *fpath)
>    msg_size = sizeof(RecMessageHdr) + (msg->o_write - msg->o_start);
>    if ((h_file = RecFileOpenW(fpath)) != REC_HANDLE_INVALID) {
>      if (RecFileWrite(h_file, (char *) msg, msg_size, &bytes_written) ==
>      REC_ERR_FAIL) {
> +      RecFileClose(h_file);
>        return REC_ERR_FAIL;
>      }
>      RecFileClose(h_file);
> 
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7fd40eba/lib/records/RecProcess.cc
> ----------------------------------------------------------------------
> diff --git a/lib/records/RecProcess.cc b/lib/records/RecProcess.cc
> index 75b5829..5dc7392 100644
> --- a/lib/records/RecProcess.cc
> +++ b/lib/records/RecProcess.cc
> @@ -392,11 +392,7 @@ struct sync_cont: public Continuation
>      send_push_message();
>      RecSyncStatsFile();
>      if (RecSyncConfigToTB(m_tb) == REC_ERR_OKAY) {
> -      int nbytes;
> -      RecDebug(DL_Note, "Writing '%s'", g_rec_config_fpath);
> -      RecHandle h_file = RecFileOpenW(g_rec_config_fpath);
> -      RecFileWrite(h_file, m_tb->bufPtr(), m_tb->spaceUsed(), &nbytes);
> -      RecFileClose(h_file);
> +        RecWriteConfigFile(m_tb);
>      }
>      Debug("statsproc", "sync_cont() processed");
>  
> 
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7fd40eba/mgmt/Rollback.cc
> ----------------------------------------------------------------------
> diff --git a/mgmt/Rollback.cc b/mgmt/Rollback.cc
> index 6c219e3..4489a18 100644
> --- a/mgmt/Rollback.cc
> +++ b/mgmt/Rollback.cc
> @@ -140,7 +140,7 @@ Rollback::Rollback(const char *baseFileName, bool
> root_access_needed_)
>            mgmt_log(stderr, "[RollBack::Rollback] %s\n", alarmMsg);
>            lmgmt->alarm_keeper->signalAlarm(MGMT_ALARM_CONFIG_UPDATE_FAILED,
>            alarmMsg);
>            ats_free(alarmMsg);
> -          closeFile(fd);
> +          closeFile(fd, true);
>          } else {
>            mgmt_fatal(stderr, 0,
>                       "[RollBack::Rollback] Unable to find configuration file
>                       %s.\n\tCreation of a placeholder failed : %s\n",
> @@ -201,11 +201,11 @@ Rollback::Rollback(const char *baseFileName, bool
> root_access_needed_)
>        snprintf(alarmMsg, 2048, "Config file is read-only");
>        mgmt_log(stderr, "[Rollback::Rollback] %s : %s\n", alarmMsg,
>        fileName);
>        lmgmt->alarm_keeper->signalAlarm(MGMT_ALARM_CONFIG_UPDATE_FAILED,
>        alarmMsg);
> -      closeFile(testFD);
> +      closeFile(testFD, false);
>      }
>      ats_free(alarmMsg);
>    } else {
> -    closeFile(testFD);
> +    closeFile(testFD, true);
>    }
>  }
>  
> @@ -338,8 +338,9 @@ Rollback::openFile(version_t version, int oflags, int
> *errnoPtr)
>      }
>      mgmt_log(stderr, "[Rollback::openFile] Open of %s failed: %s\n",
>      fileName, strerror(errno));
>    }
> -
> -  fcntl(fd, F_SETFD, 1);
> +  else {
> +    fcntl(fd, F_SETFD, 1);
> +  }
>  
>    delete[]filePath;
>  
> @@ -347,18 +348,21 @@ Rollback::openFile(version_t version, int oflags, int
> *errnoPtr)
>  }
>  
>  int
> -Rollback::closeFile(int fd)
> +Rollback::closeFile(int fd, bool callSync)
>  {
> -  if (fsync(fd) == -1) {
> -    // INKqa11574: SGI fsync will return EBADF error if the file was
> -    // open w/o write access (e.g. read-only).  Do not print an error
> -    // if we get this errno back -- just in case this fd was open
> -    // O_RDONLY.
> -    if (errno != EBADF) {
> -      mgmt_log(stderr, "[Rollback::closeFile] fsync failed for file '%s'
> (%d: %s)\n", fileName, errno, strerror(errno));
> -    }
> +  int result = 0;
> +  if (callSync && fsync(fd) < 0) {
> +    result = -1;
> +    mgmt_log(stderr, "[Rollback::closeFile] fsync failed for file '%s' (%d:
> %s)\n", fileName, errno, strerror(errno));
>    }
> -  return (close(fd));
> +
> +  if (result == 0) {
> +    result = close(fd);
> +  }
> +  else {
> +    close(fd);
> +  }
> +  return result;
>  }
>  
>  
> @@ -422,6 +426,7 @@ Rollback::internalUpdate(textBuffer * buf, version_t
> newVersion, bool notifyChan
>    char *nextVersion;
>    int writeBytes;
>    int diskFD;
> +  int ret;
>    versionInfo *toRemove;
>    versionInfo *newBak;
>    bool failedLink = false;
> @@ -465,9 +470,9 @@ Rollback::internalUpdate(textBuffer * buf, version_t
> newVersion, bool notifyChan
>    }
>    // Write the buffer into the new configuration file
>    writeBytes = write(diskFD, buf->bufPtr(), buf->spaceUsed());
> -  closeFile(diskFD);
> -  if (writeBytes != buf->spaceUsed()) {
> -    mgmt_log(stderr, "[Rollback::intrernalUpdate] Unable to write new
> version of %s : %s\n", fileName, strerror(errno));
> +  ret = closeFile(diskFD, true);
> +  if ((ret < 0) || (writeBytes != buf->spaceUsed())) {
> +    mgmt_log(stderr, "[Rollback::internalUpdate] Unable to write new version
> of %s : %s\n", fileName, strerror(errno));
>      returnCode = SYS_CALL_ERROR_ROLLBACK;
>      goto UPDATE_CLEANUP;
>    }
> @@ -632,7 +637,7 @@ Rollback::getVersion_ml(version_t version, textBuffer **
> buffer)
>  GET_CLEANUP:
>  
>    if (diskFD >= 0) {
> -    closeFile(diskFD);
> +    closeFile(diskFD, false);
>    }
>  
>    return returnCode;
> 
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7fd40eba/mgmt/Rollback.h
> ----------------------------------------------------------------------
> diff --git a/mgmt/Rollback.h b/mgmt/Rollback.h
> index e3025a6..7b877a0 100644
> --- a/mgmt/Rollback.h
> +++ b/mgmt/Rollback.h
> @@ -203,7 +203,7 @@ public:
>  private:
>    Rollback(const Rollback &);
>    int openFile(version_t version, int oflags, int *errnoPtr = NULL);
> -  int closeFile(int fd);
> +  int closeFile(int fd, bool callSync);
>    int statFile(version_t version, struct stat *buf);
>    char *createPathStr(version_t version);
>    RollBackCodes internalUpdate(textBuffer * buf, version_t newVersion, bool
>    notifyChange = true, bool incVersion = true);
> 
> 

-- 
Igor Galić

Tel: +43 (0) 664 886 22 883
Mail: i.galic@brainsware.org
URL: http://brainsware.org/
GPG: 8716 7A9F 989B ABD5 100F  4008 F266 55D6 2998 1641