Browse Source

Several changes, centered around improving logging and handling
read-only initial filesystem.

* introduce basic logging functions
* allow "onstart" commands to specify if console output should
stop and/or if control socket should be opened after a service
starts
* fix a bug in load_service / service constructor interaction,
caused services with arguments to fail

Davin McCall 8 years ago
parent
commit
d4a258fb46
7 changed files with 232 additions and 88 deletions
  1. 2 2
      Makefile
  2. 36 0
      dinit-log.cc
  3. 44 0
      dinit-log.h
  4. 69 50
      dinit.cc
  5. 21 25
      load_service.cc
  6. 16 1
      service.cc
  7. 44 10
      service.h

+ 2 - 2
Makefile

@@ -1,8 +1,8 @@
 -include mconfig
 
-objects = dinit.o load_service.o service.o dinit-start.o
+objects = dinit.o load_service.o service.o dinit-log.o dinit-start.o
 
-dinit_objects = dinit.o load_service.o service.o
+dinit_objects = dinit.o load_service.o service.o dinit-log.o
 
 all: dinit dinit-start
 

+ 36 - 0
dinit-log.cc

@@ -0,0 +1,36 @@
+#include <iostream>
+#include "dinit-log.h"
+
+LogLevel log_level = LogLevel::WARN;
+bool log_to_console = true;    // whether we should output log messages to console
+
+// Log a message
+void log(LogLevel lvl, const char *msg) noexcept
+{
+    if (lvl >= log_level) {
+        if (log_to_console) {
+            std::cout << "dinit: " << msg << std::endl;
+        }
+    }
+}
+
+void logServiceStarted(const char *service_name) noexcept
+{
+    if (log_to_console) {
+        std::cout << "[  OK  ] " << service_name << std::endl;
+    }
+}
+
+void logServiceFailed(const char *service_name) noexcept
+{
+    if (log_to_console) {
+        std::cout << "[FAILED] " << service_name << std::endl;
+    }
+}
+
+void logServiceStopped(const char *service_name) noexcept
+{
+    if (log_to_console) {
+        std::cout << "[STOPPED] " << service_name << std::endl;
+    }
+}

+ 44 - 0
dinit-log.h

@@ -0,0 +1,44 @@
+#ifndef DINIT_LOG_H
+#define DINIT_LOG_H
+
+// Logging for Dinit
+
+#include <string>
+
+enum class LogLevel {
+    DEBUG,
+    INFO,
+    WARN,
+    ERROR,
+    ZERO    // log absolutely nothing
+};
+
+extern LogLevel log_level;
+extern bool log_to_console;
+
+void log(LogLevel lvl, const char *msg) noexcept;
+void logServiceStarted(const char *service_name) noexcept;
+void logServiceFailed(const char *service_name) noexcept;
+void logServiceStopped(const char *service_name) noexcept;
+
+static inline void log(LogLevel lvl, const std::string &str)
+{
+    log(lvl, str.c_str());
+}
+
+static inline void logServiceStarted(const std::string &str)
+{
+    logServiceStarted(str.c_str());
+}
+
+static inline void logServiceFailed(const std::string &str)
+{
+    logServiceFailed(str.c_str());
+}
+
+static inline void logServiceStopped(const std::string &str)
+{
+    logServiceStopped(str.c_str());
+}
+
+#endif

+ 69 - 50
dinit.cc

@@ -1,7 +1,8 @@
 #include <iostream>
+#include <list>
 #include <cstring>
 #include <csignal>
-#include <list>
+#include <cstddef>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/un.h>
@@ -11,7 +12,7 @@
 #include "service.h"
 #include "ev++.h"
 #include "control.h"
-
+#include "dinit-log.h"
 
 /* TODO: prevent services from respawning too quickly */
 /* TODO: detect/guard against dependency cycles */
@@ -47,8 +48,8 @@
  * SIGINT - (ctrl+alt+del handler) - fork & exec "reboot"
  * 
  * On the contrary dinit currently uses:
- * SIGTERM - roll back services and then exec /sbin/halt
- * SIGINT - roll back services and then exec /sbin/reboot
+ * SIGTERM - roll back services and then fork/exec /sbin/halt
+ * SIGINT - roll back services and then fork/exec /sbin/reboot
  *
  * It's an open question about whether dinit should roll back services *before*
  * running halt/reboot, since those commands should prompt rollback of services
@@ -63,11 +64,13 @@ static ServiceSet *service_set;
 static bool am_system_init = false; // true if we are the system init process
 static bool reboot = false; // whether to reboot (instead of halting)
 
+static bool control_socket_open = false;
+
 static void sigint_reboot_cb(struct ev_loop *loop, ev_signal *w, int revents);
 static void sigquit_cb(struct ev_loop *loop, ev_signal *w, int revents);
 static void sigterm_cb(struct ev_loop *loop, ev_signal *w, int revents);
 
-static void open_control_socket(struct ev_loop *loop);
+void open_control_socket(struct ev_loop *loop);
 
 struct ev_io control_socket_io;
 
@@ -103,9 +106,9 @@ int main(int argc, char **argv)
     
     // Arguments, if given, specify a list of services to start.
     // If we are running as init (PID=1), the kernel gives us any command line
-    // arguments it was given but didn't recognize, including "single" (usual
+    // arguments it was given but didn't recognize, including "single" (usually
     // for "boot to single user mode" aka just start the shell). We can treat
-    // them as services. In the worst case we can't find any of the named
+    // them as service names. In the worst case we can't find any of the named
     // services, and so we'll start the "boot" service by default.
     if (argc > 1) {
       for (int i = 1; i < argc; i++) {
@@ -189,12 +192,12 @@ int main(int argc, char **argv)
             service_set->startService(*i);
         }
         catch (ServiceNotFound &snf) {
-            // TODO log this better
-            cerr << "Could not find service description: " << snf.serviceName << endl;
+            log(LogLevel::ERROR, "Could not find service description: " + snf.serviceName);
+            // TODO catch bad_alloc
         }
         catch (ServiceLoadExc &sle) {
-            // TODO log this better
-            cerr << "Problem loading service description: " << sle.serviceName << endl;
+            log(LogLevel::ERROR, "Problem loading service description: " + sle.serviceName);
+            // TODO catch bad_alloc
         }
     }
     
@@ -206,8 +209,7 @@ int main(int argc, char **argv)
     }
     
     if (am_system_init) {
-        // TODO log this output properly
-        cout << "dinit: No more active services.";
+        log(LogLevel::INFO, " No more active services.");
         if (reboot) {
             cout << " Will reboot.";
         }
@@ -244,8 +246,8 @@ int main(int argc, char **argv)
                 goto event_loop; // yes, the "evil" goto
             }
             catch (...) {
-                // TODO catch exceptions and log message as appropriate
                 // Now WTF do we do? try and reboot
+                log(LogLevel::ERROR, "Could not start 'boot' service; rebooting.");
                 if (fork() == 0) {
                     execl("/sbin/reboot", "/sbin/reboot", (char *) 0);
                 }
@@ -270,9 +272,9 @@ static void control_socket_cb(struct ev_loop *loop, ev_io *w, int revents)
 
     // Accept a connection
     int sockfd = w->fd;
-    
+
     int newfd = accept4(sockfd, nullptr, nullptr, SOCK_NONBLOCK | SOCK_CLOEXEC);
-    
+
     if (newfd != -1) {    
         new ControlConn(loop, service_set, newfd);  // will delete itself when it's finished
         // TODO keep a set of control connections so that we can close them when
@@ -280,50 +282,64 @@ static void control_socket_cb(struct ev_loop *loop, ev_io *w, int revents)
     }
 }
 
-static void open_control_socket(struct ev_loop *loop)
+void open_control_socket(struct ev_loop *loop)
 {
-    // TODO make this use a per-user address if PID != 1, and make the address
-    // overridable from the command line
-    
-    const char * saddrname = "/dev/dinitctl";
-    struct sockaddr_un name;
+    if (! control_socket_open) {
+        // TODO make this use a per-user address if PID != 1, and make the address
+        // overridable from the command line
 
-    unlink(saddrname);
+        const char * saddrname = "/dev/dinitctl";
+        struct sockaddr_un name;
 
-    name.sun_family = AF_UNIX;
-    strcpy(name.sun_path, saddrname); // TODO make this safe for long names
-    int namelen = 2 + strlen(saddrname);
-    //int namelen = sizeof(name);
-    
-    int sockfd = socket(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK | SOCK_CLOEXEC, 0);
-    if (sockfd == -1) {
-        // TODO log error
-        perror("socket");
-        return;
-    }
-    
-    if (bind(sockfd, (struct sockaddr *) &name, namelen) == -1) {
-        // TODO log error
-        perror("bind");
-        close(sockfd);
-        return;
-    }
-    
-    if (listen(sockfd, 10) == -1) {
-        // TODO log error
-        perror("listen");
-        close(sockfd);
-        return;
+        if (am_system_init) {
+            unlink(saddrname);
+        }
+
+        name.sun_family = AF_UNIX;
+        strcpy(name.sun_path, saddrname); // TODO make this safe for long names
+        int namelen = offsetof(struct sockaddr_un, sun_path) + 1 + strlen(saddrname);
+
+        int sockfd = socket(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK | SOCK_CLOEXEC, 0);
+        if (sockfd == -1) {
+            log(LogLevel::ERROR, std::string("Error creating control socket: ") + strerror(errno));
+            // TODO catch/prevent bad_alloc
+            return;
+        }
+
+        if (bind(sockfd, (struct sockaddr *) &name, namelen) == -1) {
+            log(LogLevel::ERROR, std::string("Error binding control socket: ") + strerror(errno));
+            // TODO catch/prevent bad_alloc
+            close(sockfd);
+            return;
+        }
+
+        // No connections can be made until we listen, so it is fine to change the permissions now
+        // (and anyway there is no way to atomically create the socket and set permissions):
+        if (chmod(saddrname, S_IRUSR | S_IWUSR) == -1) {
+            log(LogLevel::ERROR, std::string("Error setting control socket permissions: ") + strerror(errno));
+            // TODO catch/prevent bad_alloc
+            close(sockfd);
+            return;
+        }
+
+        if (listen(sockfd, 10) == -1) {
+            log(LogLevel::ERROR, std::string("Error listening on control socket: ") + strerror(errno));
+            // TODO catch/prevent bad_alloc
+            close(sockfd);
+            return;
+        }
+
+        control_socket_open = true;
+        ev_io_init(&control_socket_io, control_socket_cb, sockfd, EV_READ);
+        ev_io_start(loop, &control_socket_io);
     }
-    
-    ev_io_init(&control_socket_io, control_socket_cb, sockfd, EV_READ);
-    ev_io_start(loop, &control_socket_io);
 }
 
 /* handle SIGINT signal (generated by kernel when ctrl+alt+del pressed) */
 static void sigint_reboot_cb(struct ev_loop *loop, ev_signal *w, int revents)
 {
     reboot = true;
+    log_to_console = true;
     service_set->stop_all_services();
 }
 
@@ -334,11 +350,14 @@ static void sigquit_cb(struct ev_loop *loop, ev_signal *w, int revents)
     // unlinked. In that case the kernel holds the binary open, so that it can't be
     // properly removed.
     execl("/sbin/shutdown", "/sbin/shutdown", (char *) 0);
+    log(LogLevel::ERROR, std::string("Error executing /sbin/shutdown: ") + strerror(errno));
+    // TODO catch/prevent bad_alloc
 }
 
 /* handle SIGTERM - stop all services */
 static void sigterm_cb(struct ev_loop *loop, ev_signal *w, int revents)
 {
     got_sigterm = true;
+    log_to_console = true;
     service_set->stop_all_services();
 }

+ 21 - 25
load_service.cc

@@ -162,24 +162,6 @@ static string read_setting_value(string_iterator & i, string_iterator end,
 }
 
 
-// Given a string and a list of pairs of (start,end) indices for each argument in that string,
-// store a null terminator for the argument. Return a `char *` array pointing at the beginning
-// of each argument. (The returned array is invalidated if the string is later modified).
-static const char ** separate_args(string &s, std::list<std::pair<unsigned,unsigned>> &arg_indices)
-{
-    const char * cstr = s.c_str();
-    const char ** r = new const char *[arg_indices.size()];
-    int i = 0;
-    for (auto index_pair : arg_indices) {
-        r[i] = cstr + index_pair.first;
-        if (index_pair.second < s.length()) {
-            s[index_pair.second] = 0;
-        }
-        i++;
-    }
-    return r;
-}
-
 // Find a service record, or load it from file. If the service has
 // dependencies, load those also.
 //
@@ -212,13 +194,13 @@ ServiceRecord * ServiceSet::loadServiceRecord(const char * name)
     service_filename += name;
     
     string command;
-    const char ** commands = nullptr;
-    int num_args = 0;
+    std::list<std::pair<unsigned,unsigned>> command_offsets;
 
     ServiceType service_type = ServiceType::PROCESS;
     std::list<ServiceRecord *> depends_on;
     std::list<ServiceRecord *> depends_soft;
     string logfile;
+    OnstartFlags onstart_flags = {false, false};
     
     string line;
     bool auto_restart = false;
@@ -257,10 +239,7 @@ ServiceRecord * ServiceSet::loadServiceRecord(const char * name)
             i = skipws(++i, end);
             
             if (setting == "command") {
-                std::list<std::pair<unsigned,unsigned>> indices;
-                command = read_setting_value(i, end, &indices);
-                commands = separate_args(command, indices);
-                num_args = indices.size();
+                command = read_setting_value(i, end, &command_offsets);
             }
             else if (setting == "depends-on") {
                 string dependency_name = read_setting_value(i, end);
@@ -293,6 +272,22 @@ ServiceRecord * ServiceSet::loadServiceRecord(const char * name)
                         " or \"process\" or \"internal\"");
                 }
             }
+            else if (setting == "onstart") {
+                std::list<std::pair<unsigned,unsigned>> indices;
+                string onstart_cmds = read_setting_value(i, end, &indices);
+                for (auto indexpair : indices) {
+                    string onstart_cmd = onstart_cmds.substr(indexpair.first, indexpair.second - indexpair.first);
+                    if (onstart_cmd == "release_console") {
+                        onstart_flags.release_console = true;
+                    }
+                    else if (onstart_cmd == "rw_ready") {
+                        onstart_flags.rw_ready = true;
+                    }
+                    else {
+                        throw new ServiceDescriptionExc(name, "Unknown onstart command: " + onstart_cmd);
+                    }
+                }
+            }
             else {
                 throw ServiceDescriptionExc(name, "Unknown setting: " + setting);
             }
@@ -307,10 +302,11 @@ ServiceRecord * ServiceSet::loadServiceRecord(const char * name)
         if (*iter == rval) {
             // We've found the dummy record
             delete rval;
-            rval = new ServiceRecord(this, string(name), service_type, command, commands, num_args,
+            rval = new ServiceRecord(this, string(name), service_type, std::move(command), command_offsets,
                     & depends_on, & depends_soft);
             rval->setLogfile(logfile);
             rval->setAutoRestart(auto_restart);
+            rval->setOnstartFlags(onstart_flags);
             *iter = rval;
             break;
         }

+ 16 - 1
service.cc

@@ -1,4 +1,3 @@
-#include "service.h"
 #include <cstring>
 #include <cerrno>
 #include <sstream>
@@ -8,6 +7,11 @@
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <unistd.h>
+#include "service.h"
+#include "dinit-log.h"
+
+// from dinit.cc:
+void open_control_socket(struct ev_loop *loop);
 
 
 // Find the requested service by name
@@ -48,6 +52,7 @@ void ServiceSet::stopService(const std::string & name)
 // Called when a service has actually stopped.
 void ServiceRecord::stopped()
 {
+    logServiceStopped(service_name);
     service_state = ServiceState::STOPPED;
     force_stop = false;
     
@@ -208,9 +213,18 @@ void ServiceRecord::start()
 
 void ServiceRecord::started()
 {
+    logServiceStarted(service_name);
     service_state = ServiceState::STARTED;
     // TODO - inform listeners
 
+    if (onstart_flags.release_console) {
+        log_to_console = false;
+    }
+
+    if (onstart_flags.rw_ready) {
+        open_control_socket(ev_default_loop(EVFLAG_AUTO));
+    }
+
     if (desired_state == ServiceState::STARTED) {
         // Start any dependents whose desired state is STARTED:
         for (auto i = dependents.begin(); i != dependents.end(); i++) {
@@ -231,6 +245,7 @@ void ServiceRecord::started()
 
 void ServiceRecord::failed_to_start()
 {
+    logServiceFailed(service_name);
     service_state = ServiceState::STOPPED;
     desired_state = ServiceState::STOPPED;
     service_set->service_inactive(this);

+ 44 - 10
service.h

@@ -44,6 +44,11 @@ enum class ServiceType {
 };
 
 
+struct OnstartFlags {
+    bool release_console : 1;
+    bool rw_ready : 1;
+};
+
 // Exception while loading a service
 class ServiceLoadExc
 {
@@ -93,26 +98,48 @@ class ServiceDep
 {
     ServiceRecord * from;
     ServiceRecord * to;
-    
+
     public:
     /* Whether the 'from' service is waiting for the 'to' service to start */
     bool waiting_on;
-    
+
     ServiceDep(ServiceRecord * from, ServiceRecord * to) : from(from), to(to), waiting_on(false)
     {  }
-    
+
     ServiceRecord * getFrom()
     {
         return from;
     }
-    
+
     ServiceRecord * getTo()
     {
         return to;
     }
 };
 
+// Given a string and a list of pairs of (start,end) indices for each argument in that string,
+// store a null terminator for the argument. Return a `char *` array pointing at the beginning
+// of each argument. (The returned array is invalidated if the string is later modified).
+static const char ** separate_args(std::string &s, std::list<std::pair<unsigned,unsigned>> &arg_indices)
+{
+    const char ** r = new const char *[arg_indices.size()];
+    int i = 0;
 
+    // First store nul terminator for each part:
+    for (auto index_pair : arg_indices) {
+        if (index_pair.second < s.length()) {
+            s[index_pair.second] = 0;
+        }
+    }
+
+    // Now we can get the C string (c_str) and store offsets into it:
+    const char * cstr = s.c_str();
+    for (auto index_pair : arg_indices) {
+        r[i] = cstr + index_pair.first;
+        i++;
+    }
+    return r;
+}
 
 class ServiceRecord
 {
@@ -129,6 +156,7 @@ class ServiceRecord
     string program_name;          /* storage for program/script and arguments */
     const char **exec_arg_parts;  /* pointer to each argument/part of the program_name */
     int num_args;                 /* number of argumrnets (including program) */
+    OnstartFlags onstart_flags;
 
     string logfile; /* log file name, empty string specifies /dev/null */
     bool auto_restart; /* whether to restart this (process) if it dies unexpectedly */
@@ -207,8 +235,8 @@ class ServiceRecord
         service_type = ServiceType::DUMMY;
     }
     
-    ServiceRecord(ServiceSet *set, string name, ServiceType service_type, string command, const char ** commands,
-            int num_argsx, sr_list * pdepends_on, sr_list * pdepends_soft)
+    ServiceRecord(ServiceSet *set, string name, ServiceType service_type, string &&command, std::list<std::pair<unsigned,unsigned>> &command_offsets,
+            sr_list * pdepends_on, sr_list * pdepends_soft)
         : service_state(ServiceState::STOPPED), desired_state(ServiceState::STOPPED), force_stop(false), auto_restart(false)
     {
         service_set = set;
@@ -217,13 +245,13 @@ class ServiceRecord
         this->depends_on = std::move(*pdepends_on);
 
         program_name = command;
-        exec_arg_parts = commands;
-        num_args = num_argsx;
+        exec_arg_parts = separate_args(program_name, command_offsets);
+        num_args = command_offsets.size();
 
-        for (sr_iter i = pdepends_on->begin(); i != pdepends_on->end(); ++i) {
+        for (sr_iter i = depends_on.begin(); i != depends_on.end(); ++i) {
             (*i)->dependents.push_back(this);
         }
-        
+
         // Soft dependencies
         auto b_iter = soft_deps.end();
         for (sr_iter i = pdepends_soft->begin(); i != pdepends_soft->end(); ++i) {
@@ -247,6 +275,12 @@ class ServiceRecord
         this->auto_restart = auto_restart;
     }
     
+    // Set "on start" flags (commands)
+    void setOnstartFlags(OnstartFlags flags)
+    {
+        this->onstart_flags = flags;
+    }
+
     const char *getServiceName() const { return service_name.c_str(); }
     ServiceState getState() const { return service_state; }