lib: use fsync() for config writes, plug fd leak

sync() has a HUGE impact on systems that perform actual I/O, i.e. real
servers...

Also, we were leaking a fd on each config write ever since
c5e69a0 "lib/vty: add separate output fd support to VTYs"
(by myself :( ...)

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
This commit is contained in:
David Lamparter 2017-02-08 16:14:10 +01:00
parent cf80f28161
commit 1520d0aca9
2 changed files with 45 additions and 27 deletions

View File

@ -3139,9 +3139,9 @@ DEFUN (config_write_file,
"Write to configuration file\n") "Write to configuration file\n")
{ {
unsigned int i; unsigned int i;
int fd; int fd, dirfd;
struct cmd_node *node; struct cmd_node *node;
char *config_file; char *config_file, *slash;
char *config_file_tmp = NULL; char *config_file_tmp = NULL;
char *config_file_sav = NULL; char *config_file_sav = NULL;
int ret = CMD_WARNING; int ret = CMD_WARNING;
@ -3162,6 +3162,21 @@ DEFUN (config_write_file,
/* Get filename. */ /* Get filename. */
config_file = host.config; config_file = host.config;
#ifndef O_DIRECTORY
#define O_DIRECTORY 0
#endif
slash = strrchr (config_file, '/');
if (slash)
{
char *config_dir = XSTRDUP (MTYPE_TMP, config_file);
config_dir[slash - config_file] = '\0';
dirfd = open(config_dir, O_DIRECTORY | O_RDONLY);
XFREE (MTYPE_TMP, config_dir);
}
else
dirfd = open(".", O_DIRECTORY | O_RDONLY);
/* if dirfd is invalid, directory sync fails, but we're still OK */
config_file_sav = config_file_sav =
XMALLOC (MTYPE_TMP, strlen (config_file) + strlen (CONF_BACKUP_EXT) + 1); XMALLOC (MTYPE_TMP, strlen (config_file) + strlen (CONF_BACKUP_EXT) + 1);
strcpy (config_file_sav, config_file); strcpy (config_file_sav, config_file);
@ -3180,6 +3195,13 @@ DEFUN (config_write_file,
goto finished; goto finished;
} }
if (fchmod (fd, CONFIGFILE_MASK) != 0)
{
vty_out (vty, "Can't chmod configuration file %s: %s (%d).%s",
config_file_tmp, safe_strerror(errno), errno, VTY_NEWLINE);
goto finished;
}
/* Make vty for configuration file. */ /* Make vty for configuration file. */
file_vty = vty_new (); file_vty = vty_new ();
file_vty->wfd = fd; file_vty->wfd = fd;
@ -3213,35 +3235,24 @@ DEFUN (config_write_file,
VTY_NEWLINE); VTY_NEWLINE);
goto finished; goto finished;
} }
sync (); fsync (dirfd);
if (unlink (config_file) != 0)
{
vty_out (vty, "Can't unlink configuration file %s.%s", config_file,
VTY_NEWLINE);
goto finished;
} }
} if (rename (config_file_tmp, config_file) != 0)
if (link (config_file_tmp, config_file) != 0)
{ {
vty_out (vty, "Can't save configuration file %s.%s", config_file, vty_out (vty, "Can't save configuration file %s.%s", config_file,
VTY_NEWLINE); VTY_NEWLINE);
goto finished; goto finished;
} }
sync (); fsync (dirfd);
if (chmod (config_file, CONFIGFILE_MASK) != 0)
{
vty_out (vty, "Can't chmod configuration file %s: %s (%d).%s",
config_file, safe_strerror(errno), errno, VTY_NEWLINE);
goto finished;
}
vty_out (vty, "Configuration saved to %s%s", config_file, vty_out (vty, "Configuration saved to %s%s", config_file,
VTY_NEWLINE); VTY_NEWLINE);
ret = CMD_SUCCESS; ret = CMD_SUCCESS;
finished: finished:
if (ret != CMD_SUCCESS)
unlink (config_file_tmp); unlink (config_file_tmp);
close (dirfd);
XFREE (MTYPE_TMP, config_file_tmp); XFREE (MTYPE_TMP, config_file_tmp);
XFREE (MTYPE_TMP, config_file_sav); XFREE (MTYPE_TMP, config_file_sav);
return ret; return ret;

View File

@ -2330,9 +2330,16 @@ vty_close (struct vty *vty)
/* Unset vector. */ /* Unset vector. */
vector_unset (vtyvec, vty->fd); vector_unset (vtyvec, vty->fd);
if (vty->wfd > 0 && vty->type == VTY_FILE)
fsync (vty->wfd);
/* Close socket. */ /* Close socket. */
if (vty->fd > 0) if (vty->fd > 0)
{
close (vty->fd); close (vty->fd);
if (vty->wfd > 0 && vty->wfd != vty->fd)
close (vty->wfd);
}
else else
vty_stdio_reset (); vty_stdio_reset ();