lib: create `frr_daemon_state_{load,save}`

These functions load daemon-specific persistent state from
`/var/lib/frr` and supersede open-coded variants of similar calls in
ospfd, ospf6d and isisd to save GR state and/or sequence numbers.

Unlike the open-coded variants, the save call correctly `fsync()`s the
saved data to ensure disk contents are consistent.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

diff --git a/lib/command.c b/lib/command.c
index becba8452bad2482f4d696aba7f689f8b8663b66..244bf86e8e949d45d29ced16c693c0eedb6cd50a 100644 (file)
--- a/lib/command.c
+++ b/lib/command.c
@@ -1633,6 +1633,10 @@ static int vty_write_config(struct vty *vty)
        return CMD_SUCCESS;
 }
 
+/* cross-reference frr_daemon_state_save in libfrr.c
+ * the code there is similar but not identical (state files always use the same
+ * name for the new write, and don't keep a backup of previous state.)
+ */
 static int file_write_config(struct vty *vty)
 {
        int fd, dirfd;

diff --git a/lib/libfrr.c b/lib/libfrr.c
index 85a906a7bc4cad85be590c069cfbc13d4220bce2..131453c4de84b1467a48decc9cc65c3a9eb8c3ef 100644 (file)
--- a/lib/libfrr.c
+++ b/lib/libfrr.c
@@ -1272,6 +1272,161 @@ void frr_fini(void)
        }
 }
 
+struct json_object *frr_daemon_state_load(void)
+{
+       struct json_object *state;
+       char **state_path;
+
+       assertf(di->state_paths,
+               "CODE BUG: daemon trying to load state, but no state path in frr_daemon_info");
+
+       for (state_path = di->state_paths; *state_path; state_path++) {
+               state = json_object_from_file(*state_path);
+               if (state)
+                       return state;
+       }
+
+       return json_object_new_object();
+}
+
+/* cross-reference file_write_config() in command.c
+ * the code there is similar but not identical (configs use a unique temporary
+ * name for writing and keep a backup of the previous config.)
+ */
+void frr_daemon_state_save(struct json_object **statep)
+{
+       struct json_object *state = *statep;
+       char *state_path, *slash, *temp_name, **other;
+       size_t name_len, json_len;
+       const char *json_str;
+       int dirfd, fd;
+
+       assertf(di->state_paths,
+               "CODE BUG: daemon trying to save state, but no state path in frr_daemon_info");
+
+       state_path = di->state_paths[0];
+       json_str = json_object_to_json_string_ext(state,
+                                                 JSON_C_TO_STRING_PRETTY);
+       json_len = strlen(json_str);
+
+       /* To correctly fsync() and ensure we have either consistent old state
+        * or consistent new state but no fs-damage garbage inbetween, we need
+        * to work with a directory fd.  If we need that anyway we might as
+        * well use the dirfd with openat() & co in fd-relative operations.
+        */
+
+       slash = strrchr(state_path, '/');
+       if (slash) {
+               char *state_dir;
+
+               state_dir = XSTRDUP(MTYPE_TMP, state_path);
+               state_dir[slash - state_path] = '\0';
+               dirfd = open(state_dir, O_DIRECTORY | O_RDONLY);
+               XFREE(MTYPE_TMP, state_dir);
+
+               if (dirfd < 0) {
+                       zlog_err("failed to open directory %pSQq for saving daemon state: %m",
+                                state_dir);
+                       return;
+               }
+
+               /* skip to file name */
+               slash++;
+       } else {
+               dirfd = open(".", O_DIRECTORY | O_RDONLY);
+               if (dirfd < 0) {
+                       zlog_err(
+                               "failed to open current directory for saving daemon state: %m");
+                       return;
+               }
+
+               /* file name = path */
+               slash = state_path;
+       }
+
+       /* unlike saving configs, a temporary unique filename is unhelpful
+        * here as it might litter files on limited write-heavy storage
+        * (think switch with small NOR flash for frequently written data.)
+        *
+        * => always use filename with .sav suffix, worst case it litters one
+        * file.
+        */
+       name_len = strlen(slash);
+       temp_name = XMALLOC(MTYPE_TMP, name_len + 5);
+       memcpy(temp_name, slash, name_len);
+       memcpy(temp_name + name_len, ".sav", 5);
+
+       /* state file is always 0600, it's by and for FRR itself only */
+       fd = openat(dirfd, temp_name, O_WRONLY | O_CREAT | O_TRUNC, 0600);
+       if (fd < 0) {
+               zlog_err("failed to open temporary daemon state save file for %pSQq: %m",
+                        state_path);
+               goto out_closedir_free;
+       }
+
+       while (json_len) {
+               ssize_t nwr = write(fd, json_str, json_len);
+
+               if (nwr <= 0) {
+                       zlog_err("failed to write temporary daemon state to %pSQq: %m",
+                                state_path);
+
+                       close(fd);
+                       unlinkat(dirfd, temp_name, 0);
+                       goto out_closedir_free;
+               }
+
+               json_str += nwr;
+               json_len -= nwr;
+       }
+
+       /* fsync is theoretically implicit in close(), but... */
+       if (fsync(fd) < 0)
+               zlog_warn("fsync for daemon state %pSQq failed: %m", state_path);
+       close(fd);
+
+       /* this is the *actual* fsync that ensures we're consistent.  The
+        * file fsync only syncs the inode, but not the directory entry
+        * referring to it.
+        */
+       if (fsync(dirfd) < 0)
+               zlog_warn("directory fsync for daemon state %pSQq failed: %m",
+                         state_path);
+
+       /* atomic, hopefully. */
+       if (renameat(dirfd, temp_name, dirfd, slash) < 0) {
+               zlog_err("renaming daemon state %pSQq to %pSQq failed: %m",
+                        temp_name, state_path);
+               /* no unlink here, give the user a chance to investigate */
+               goto out_closedir_free;
+       }
+
+       /* and the rename needs to be synced too */
+       if (fsync(dirfd) < 0)
+               zlog_warn("directory fsync for daemon state %pSQq failed after rename: %m",
+                         state_path);
+
+       /* daemon may specify other deprecated paths to load from; since we
+        * just saved successfully we should delete those.
+        */
+       for (other = di->state_paths + 1; *other; other++) {
+               if (unlink(*other) == 0)
+                       continue;
+               if (errno == ENOENT || errno == ENOTDIR)
+                       continue;
+
+               zlog_warn("failed to remove deprecated daemon state file %pSQq: %m",
+                         *other);
+       }
+
+out_closedir_free:
+       XFREE(MTYPE_TMP, temp_name);
+       close(dirfd);
+
+       json_object_free(state);
+       *statep = NULL;
+}
+
 #ifdef INTERP
 static const char interp[]
        __attribute__((section(".interp"), used)) = INTERP;

diff --git a/lib/libfrr.h b/lib/libfrr.h
index 189abf0aec3adfbc109342be9da3ec1ff5c6a117..27b877da18b13045ab3650fd3ba2059964263197 100644 (file)
--- a/lib/libfrr.h
+++ b/lib/libfrr.h
@@ -87,6 +87,7 @@ struct frr_daemon_info {
        const char *vty_path;
        const char *module_path;
        const char *script_path;
+       char **state_paths;
 
        const char *pathspace;
        bool zpathspace;
@@ -162,6 +163,10 @@ extern void frr_vty_serv_stop(void);
 extern bool frr_zclient_addr(struct sockaddr_storage *sa, socklen_t *sa_len,
                             const char *path);
 
+struct json_object;
+extern struct json_object *frr_daemon_state_load(void);
+extern void frr_daemon_state_save(struct json_object **statep);
+
 /* these two are before the protocol daemon does its own shutdown
  * it's named this way being the counterpart to frr_late_init */
 DECLARE_KOOH(frr_early_fini, (), ());