From: Richard Kettlewell <rjk@greenend.org.uk>
Date: Sun, 12 Apr 2009 16:52:03 +0000 (+0100)
Subject: Do some basic compatibility checking when installing a new server
X-Git-Tag: 5.0~137
X-Git-Url: https://www.chiark.greenend.org.uk/ucgi/~mdw/git/disorder/commitdiff_plain/02ba7921a8b083f421a747b89c65a87d0afd5841

Do some basic compatibility checking when installing a new server
configuration.  There is plenty more that could be done.
---

diff --git a/README.reload b/README.reload
index 66d74a8..6c645ec 100644
--- a/README.reload
+++ b/README.reload
@@ -5,14 +5,15 @@ This is current rather vaguely defined and implemented.  This file
 will lay out what you can and cannot change over a reconfigure.  Any
 other changes will require a full server restart.
 
-I intend to improve the situation, but for now this is how it is.
+The situation is gradually improving; this file tracks the current
+state.
 
 * Options that might have to remain the same across restart
 
 Arguably if there is anything in this section then that's a serious
 bug!
 
-** alias
+** alias (enforced at reload only)
 
 This defines how aliases are inserted into the track database.  Need
 to think about how changing it will affect things.
@@ -26,7 +27,7 @@ Probably affects alias construction.
 The search database will have to be rebuilt from scratch.  This is
 do-able but AFAIK we don't detect it.
 
-** user
+** user (enforced at reload only)
 
 All the files will be owned by the wrong user!
 
@@ -36,20 +37,18 @@ Some things will just require a restart.  We should either enforce
 this (refusing to accept modified configurations that purport to
 change them) or explicitly ignore it.
 
-** home
+** home (enforced at reload)
 
-We absolutely cannot accept changing our state directory.  If this is
-attempted it must be rejected.
+We absolutely cannot accept changing our state directory.
 
-** lock
+** lock (generates a deprecation warning)
 
 Liable to be removed anyway.
 
-** nice_speaker
+** nice_speaker (generates a warning)
 
-Can't renice a running speaker to make it less nice (and we don't try
-to make it more nice).  We ought to issue an error message and
-otherwise ignore.
+You can't renice a running speaker to make it less nice (and we don't
+try to make it more nice).
 
 * Options that ought to be changable across reload but aren't
 
diff --git a/cgi/cgimain.c b/cgi/cgimain.c
index 583af30..e4c7220 100644
--- a/cgi/cgimain.c
+++ b/cgi/cgimain.c
@@ -48,7 +48,7 @@ int main(int argc, char **argv) {
   if(getenv("DISORDER_DEBUG"))
     debugging = 1;
   /* Read configuration */
-  if(config_read(0/*!server*/))
+  if(config_read(0/*!server*/, NULL))
     exit(EXIT_FAILURE);
   /* Figure out our URL.  This can still be overridden from the config file if
    * necessary but it shouldn't be necessary in ordinary installations. */
diff --git a/clients/disorder.c b/clients/disorder.c
index 2466311..bc2a8ec 100644
--- a/clients/disorder.c
+++ b/clients/disorder.c
@@ -158,7 +158,7 @@ static void cf_shutdown(char attribute((unused)) **argv) {
 
 static void cf_reconfigure(char attribute((unused)) **argv) {
   /* Re-check configuration for server */
-  if(config_read(1)) fatal(0, "cannot read configuration");
+  if(config_read(1, NULL)) fatal(0, "cannot read configuration");
   if(disorder_reconfigure(getclient())) exit(EXIT_FAILURE);
 }
 
@@ -754,7 +754,7 @@ int main(int argc, char **argv) {
     default: fatal(0, "invalid option");
     }
   }
-  if(config_read(0)) fatal(0, "cannot read configuration");
+  if(config_read(0, NULL)) fatal(0, "cannot read configuration");
   if(user) {
     config->username = user;
     config->password = 0;
diff --git a/clients/playrtp.c b/clients/playrtp.c
index f5f538d..81c1e8e 100644
--- a/clients/playrtp.c
+++ b/clients/playrtp.c
@@ -621,7 +621,7 @@ int main(int argc, char **argv) {
     default: fatal(0, "invalid option");
     }
   }
-  if(config_read(0)) fatal(0, "cannot read configuration");
+  if(config_read(0, NULL)) fatal(0, "cannot read configuration");
   if(!maxbuffer)
     maxbuffer = 2 * minbuffer;
   argc -= optind;
diff --git a/disobedience/disobedience.c b/disobedience/disobedience.c
index dea9d59..a4e9d20 100644
--- a/disobedience/disobedience.c
+++ b/disobedience/disobedience.c
@@ -460,7 +460,7 @@ int main(int argc, char **argv) {
   /* create the event loop */
   D(("create main loop"));
   mainloop = g_main_loop_new(0, 0);
-  if(config_read(0)) fatal(0, "cannot read configuration");
+  if(config_read(0, NULL)) fatal(0, "cannot read configuration");
   /* we'll need mixer support */
   backend = uaudio_apis[0];
   if(backend->open_mixer)
diff --git a/lib/configuration.c b/lib/configuration.c
index 1fcfb1a..dd08949 100644
--- a/lib/configuration.c
+++ b/lib/configuration.c
@@ -1156,7 +1156,7 @@ static struct config *config_default(void) {
   logname = pw->pw_name;
   c->username = xstrdup(logname);
   c->refresh = 15;
-  c->prefsync = 3600;
+  c->prefsync = 0;
   c->signal = SIGKILL;
   c->alias = xstrdup("{/artist}{/album}{/title}{ext}");
   c->lock = 1;
@@ -1326,8 +1326,14 @@ static void config_postdefaults(struct config *c,
 
 /** @brief (Re-)read the config file
  * @param server If set, do extra checking
+ * @param oldconfig Old configuration for compatibility check
+ * @return 0 on success, non-0 on error
+ *
+ * If @p oldconfig is set, then certain compatibility checks are done between
+ * the old and new configurations.
  */
-int config_read(int server) {
+int config_read(int server,
+                const struct config *oldconfig) {
   struct config *c;
   char *privconf;
   struct passwd *pw;
@@ -1362,6 +1368,31 @@ int config_read(int server) {
   }
   /* install default namepart and transform settings */
   config_postdefaults(c, server);
+  if(oldconfig)  {
+    int failed = 0;
+    if(strcmp(c->home, oldconfig->home)) {
+      error(0, "'home' cannot be changed without a restart");
+      failed = 1;
+    }
+    if(strcmp(c->alias, oldconfig->alias)) {
+      error(0, "'alias' cannot be changed without a restart");
+      failed = 1;
+    }
+    if(strcmp(c->user, oldconfig->user)) {
+      error(0, "'user' cannot be changed without a restart");
+      failed = 1;
+    }
+    if(c->nice_speaker != oldconfig->nice_speaker) {
+      error(0, "'nice_speaker' cannot be changed without a restart");
+      /* ...but we accept the new config anyway */
+    }
+    /* TODO namepart */
+    /* TODO stopword */
+    if(failed) {
+      error(0, "not installing incompatible new configuration");
+      return -1;
+    }
+  }
   /* everything is good so we shall use the new config */
   config_free(config);
   /* warn about obsolete directives */
@@ -1371,6 +1402,12 @@ int config_read(int server) {
     error(0, "'allow' will be removed in a future version");
   if(c->trust.n)
     error(0, "'trust' will be removed in a future version");
+  if(!c->lock)
+    error(0, "'lock' will be removed in a future version");
+  if(c->gap)
+    error(0, "'gap' will be removed in a future version");
+  if(c->prefsync)
+    error(0, "'prefsync' will be removed in a future version");
   config = c;
   return 0;
 }
diff --git a/lib/configuration.h b/lib/configuration.h
index 4cb5677..e9fce73 100644
--- a/lib/configuration.h
+++ b/lib/configuration.h
@@ -292,7 +292,8 @@ struct config {
 extern struct config *config;
 /* the current configuration */
 
-int config_read(int server);
+int config_read(int server,
+                const struct config *oldconfig);
 /* re-read config, return 0 on success or non-0 on error.
  * Only updates @config@ if the new configuration is valid. */
 
diff --git a/server/choose.c b/server/choose.c
index bc82475..10ed360 100644
--- a/server/choose.c
+++ b/server/choose.c
@@ -281,7 +281,7 @@ int main(int argc, char **argv) {
     openlog(progname, LOG_PID, LOG_DAEMON);
     log_default = &log_syslog;
   }
-  if(config_read(0)) fatal(0, "cannot read configuration");
+  if(config_read(0, NULL)) fatal(0, "cannot read configuration");
   /* Find out current queue/recent list */
   queue_read();
   recent_read();
diff --git a/server/dbupgrade.c b/server/dbupgrade.c
index bd08eed..09dd789 100644
--- a/server/dbupgrade.c
+++ b/server/dbupgrade.c
@@ -333,7 +333,7 @@ int main(int argc, char **argv) {
     openlog(progname, LOG_PID, LOG_DAEMON);
     log_default = &log_syslog;
   }
-  if(config_read(0)) fatal(0, "cannot read configuration");
+  if(config_read(0, NULL)) fatal(0, "cannot read configuration");
   /* Open the database */
   trackdb_init(TRACKDB_NO_RECOVER);
   trackdb_open(TRACKDB_OPEN_FOR_UPGRADE);
diff --git a/server/deadlock.c b/server/deadlock.c
index 3c13351..d90a123 100644
--- a/server/deadlock.c
+++ b/server/deadlock.c
@@ -71,7 +71,7 @@ int main(int argc, char **argv) {
     openlog(progname, LOG_PID, LOG_DAEMON);
     log_default = &log_syslog;
   }
-  if(config_read(0)) fatal(0, "cannot read configuration");
+  if(config_read(0, NULL)) fatal(0, "cannot read configuration");
   info("started");
   trackdb_init(TRACKDB_NO_RECOVER);
   while(getppid() != 1) {
diff --git a/server/disorderd.c b/server/disorderd.c
index 74fc07e..ab6221a 100644
--- a/server/disorderd.c
+++ b/server/disorderd.c
@@ -230,7 +230,7 @@ int main(int argc, char **argv) {
   if(ev_child_setup(ev)) fatal(0, "ev_child_setup failed");
   /* read config */
   config_uaudio_apis = uaudio_apis;
-  if(config_read(1))
+  if(config_read(1,  NULL))
     fatal(0, "cannot read configuration");
   /* make sure the home directory exists and has suitable permissions */
   make_home();
diff --git a/server/dump.c b/server/dump.c
index e08b32f..4b023aa 100644
--- a/server/dump.c
+++ b/server/dump.c
@@ -473,7 +473,7 @@ int main(int argc, char **argv) {
       fatal(0, "specify only a dump file name");
     path = argv[optind];
   }
-  if(config_read(0)) fatal(0, "cannot read configuration");
+  if(config_read(0, NULL)) fatal(0, "cannot read configuration");
   trackdb_init(recover|TRACKDB_MAY_CREATE);
   trackdb_open(TRACKDB_NO_UPGRADE);
   if(dump) {
diff --git a/server/normalize.c b/server/normalize.c
index db5b9d5..717f3cc 100644
--- a/server/normalize.c
+++ b/server/normalize.c
@@ -141,7 +141,7 @@ int main(int argc, char attribute((unused)) **argv) {
     default: fatal(0, "invalid option");
     }
   }
-  if(config_read(1))
+  if(config_read(1, NULL))
     fatal(0, "cannot read configuration");
   if(logsyslog) {
     openlog(progname, LOG_PID, LOG_DAEMON);
diff --git a/server/rescan.c b/server/rescan.c
index 1380e24..ebff098 100644
--- a/server/rescan.c
+++ b/server/rescan.c
@@ -380,7 +380,7 @@ int main(int argc, char **argv) {
     openlog(progname, LOG_PID, LOG_DAEMON);
     log_default = &log_syslog;
   }
-  if(config_read(0)) fatal(0, "cannot read configuration");
+  if(config_read(0, NULL)) fatal(0, "cannot read configuration");
   xnice(config->nice_rescan);
   sa.sa_handler = signal_handler;
   sa.sa_flags = SA_RESTART;
diff --git a/server/speaker.c b/server/speaker.c
index b41c94f..adda2eb 100644
--- a/server/speaker.c
+++ b/server/speaker.c
@@ -608,7 +608,7 @@ static void mainloop(void) {
 	  break;
 	case SM_RELOAD:
           D(("SM_RELOAD"));
-	  if(config_read(1))
+	  if(config_read(1, NULL))
             error(0, "cannot read configuration");
           info("reloaded configuration");
 	  break;
@@ -707,7 +707,7 @@ int main(int argc, char **argv) {
     log_default = &log_syslog;
   }
   config_uaudio_apis = uaudio_apis;
-  if(config_read(1)) fatal(0, "cannot read configuration");
+  if(config_read(1, NULL)) fatal(0, "cannot read configuration");
   /* ignore SIGPIPE */
   signal(SIGPIPE, SIG_IGN);
   /* set nice value */
diff --git a/server/state.c b/server/state.c
index a881e9b..1c1e2aa 100644
--- a/server/state.c
+++ b/server/state.c
@@ -164,7 +164,7 @@ int reconfigure(ev_source *ev, int reload) {
     /* Close the track database */
     trackdb_close();
     /* (Re-)read the configuration */
-    if(config_read(1/*server*/))
+    if(config_read(1/*server*/, config))
       ret = -1;
     else {
       /* Tell the speaker it needs to reload its config too. */
diff --git a/server/stats.c b/server/stats.c
index 81c92f4..ad35cb8 100644
--- a/server/stats.c
+++ b/server/stats.c
@@ -74,7 +74,7 @@ int main(int argc, char **argv) {
     openlog(progname, LOG_PID, LOG_DAEMON);
     log_default = &log_syslog;
   }
-  if(config_read(0))
+  if(config_read(0, NULL))
     fatal(0, "cannot read configuration");
   trackdb_init(TRACKDB_NO_RECOVER);
   trackdb_open(TRACKDB_NO_UPGRADE);
diff --git a/server/trackname.c b/server/trackname.c
index ec84f68..b661e0c 100644
--- a/server/trackname.c
+++ b/server/trackname.c
@@ -57,7 +57,7 @@ int main(int argc, char **argv) {
   }
   if(argc - optind < 3) fatal(0, "not enough arguments");
   if(argc - optind > 3) fatal(0, "too many arguments");
-  if(config_read(0)) fatal(0, "cannot read configuration");
+  if(config_read(0, NULL)) fatal(0, "cannot read configuration");
   s = trackname_part(argv[optind], argv[optind+1], argv[optind+2]);
   if(!s) fatal(0, "trackname_part returned NULL");
   xprintf("%s\n", nullcheck(utf82mb(s)));