diff --git a/conf.c b/conf.c index c49c6d9f..60c7894e 100644 --- a/conf.c +++ b/conf.c @@ -25,16 +25,10 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. #include "log.h" #include "str.h" #include "mem.h" +#include "os.h" #define CONFFILE_NAME "serval.conf" -struct file_meta { - time_t mtime; - off_t size; -}; - -#define FILE_META_UNKNOWN ((struct file_meta){ .mtime = -1, .size = -1 }) - struct cf_om_node *cf_om_root = NULL; static struct file_meta conffile_meta = FILE_META_UNKNOWN; @@ -50,48 +44,23 @@ static const char *conffile_path() return path; } -static int get_meta(const char *path, struct file_meta *metap) -{ - struct stat st; - if (stat(path, &st) == -1) { - if (errno != ENOENT) - return WHYF_perror("stat(%s)", path); - // Do not return FILE_META_UNKNOWN on ENOENT, otherwise reload logic breaks. A non-existent - // file is treated as size == 0. - metap->size = 0; - metap->mtime = -1; - } else { - metap->size = st.st_size; - metap->mtime = st.st_mtime; - } - return 0; -} - -static int cmp_meta(const struct file_meta *a, const struct file_meta *b) -{ - return a->mtime < b->mtime ? -1 : a->mtime > b->mtime ? 1 : a->size < b->size ? -1 : a->size > b->size ? 1 : 0; -} - static int reload(const char *path, int *resultp) { if (config.debug.config) - DEBUGF("path=%s", alloca_str_toprint(path)); + DEBUGF(" file path=%s", alloca_str_toprint(path)); struct file_meta meta; - if (get_meta(conffile_path(), &meta) == -1) + if (get_file_meta(conffile_path(), &meta) == -1) return -1; if (config.debug.config) { - struct tm tm; - (void)localtime_r(&meta.mtime, &tm); - DEBUGF("meta.mtime=%s meta.size=%ld", alloca_strftime("%Y/%m/%d %H:%M:%S", &tm), (long)meta.size); - (void)localtime_r(&conffile_meta.mtime, &tm); - DEBUGF("conffile_meta.mtime=%s conffile_meta.size=%ld", alloca_strftime("%Y/%m/%d %H:%M:%S", &tm), (long)conffile_meta.size); + DEBUGF(" file meta=%s", alloca_file_meta(&meta)); + DEBUGF("conffile_meta=%s", alloca_file_meta(&conffile_meta)); } - if (cmp_meta(&meta, &conffile_meta) == 0) + if (cmp_file_meta(&meta, &conffile_meta) == 0) return 0; - if (conffile_meta.mtime != -1 && serverMode) + if (conffile_meta.mtime.tv_sec != -1 && serverMode) INFOF("config file %s -- detected new version", conffile_path()); char *buf = NULL; - if (meta.mtime == -1) { + if (meta.mtime.tv_sec == -1) { WARNF("config file %s does not exist -- using all defaults", path); } else if (meta.size > CONFIG_FILE_MAX_SIZE) { WHYF("config file %s is too big (%ju bytes exceeds limit %d)", path, (uintmax_t)meta.size, CONFIG_FILE_MAX_SIZE); @@ -126,11 +95,8 @@ static int reload(const char *path, int *resultp) INFOF("config file %s successfully read %ld bytes", path, (long) meta.size); } conffile_meta = meta; - if (config.debug.config) { - struct tm tm; - (void)localtime_r(&conffile_meta.mtime, &tm); - DEBUGF("conffile_meta.mtime=%s conffile_meta.size=%ld", alloca_strftime("%Y/%m/%d %H:%M:%S", &tm), (long)conffile_meta.size); - } + if (config.debug.config) + DEBUGF("set conffile_meta=%s", alloca_file_meta(&conffile_meta)); struct cf_om_node *new_root = NULL; *resultp = cf_om_parse(path, buf, meta.size, &new_root); free(buf); @@ -157,6 +123,9 @@ int cf_om_save() { if (cf_om_root) { const char *path = conffile_path(); + struct file_meta meta; + if (get_file_meta(path, &meta) == -1) + return -1; char tempfile[1024]; FILE *outf = NULL; if (!FORMF_SERVAL_ETC_PATH(tempfile, CONFFILE_NAME ".temp")) @@ -175,16 +144,19 @@ int cf_om_save() unlink(tempfile); return -1; } - struct file_meta meta; - if (get_meta(path, &meta) == -1) + struct file_meta newmeta; + int r = alter_file_meta(path, &meta, &newmeta); + if (r == -1) return -1; - INFOF("successfully wrote %s", path); - conffile_meta = meta; - if (config.debug.config) { - struct tm tm; - (void)localtime_r(&conffile_meta.mtime, &tm); - DEBUGF("conffile_meta.mtime=%s conffile_meta.size=%ld", alloca_strftime("%Y/%m/%d %H:%M:%S", &tm), (long)conffile_meta.size); - } + if (r) + INFOF("wrote %s; set mtime=%s", path, alloca_time_t(newmeta.mtime.tv_sec)); + else if (cmp_file_meta(&meta, &newmeta) == 0) + WARNF("wrote %s; mtime not altered", path); + else + INFOF("wrote %s", path); + conffile_meta = newmeta; + if (config.debug.config) + DEBUGF("set conffile_meta=%s", alloca_file_meta(&conffile_meta)); } return 0; } @@ -199,39 +171,55 @@ int cf_init() return 0; } +/* (Re-)load and parse the configuration file. + * + * The 'strict' flag controls whether this function will load a defective config file. If nonzero, + * then it will not load a defective file, but will log an error and return -1. If zero, then it + * will load a defective file and use the 'permissive' flag to decide what to log and return. + * + * The 'permissive' flag only applies if 'strict' is zero, and determines how this function deals + * with a loaded defective config file. If 'permissive' is zero, then it logs an error and returns + * -1. If nonzero, then it logs a warning and returns 0. + * + * @author Andrew Bettison + */ static int reload_and_parse(int permissive, int strict) { int result = CFOK; + int changed = 0; if (cf_limbo) result = cf_dfl_config_main(&config); if (result == CFOK || result == CFEMPTY) { if (reload(conffile_path(), &result) == -1) result = CFERROR; - else if (!cf_limbo && cmp_meta(&conffile_meta, &config_meta) == 0) + else if (!cf_limbo && cmp_file_meta(&conffile_meta, &config_meta) == 0) return 0; else { config_meta = conffile_meta; if (result == CFOK || result == CFEMPTY) { struct config_main new_config; + int update = 0; memset(&new_config, 0, sizeof new_config); result = cf_dfl_config_main(&new_config); if (result == CFOK || result == CFEMPTY) { result = cf_om_root ? cf_opt_config_main(&new_config, cf_om_root) : CFEMPTY; if (result == CFOK || result == CFEMPTY) { result = CFOK; - config = new_config; + update = 1; } else if (result != CFERROR && !strict) { result &= ~CFEMPTY; // don't log "empty" as a problem - config = new_config; + update = 1; } } + if (update && cf_cmp_config_main(&config, &new_config) != 0) { + config = new_config; + changed = 1; + } } } } - int ret = 1; - if (result == CFOK) { - logConfigChanged(); - } else { + int ret = changed; + if (result != CFOK) { strbuf b = strbuf_alloca(180); strbuf_cf_flag_reason(b, result); if (strict) @@ -241,11 +229,14 @@ static int reload_and_parse(int permissive, int strict) ret = WHYF("config file %s loaded despite defects -- %s", conffile_path(), strbuf_str(b)); else WARNF("config file %s loaded despite defects -- %s", conffile_path(), strbuf_str(b)); - logConfigChanged(); } } cf_limbo = 0; // let log messages out logFlush(); + if (changed) { + logConfigChanged(); + cf_on_config_change(); + } return ret; } diff --git a/conf.h b/conf.h index c325ecf2..69738168 100644 --- a/conf.h +++ b/conf.h @@ -298,9 +298,9 @@ void cf_om_iter_start(struct cf_om_iterator *, const struct cf_om_node *); int cf_om_iter_next(struct cf_om_iterator *); struct cf_om_node *cf_om_root; -int cf_om_load(); -int cf_om_reload(); -int cf_om_save(); +int cf_om_load(void); +int cf_om_reload(void); +int cf_om_save(void); /* Diagnostic functions for use in config schema parsing functions, cf_opt_xxx(). */ @@ -697,12 +697,14 @@ int cf_fmt_encapsulation(const char **, const short *encapp); extern int cf_limbo; extern struct config_main config; -int cf_init(); -int cf_load(); -int cf_load_strict(); -int cf_load_permissive(); -int cf_reload(); -int cf_reload_strict(); -int cf_reload_permissive(); +int cf_init(void); +int cf_load(void); +int cf_load_strict(void); +int cf_load_permissive(void); +int cf_reload(void); +int cf_reload_strict(void); +int cf_reload_permissive(void); + +void cf_on_config_change(void); #endif //__SERVAL_DNA__CONF_H diff --git a/directory_service.c b/directory_service.c index 05e1f8e0..71b1b88d 100644 --- a/directory_service.c +++ b/directory_service.c @@ -180,6 +180,10 @@ static void resolve_request(){ } } +void cf_on_config_change() +{ +} + int main(void){ struct pollfd fds[2]; int mdp_sockfd; diff --git a/os.c b/os.c index 95ae3265..f9e9bd40 100644 --- a/os.c +++ b/os.c @@ -23,6 +23,7 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. #include "mem.h" #include "str.h" #include "log.h" +#include "strbuf_helpers.h" #include #include @@ -219,3 +220,113 @@ int malloc_read_whole_file(const char *path, unsigned char **bufp, size_t *sizp) ret = WHYF_perror("close(%d)", fd); return ret; } + +int get_file_meta(const char *path, struct file_meta *metap) +{ + struct stat st; + if (stat(path, &st) == -1) { + if (errno != ENOENT) + return WHYF_perror("stat(%s)", path); + *metap = FILE_META_NONEXIST; + } else { + metap->size = st.st_size; + metap->mtime = st.st_mtim; + // Truncate to whole seconds to ensure that this code will work on file systems that only have + // whole-second time stamp resolution. + metap->mtime.tv_nsec = 0; + } + return 0; +} + +static int cmp_timespec(const struct timespec *a, const struct timespec *b) +{ + return a->tv_sec < b->tv_sec ? -1 : a->tv_sec > b->tv_sec ? 1 : a->tv_nsec < b->tv_nsec ? -1 : a->tv_nsec > b->tv_nsec ? 1 : 0; +} + +static void add_timespec(struct timespec *tv, long sec, long nsec) +{ + const long NANO = 1000000000; + tv->tv_sec += sec; + // Bring nsec into range -NANO < nsec < NANO. + if (nsec >= NANO) { + sec = nsec / NANO; + tv->tv_sec += sec; + nsec -= sec * NANO; + } else if (nsec <= -NANO) { + // The C standard does not define whether negative integer division truncates towards negative + // infinity or rounds towards zero. So we have to use positive integer division, which always + // truncates towards zero. + sec = -nsec / NANO; + tv->tv_sec -= sec; + nsec += sec * NANO; + } + assert(nsec > -NANO); + assert(nsec < NANO); + tv->tv_nsec += nsec; + // Bring tv_nsec into range 0 <= tv_nsec < NANO. + if (tv->tv_nsec >= NANO) { + sec = tv->tv_nsec / NANO; + tv->tv_sec += sec; + tv->tv_nsec -= sec * NANO; + } else if (tv->tv_nsec < 0) { + sec = (-tv->tv_nsec + NANO - 1) / NANO; + tv->tv_sec -= sec; + tv->tv_nsec += sec * NANO; + } + assert(tv->tv_nsec >= 0); + assert(tv->tv_nsec < NANO); +} + +int cmp_file_meta(const struct file_meta *a, const struct file_meta *b) +{ + int c = cmp_timespec(&a->mtime, &b->mtime); + return c ? c : a->size < b->size ? -1 : a->size > b->size ? 1 : 0; +} + +/* Post-update file meta adjustment. + * + * If a file's meta information is used to detect changes to the file by polling at regular + * intervals, then every update to the file must guarantee to never produce the same meta + * information as any prior update. The typical case is several updates in rapid succession during + * one second that do not change the size of the file. The second and subsequent of these will not + * change the file's meta information (size or last-modified time stamp) on file systems that only + * have one-second timestamp resolution. + * + * This function can be called immediately after updating such a file, supplying the meta + * information from just prior to the update. It will alter the file's meta information (last + * modified time stamp) to ensure that it differs from the prior meta information. This typically + * involves advancing the file's last-modification time stamp. + * + * Returns -1 if an error occurs, 1 if it alters the file's meta information, 0 if the current meta + * information is already different and did not need alteration. + * + * @author Andrew Bettison + */ +int alter_file_meta(const char *path, const struct file_meta *origp, struct file_meta *metap) +{ + long nsec = 1; + long sec = 0; + // If the file's current last-modified timestamp is not greater than its original, try bumping the + // original timestamp by one nanosecond, and if that does not alter the timestamp, the file system + // does not support nanosecond timestamps, so try bumping it by one second. + while (sec <= 1) { + struct file_meta meta; + if (get_file_meta(path, &meta) == -1) + return -1; + if (metap) + *metap = meta; + if (is_file_meta_nonexist(&meta) || cmp_timespec(&origp->mtime, &meta.mtime) < 0) + return 0; + meta.mtime = origp->mtime; + add_timespec(&meta.mtime, sec, nsec); + struct timespec times[2]; + times[0].tv_sec = 0; + times[0].tv_nsec = UTIME_OMIT; + times[1] = meta.mtime; + if (utimensat(AT_FDCWD, path, times, 0) == -1) + return WHYF_perror("utimensat(AT_FDCWD,%s,[UTIME_OMIT,%s],0)", alloca_str_toprint(path), alloca_timespec(×[1])); + nsec = 0; + ++sec; + } + return 1; +} diff --git a/os.h b/os.h index 277447da..f9e04cc0 100644 --- a/os.h +++ b/os.h @@ -176,4 +176,31 @@ ssize_t read_whole_file(const char *path, unsigned char *buffer, size_t buffer_s */ int malloc_read_whole_file(const char *path, unsigned char **bufp, size_t *sizp); +/* File metadata primitives, used for detecting when a file has changed. + * + * @author Andrew Bettison + */ +struct file_meta { + struct timespec mtime; + off_t size; +}; + +#define FILE_META_UNKNOWN ((struct file_meta){ .mtime = { .tv_sec = -1, .tv_nsec = -1 }, .size = -1 }) + +// A non-existent file is treated as size == 0 and an impossible modification +// time, so that cmp_file_meta() will not compare it as equal with any existing +// file. +#define FILE_META_NONEXIST ((struct file_meta){ .mtime = { .tv_sec = -1, .tv_nsec = -1 }, .size = 0 }) + +__SERVAL_DNA__OS_INLINE int is_file_meta_nonexist(const struct file_meta *m) { + return m->mtime.tv_sec == -1 && m->mtime.tv_nsec == -1 && m->size == 0; +} + +int get_file_meta(const char *path, struct file_meta *metap); +int cmp_file_meta(const struct file_meta *a, const struct file_meta *b); + +// Ensure that the metadata of a file differs from a given original metadata, +// by bumping the file's modification time or altering its inode. +int alter_file_meta(const char *path, const struct file_meta *origp, struct file_meta *metap); + #endif //__SERVAL_DNA__OS_H diff --git a/server.c b/server.c index 016a13ca..e4c037b0 100644 --- a/server.c +++ b/server.c @@ -285,6 +285,10 @@ void server_watchdog(struct sched_ent *alarm) } } +void cf_on_config_change() +{ +} + /* Called periodically by the server process in its main loop. */ void server_shutdown_check(struct sched_ent *alarm) diff --git a/strbuf_helpers.c b/strbuf_helpers.c index db791f6e..3ebbed36 100644 --- a/strbuf_helpers.c +++ b/strbuf_helpers.c @@ -452,6 +452,36 @@ strbuf strbuf_append_iovec(strbuf sb, const struct iovec *iov, int iovcnt) return sb; } +strbuf strbuf_append_time_t(strbuf sb, time_t time) +{ + struct tm tm; + localtime_r(&time, &tm); + strbuf_append_strftime(sb, "%Y/%m/%d %H:%M:%S %z", &tm); + return sb; +} + +strbuf strbuf_append_timespec(strbuf sb, const struct timespec *tv) +{ + if (tv->tv_sec < 0 || tv->tv_nsec < 0 || tv->tv_nsec > 999999999) { + strbuf_sprintf(sb, "INVALID{tv_sec=%ld,tv_nsec=%ld}", (long)tv->tv_sec, tv->tv_nsec); + } else { + struct tm tm; + localtime_r(&tv->tv_sec, &tm); + strbuf_append_strftime(sb, "%Y/%m/%d %H:%M:%S", &tm); + strbuf_sprintf(sb, ".%.09lu", tv->tv_nsec); + strbuf_append_strftime(sb, " %z", &tm); + } + return sb; +} + +strbuf strbuf_append_file_meta(strbuf sb, const struct file_meta *metap) +{ + strbuf_puts(sb, "{ .mtime="); + strbuf_append_timespec(sb, &metap->mtime); + strbuf_sprintf(sb, ", .size=%ld }", (long)metap->size); + return sb; +} + strbuf strbuf_append_quoted_string(strbuf sb, const char *str) { strbuf_putc(sb, '"'); diff --git a/strbuf_helpers.h b/strbuf_helpers.h index 3967c7aa..92820a5f 100644 --- a/strbuf_helpers.h +++ b/strbuf_helpers.h @@ -161,6 +161,26 @@ struct iovec; strbuf strbuf_append_iovec(strbuf sb, const struct iovec *iov, int iovcnt); #define alloca_iovec(iov,cnt) strbuf_str(strbuf_append_iovec(strbuf_alloca(200), (iov), (cnt))) +/* Append a representation of a time_t value. + * @author Andrew Bettison + */ +strbuf strbuf_append_time_t(strbuf sb, time_t); +#define alloca_time_t(t) strbuf_str(strbuf_append_time_t(strbuf_alloca(40), (t))) + +/* Append a representation of a struct timespec. + * @author Andrew Bettison + */ +struct timespec; +strbuf strbuf_append_timespec(strbuf sb, const struct timespec *tv); +#define alloca_timespec(tv) strbuf_str(strbuf_append_timespec(strbuf_alloca(50), (tv))) + +/* Append a representation of a struct file_meta. + * @author Andrew Bettison + */ +struct file_meta; +strbuf strbuf_append_file_meta(strbuf sb, const struct file_meta *metap); +#define alloca_file_meta(metap) strbuf_str(strbuf_append_file_meta(strbuf_alloca(80), (metap))) + /* Append a string using HTTP quoted-string format: delimited by double quotes (") and * internal double quotes and backslash escaped by leading backslash. * @author Andrew Bettison diff --git a/testdefs.sh b/testdefs.sh index 6b44fbbb..ee6577e3 100644 --- a/testdefs.sh +++ b/testdefs.sh @@ -385,6 +385,28 @@ servald_start() { SERVALD_SERVER_CHDIR="$instance_dir" SERVALD_LOG_FILE="$instance_servald_log" $servald start "$@" } +# Utility function: +# - test whether the daemon's HTTP server has started +servald_http_server_started() { + local logvar=LOG${1#+} + $GREP 'HTTP SERVER START.*port=[0-9]' "${!logvar}" +} + +# Utility function: +# - fetch the daemon's HTTP server port number +get_servald_http_server_port() { + push_and_set_instance $2 || return $? + local _var="$1" + local _port=$(<"$SERVALINSTANCE_PATH/proc/http_port") + assert --message="instance $instance_name HTTP server port number is known" [ -n "$_port" ] + if [ -n "$_var" ]; then + eval "$_var=\$_port" + tfw_log "$_var=$_port" + fi + pop_instance + return 0 +} + # Utility function: # - stop a servald server process instance in an orderly fashion # - cat its log file into the test log diff --git a/testdefs_rhizome.sh b/testdefs_rhizome.sh index 61ff81f7..04939d28 100644 --- a/testdefs_rhizome.sh +++ b/testdefs_rhizome.sh @@ -379,16 +379,7 @@ rhizome_http_server_started() { } get_rhizome_server_port() { - push_and_set_instance $2 || return $? - local _var="$1" - local _port=$(<"$SERVALINSTANCE_PATH/proc/http_port") - assert --message="instance $instance_name Rhizome HTTP server port number is known" [ -n "$_port" ] - if [ -n "$_var" ]; then - eval "$_var=\$_port" - tfw_log "$_var=$_port" - fi - pop_instance - return 0 + get_servald_http_server_port "$@" } # Predicate function: diff --git a/tests/server b/tests/server index 381e16fb..5ee39718 100755 --- a/tests/server +++ b/tests/server @@ -238,11 +238,6 @@ test_ReloadConfigSync() { >trace1 wait_until --sleep=0.5 --timeout=15 line_count_at_least trace1 3 assert [ $(wc -l trace1 @@ -252,4 +247,60 @@ test_ReloadConfigSync() { assert_servald_server_no_errors } +doc_ReloadConfigSetSync="Server configuration always syncs after set" +setup_ReloadConfigSetSync() { + setup_curl 7 + setup + set_instance +A + create_single_identity + executeOk_servald config \ + set log.console.level debug \ + set log.console.show_time true \ + set log.console.show_pid true \ + set debug.config on \ + set debug.mdprequests on \ + set server.config_reload_interval_ms 600000 \ + set server.motd "Abcdef" + conf_size=$(wc -l <"$SERVALINSTANCE_PATH/serval.conf") + assert [ "$conf_size" -gt 0 ] + start_servald_server + wait_until servald_http_server_started +A + get_servald_http_server_port PORTA +A +} +test_ReloadConfigSetSync() { + # Set the MOTD without changing the config file size and sync it, and the + # HTTP server returns it in the root page immediately. + executeOk_servald config set server.motd "Yoicks" sync + tfw_cat --stderr + assert [ $conf_size -eq $(wc -l <"$SERVALINSTANCE_PATH/serval.conf") ] + executeOk curl \ + --silent --fail --show-error \ + --output root.html \ + --dump-header http.headers \ + "http://$addr_localhost:$PORTA/" + assertGrep root.html Yoicks + # Set the MOTD without changing the config file size but do not sync it, and + # the HTTP server still returns the prior value of the MOTD. + executeOk_servald config set server.motd "Flarng" + tfw_cat --stderr + assert [ $conf_size -eq $(wc -l <"$SERVALINSTANCE_PATH/serval.conf") ] + executeOk curl \ + --silent --fail --show-error \ + --output root.html \ + --dump-header http.headers \ + "http://$addr_localhost:$PORTA/" + assertGrep root.html Yoicks + # Sync the config to the daemon, and now the HTTP server returns the new + # MOTD. + executeOk_servald config sync + executeOk curl \ + --silent --fail --show-error \ + --output root.html \ + --dump-header http.headers \ + "http://$addr_localhost:$PORTA/" + assertGrep root.html Flarng + stop_servald_server + assert_servald_server_no_errors +} + runTests "$@"