Re-work restart function.

Restart is now always "gentle", i.e. cannot be forced and isn't possible
when dependents (with hard dependency) exist.

diff --git a/src/control.cc b/src/control.cc
index 3b72a85b285858112e9b5935e64f162926c2f612..f2dfcc77eee9c7cd95c0f50650d8af997e488b47 100644 (file)
--- a/src/control.cc
+++ b/src/control.cc
@@ -257,8 +257,8 @@ bool control_conn_t::process_start_stop(int pktType)
         case DINIT_CP_STOPSERVICE:
         {
             // force service to stop
-            bool gentle = ((rbuf[1] & 2) == 2);
             bool do_restart = ((rbuf[1] & 4) == 4);
+            bool gentle = ((rbuf[1] & 2) == 2) || do_restart;  // restart is always "gentle"
             if (do_restart && services->is_shutting_down()) {
                 ack_buf[0] = DINIT_RP_NAK;
                 break;
@@ -276,7 +276,10 @@ bool control_conn_t::process_start_stop(int pktType)
             }
             service_state_t wanted_state;
             if (do_restart) {
-                service->restart(); // TODO XXX check return, reply NAK if fail
+                if (! service->restart()) {
+                    ack_buf[0] = DINIT_RP_NAK;
+                    break;
+                }
                 wanted_state = service_state_t::STARTED;
             }
             else {
@@ -286,7 +289,7 @@ bool control_conn_t::process_start_stop(int pktType)
             }
             service->forced_stop();
             services->process_queues();
-            if (service->get_state() == wanted_state) ack_buf[0] = DINIT_RP_ALREADYSS;
+            if (service->get_state() == wanted_state && !do_restart) ack_buf[0] = DINIT_RP_ALREADYSS;
             break;
         }
         case DINIT_CP_WAKESERVICE:

diff --git a/src/includes/service.h b/src/includes/service.h
index f1d62441997fd238b2958c558d00c2df7789d4b5..7db10368118d125ff97925424edd2f2289f507c9 100644 (file)
--- a/src/includes/service.h
+++ b/src/includes/service.h
@@ -518,7 +518,7 @@ class service_record
     
     void start(bool activate = true) noexcept;  // start the service
     void stop(bool bring_down = true) noexcept;   // stop the service
-    void restart() noexcept; // restart the service
+    bool restart() noexcept; // restart the service, returns true iff restart issued
     
     void forced_stop() noexcept; // force-stop this service and all dependents
     

diff --git a/src/service.cc b/src/service.cc
index 1fc157c8c15e220d08da7d16fcb35fabaee4b333..d83a4af6d2e085d67803601b4774da00140b0b66 100644 (file)
--- a/src/service.cc
+++ b/src/service.cc
@@ -499,7 +499,7 @@ void service_record::stop(bool bring_down) noexcept
     }
 }
 
-void service_record::restart() noexcept
+bool service_record::restart() noexcept
 {
     // Re-start without affecting dependency links/activation.
 
@@ -507,7 +507,11 @@ void service_record::restart() noexcept
         restarting = true;
         stop_reason = stopped_reason_t::NORMAL;
         do_stop();
+        return true;
     }
+
+    // Wrong state
+    return false;
 }
 
 void service_record::do_stop() noexcept

diff --git a/src/tests/cptests/cptests.cc b/src/tests/cptests/cptests.cc
index 0fd1994ebd30dac4017983323d841a6753c98b35..5b204e8f8bb2469db45b96545ade1bc7aa5cebbf 100644 (file)
--- a/src/tests/cptests/cptests.cc
+++ b/src/tests/cptests/cptests.cc
@@ -9,6 +9,8 @@
 #include "baseproc-sys.h"
 #include "control.h"
 
+#include "../test_service.h"
+
 // Control protocol tests.
 
 class control_conn_t_test
@@ -901,6 +903,109 @@ void cptest_enableservice()
 
     assert(s2->get_state() == service_state_t::STOPPED);
 
+    bp_sys::extract_written_data(fd, wdata);
+
+    delete cc;
+}
+
+void cptest_restart()
+{
+    service_set sset;
+
+    const char * const service_name = "test-service-1";
+
+    test_service *s1 = new test_service(&sset, "test-service-1", service_type_t::INTERNAL, {});
+    sset.add_service(s1);
+
+    int fd = bp_sys::allocfd();
+    auto *cc = new control_conn_t(event_loop, &sset, fd);
+
+    // Get a service handle:
+    std::vector<char> cmd = { DINIT_CP_FINDSERVICE };
+    uint16_t name_len = strlen(service_name);
+    char *name_len_cptr = reinterpret_cast<char *>(&name_len);
+    cmd.insert(cmd.end(), name_len_cptr, name_len_cptr + sizeof(name_len));
+    cmd.insert(cmd.end(), service_name, service_name + name_len);
+
+    bp_sys::supply_read_data(fd, std::move(cmd));
+
+    event_loop.regd_bidi_watchers[fd]->read_ready(event_loop, fd);
+
+    // We expect:
+    // (1 byte)   DINIT_RP_SERVICERECORD
+    // (1 byte)   state
+    // (handle_t) handle
+    // (1 byte)   target state
+
+    std::vector<char> wdata;
+    bp_sys::extract_written_data(fd, wdata);
+
+    assert(wdata.size() == 3 + sizeof(control_conn_t::handle_t));
+    assert(wdata[0] == DINIT_RP_SERVICERECORD);
+    service_state_t s = static_cast<service_state_t>(wdata[1]);
+    assert(s == service_state_t::STOPPED);
+    service_state_t ts = static_cast<service_state_t>(wdata[6]);
+    assert(ts == service_state_t::STOPPED);
+
+    control_conn_t::handle_t h;
+    std::copy(wdata.data() + 2, wdata.data() + 2 + sizeof(h), reinterpret_cast<char *>(&h));
+
+    bp_sys::extract_written_data(fd, wdata); // DAV
+    assert(wdata.size() == 0);
+
+    // Issue restart:
+    cmd = { DINIT_CP_STOPSERVICE, 4 /* restart */ };
+    char * h_cp = reinterpret_cast<char *>(&h);
+    cmd.insert(cmd.end(), h_cp, h_cp + sizeof(h));
+
+    bp_sys::supply_read_data(fd, cmd);
+
+    event_loop.regd_bidi_watchers[fd]->read_ready(event_loop, fd);
+
+    bp_sys::extract_written_data(fd, wdata);
+
+    assert(wdata.size() == 1 /* NAK reply, wrong state */);
+    assert(wdata[0] == DINIT_RP_NAK);
+
+    // Start the service now:
+    s1->start();
+    sset.process_queues();
+    s1->started();
+    sset.process_queues();
+
+    bp_sys::extract_written_data(fd, wdata);
+
+    // Issue restart (again):
+    bp_sys::supply_read_data(fd, std::move(cmd));
+    event_loop.regd_bidi_watchers[fd]->read_ready(event_loop, fd);
+    bp_sys::extract_written_data(fd, wdata);
+
+    assert(wdata.size() == 7 + 1);  // info packet (service stopped) + ACK
+    assert(wdata[0] == DINIT_IP_SERVICEEVENT);
+    assert(wdata[1] == 7);
+    control_conn_t::handle_t ip_h;
+    std::copy(wdata.data() + 2, wdata.data() + 2 + sizeof(ip_h), reinterpret_cast<char *>(&ip_h));
+    assert(ip_h == h);
+    assert(wdata[6] == static_cast<int>(service_event_t::STOPPED));
+    assert(wdata[7] == DINIT_RP_ACK);
+
+    sset.process_queues();
+    assert(s1->get_state() == service_state_t::STARTING);
+
+    s1->started();
+    sset.process_queues();
+    assert(s1->get_state() == service_state_t::STARTED);
+
+    bp_sys::extract_written_data(fd, wdata);
+
+    assert(wdata.size() == 7);  /* info packet */
+    assert(wdata[0] == DINIT_IP_SERVICEEVENT);
+    // packetsize, key (handle), event
+    assert(wdata[1] == 7);
+    std::copy(wdata.data() + 2, wdata.data() + 2 + sizeof(ip_h), reinterpret_cast<char *>(&ip_h));
+    assert(ip_h == h);
+    assert(wdata[6] == static_cast<int>(service_event_t::STARTED));
+
     delete cc;
 }
 
@@ -924,5 +1029,6 @@ int main(int argc, char **argv)
     RUN_TEST(cptest_unload, "             ");
     RUN_TEST(cptest_addrmdeps, "          ");
     RUN_TEST(cptest_enableservice, "      ");
+    RUN_TEST(cptest_restart, "            ");
     return 0;
 }