Browse Source

lib/command: make config file robust more robust and kinder to system

* command.c: (config_write_file) Remove two very heavyweights sync()s and
  replace with an fdatasync of just the freshly writen config file data.
  Make the move of the new config into place more robust, by using
  rename instead of unlink/link.

  This should fix a performance issue on systems with slow storage,
  where the syncs were disrupting performance, see bugzilla #966. Should
  also be more robust.

  Problem diagnosed and reported by:

   Patrick Kuijvenhoven <patrick.kuijvenhoven@gmail.com>

  with an initial fix, on which this commit develops, and further work on
  testing.
Paul Jakma 1 year ago
parent
commit
dcfbf4eb45
1 changed files with 19 additions and 13 deletions
  1. 19 13
      lib/command.c

+ 19 - 13
lib/command.c

@@ -3071,7 +3071,7 @@ DEFUN (config_write_file,
        "Write to configuration file\n")
 {
   unsigned int i;
-  int fd;
+  int fd, dupfd = -1;
   struct cmd_node *node;
   char *config_file;
   char *config_file_tmp = NULL;
@@ -3124,7 +3124,20 @@ DEFUN (config_write_file,
 	if ((*node->func) (file_vty))
 	  vty_out (file_vty, "!\n");
       }
+  
+  if ((dupfd = dup (file_vty->wfd)) < 0)
+    {
+      vty_out (vty, "Couldn't dup fd (for fdatasync) for %s, %s (%d).%s", 
+               config_file, safe_strerror(errno), errno, VTY_NEWLINE);
+    }
+
   vty_close (file_vty);
+  
+  if (fdatasync (dupfd) < 0)
+    {
+      vty_out (vty, "Couldn't fdatasync %s, %s (%d)!%s",
+               config_file, safe_strerror(errno), errno, VTY_NEWLINE);
+    }
 
   if (unlink (config_file_sav) != 0)
     if (errno != ENOENT)
@@ -3139,21 +3152,12 @@ DEFUN (config_write_file,
 	        VTY_NEWLINE);
       goto finished;
     }
-  sync ();
-  if (unlink (config_file) != 0)
+  if (rename (config_file_tmp, config_file) != 0)
     {
-      vty_out (vty, "Can't unlink configuration file %s.%s", config_file,
-	        VTY_NEWLINE);
-      goto finished;
-    }
-  if (link (config_file_tmp, config_file) != 0)
-    {
-      vty_out (vty, "Can't save configuration file %s.%s", config_file,
-	       VTY_NEWLINE);
+      vty_out (vty, "Can't move configuration file %s into place.%s",
+               config_file, VTY_NEWLINE);
       goto finished;
     }
-  sync ();
-  
   if (chmod (config_file, CONFIGFILE_MASK) != 0)
     {
       vty_out (vty, "Can't chmod configuration file %s: %s (%d).%s", 
@@ -3166,6 +3170,8 @@ DEFUN (config_write_file,
   ret = CMD_SUCCESS;
 
 finished:
+  if (dupfd >= 0)
+    close (dupfd);
   unlink (config_file_tmp);
   XFREE (MTYPE_TMP, config_file_tmp);
   XFREE (MTYPE_TMP, config_file_sav);