vpn-manager: Add l2tp/ipsec vpn manager

service_manager.cc

+// Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "vpn-manager/service_manager.h"
+
+#include <stdlib.h>
+
+#include "base/eintr_wrapper.h"
+#include "base/file_util.h"
+#include "base/logging.h"
+#include "base/string_util.h"
+
+FilePath ServiceManager::temp_path_;
+
+const char kTempDirectory[] = "/home/chronos/user/tmp/l2tpipsec";
+
+ServiceManager::ServiceManager(const std::string& service_name)
+    : is_running_(false), was_stopped_(false), inner_service_(NULL),

[petkov, 2011/03/04 18:42:56]

each initializer should be on a separate line, no?

[kmixter1, 2011/03/05 02:48:59]

Done.

+      outer_service_(NULL), service_name_(service_name) {
+}
+
+ServiceManager::~ServiceManager() {
+}
+
+void ServiceManager::OnStarted() {
+  is_running_ = true;

[petkov, 2011/03/04 18:42:56]

CHECK(!is_running_ && !was_stopped)?

[kmixter1, 2011/03/05 02:48:59]

Done.

+  if (inner_service_ == NULL)
+    return;
+
+  LOG(INFO) << "Starting inner " << inner_service_->service_name();
+  if (!inner_service_->Start()) {

[petkov, 2011/03/04 18:42:56]

Hmm, so you always start the chain from the outermost service and stop be the
chain from the innermost service? It seems that you need checks and docstrings
that this is the intended mechanism.

[kmixter1, 2011/03/05 02:48:59]

Yes - start from the outermost and stop from the innermost that's already
started.  Added docstrings and extra checks.

+    // Inner service could not be started, stop this layer.
+    LOG(ERROR) << "Inner service " << inner_service_->service_name()
+               << " failed.  Stopping " << service_name();
+    Stop();
+  }
+}
+
+void ServiceManager::OnStopped(bool was_error) {
+  is_running_ = false;

[petkov, 2011/03/04 18:42:56]

CHECK(is_running && !was_stopped)?

[kmixter1, 2011/03/05 02:48:59]

It's ok to stop an already stopped service, see new docstring.

+  was_stopped_ = true;
+  if (outer_service_ != NULL) {
+    outer_service_->Stop();
+  }
+}
+
+void ServiceManager::Initialize() {
+  if (temp_path_.value().empty()) {
+    temp_path_ = FilePath(kTempDirectory);
+  }
+  FilePath base = temp_path_.DirName();

[petkov, 2011/03/04 18:42:56]

why do you split it into base and the rest? CreateDirectory will create all
parents for you, if necessary, right?

[kmixter1, 2011/03/05 02:48:59]

No kidding?  I didn't know it was so kind.  See simpler code.

+  if (!file_util::DirectoryExists(base)) {
+    CHECK(file_util::CreateDirectory(base))
+        << "Unable to create " << base.value();
+  }
+  if (!file_util::DirectoryExists(temp_path_)) {
+    CHECK(file_util::CreateDirectory(temp_path_))
+        << "Unable to create " << temp_path_.value();
+  }
+  atexit(ServiceManager::DeleteTemp);
+}
+
+void ServiceManager::DeleteTemp() {
+  file_util::Delete(temp_path_, true);
+}
+
+void ServiceManager::WriteFdToSyslog(int fd, const char* prefix) {
+  char buffer[256];
+  // Note that short reads and reads of over sizeof(buffer) amount of
+  // data are going to split into different syslog lines.  Seems ok

[petkov, 2011/03/04 18:42:56]

how is this ensured, by multiple calls to this method? don't you need a while
loop or something?

[kmixter1, 2011/03/05 02:48:59]

Right now the code does arbitrarily split lines.  I didn't think it would happen
that often which is why I said it was ok for now.  It actually does happen quite
often and I plan to fix this before submitting... stay tuned.

+  // for now.
+  int written = HANDLE_EINTR(read(fd, &buffer, sizeof(buffer) - 1));
+  if (written < 0) {
+    LOG(WARNING) << "Error condition on " << prefix << " pipe";
+    return;
+  }
+  buffer[written] = '\0';
+  std::string trimmed_contents;
+  TrimWhitespaceASCII(buffer, TRIM_TRAILING, &trimmed_contents);
+  std::vector<std::string> lines;
+  SplitString(trimmed_contents, '\n', &lines);
+  for (size_t i = 0; i < lines.size(); ++i)
+    LOG(INFO) << prefix << lines[i];
+}