commit 728faa9171eba8350d07b7e06e44c0ae8a8f9284
author Darin Petkov <petkov@chromium.org> Fri Oct 12 11:25:47 2012 +0200
committer Gerrit <chrome-bot@google.com> Tue Oct 16 02:10:07 2012 -0700
tree 50005fe8b93d20f71702eec5c3a6e5c7acbe14d5
parent 804e8d046b87b5b3a20c4e1acc6a57fe085fe4fe

shill: openvpn: Use shill's openvpn-scipt.

Also, get rid of the CONNMAN_INTERFACE variable and rename the other
CONNMAN_ task variables to SHILL_. Raise the logging visibility of the
openvpn process options to ease debugging of field issues.

This patch completes the transition of openvpn-script from flimflam to
shill.

BUG=chromium-os:27055
TEST=Unit tests. Tested on device by connecting to OpenVPN, testing
the connection and inspecting log files to make sure the new script is
used.
CQ-DEPEND=Iffe934e44562afa85f23b269d2c5c181dc0746f3

Change-Id: Icb7f42a6137ec7064957abfe0561eca3698545b7
Reviewed-on: https://gerrit.chromium.org/gerrit/35411
Commit-Ready: Darin Petkov <petkov@chromium.org>
Reviewed-by: Darin Petkov <petkov@chromium.org>
Tested-by: Darin Petkov <petkov@chromium.org>

openvpn_driver.cc

diff --git a/openvpn_driver.cc b/openvpn_driver.cc
index a0f18c7..048401b 100644
--- a/openvpn_driver.cc
+++ b/openvpn_driver.cc
@@ -69,7 +69,7 @@
 // static
 const char OpenVPNDriver::kOpenVPNPath[] = "/usr/sbin/openvpn";
 // static
-const char OpenVPNDriver::kOpenVPNScript[] = SCRIPTDIR "/openvpn-script";
+const char OpenVPNDriver::kOpenVPNScript[] = SHIMDIR "/openvpn-script";
 // static
 const VPNDriver::Property OpenVPNDriver::kProperties[] = {
   { flimflam::kOpenVPNAuthNoCacheProperty, 0 },
@@ -205,7 +205,7 @@
   if (error.IsFailure()) {
     return false;
   }
-  SLOG(VPN, 2) << "OpenVPN process options: " << JoinString(options, ' ');
+  LOG(INFO) << "OpenVPN process options: " << JoinString(options, ' ');
 
   // TODO(petkov): This code needs to be abstracted away in a separate external
   // process module (crosbug.com/27131).
@@ -593,13 +593,10 @@
   // Setup openvpn-script options and RPC information required to send back
   // Layer 3 configuration.
   options->push_back("--setenv");
-  options->push_back("CONNMAN_BUSNAME");
+  options->push_back(kRPCTaskServiceVariable);
   options->push_back(rpc_task_->GetRpcConnectionIdentifier());
   options->push_back("--setenv");
-  options->push_back("CONNMAN_INTERFACE");
-  options->push_back(rpc_task_->GetRpcInterfaceIdentifier());
-  options->push_back("--setenv");
-  options->push_back("CONNMAN_PATH");
+  options->push_back(kRPCTaskPathVariable);
   options->push_back(rpc_task_->GetRpcIdentifier());
   options->push_back("--script-security");
   options->push_back("2");

openvpn_driver_unittest.cc

diff --git a/openvpn_driver_unittest.cc b/openvpn_driver_unittest.cc
index e64f7c3..eff0db8 100644
--- a/openvpn_driver_unittest.cc
+++ b/openvpn_driver_unittest.cc
@@ -485,7 +485,7 @@
   EXPECT_TRUE(error.IsSuccess());
   EXPECT_EQ("--client", options[0]);
   ExpectInFlags(options, "--remote", kHost);
-  ExpectInFlags(options, "CONNMAN_PATH", RPCTaskMockAdaptor::kRpcId);
+  ExpectInFlags(options, kRPCTaskPathVariable, RPCTaskMockAdaptor::kRpcId);
   ExpectInFlags(options, "--dev", kInterfaceName);
   ExpectInFlags(options, "--group", "openvpn");
   EXPECT_EQ(kInterfaceName, driver_->tunnel_interface_);

rpc_task.h

diff --git a/rpc_task.h b/rpc_task.h
index 48951e9..5d8a264 100644
--- a/rpc_task.h
+++ b/rpc_task.h
@@ -13,6 +13,10 @@
 
 namespace shill {
 
+// Declared in the header to avoid linking unused code into shims.
+static const char kRPCTaskServiceVariable[] = "SHILL_TASK_SERVICE";
+static const char kRPCTaskPathVariable[] = "SHILL_TASK_PATH";
+
 class ControlInterface;
 class RPCTaskAdaptorInterface;
 

shims/openvpn_script.cc

diff --git a/shims/openvpn_script.cc b/shims/openvpn_script.cc
index 041ded8..3fbb5e1 100644
--- a/shims/openvpn_script.cc
+++ b/shims/openvpn_script.cc
@@ -11,11 +11,11 @@
 #include <chromeos/syslog_logging.h>
 #include <dbus-c++/eventloop-integration.h>
 
+#include "shill/rpc_task.h"
 #include "shill/shims/environment.h"
 #include "shill/shims/task_proxy.h"
 
 using shill::shims::Environment;
-using shill::shims::TaskProxy;
 using std::map;
 using std::string;
 
@@ -26,10 +26,8 @@
 
   Environment *environment = Environment::GetInstance();
   string service, path, reason;
-  // TODO(petkov): When switching shill to use this shim, rename the variables
-  // and maybe use shared kConsts. Also, get rid of CONNMAN_INTERFACE.
-  if (!environment->GetVariable("CONNMAN_BUSNAME", &service) ||
-      !environment->GetVariable("CONNMAN_PATH", &path) ||
+  if (!environment->GetVariable(shill::kRPCTaskServiceVariable, &service) ||
+      !environment->GetVariable(shill::kRPCTaskPathVariable, &path) ||
       !environment->GetVariable("script_type", &reason)) {
     LOG(ERROR) << "Environment variables not available.";
     return EXIT_FAILURE;
@@ -38,7 +36,7 @@
   DBus::BusDispatcher dispatcher;
   DBus::default_dispatcher = &dispatcher;
   DBus::Connection connection(DBus::Connection::SystemBus());
-  TaskProxy proxy(&connection, path, service);
+  shill::shims::TaskProxy proxy(&connection, path, service);
   map<string, string> env = environment->AsMap();
   proxy.Notify(reason, env);
   return EXIT_SUCCESS;