This fixes static IP setting on Ethernet when using flimflam.

The main fix here is to use the ipconfig_static_path instead of
ipconfig->device_path when setting the parameters.

Also added some logging and cleaned up error messages.

Note that this only fixes static IP setting when flimflam is
enabled: this code doesn't properly talk to shill for that
operation because it still tries to create IPConfigs, which
shill doesn't implement.

BUG=chromium-os:29050
TEST=Ran on device, configured ethernet for static IP, and verified
     that it showed up in the flimflam profile.

Review URL: https://chromiumcodereview.appspot.com/10412026

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@138233 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/chromeos/cros/cros_network_functions.cc

@@ -1064,7 +1064,8 @@ bool CrosAddIPConfig(const std::string& device_path, IPConfigType type) {
         DBusThreadManager::Get()->GetFlimflamDeviceClient()->
         CallAddIPConfigAndBlock(dbus::ObjectPath(device_path), type_str);
     if (result.value().empty()) {
-      LOG(WARNING) <<"Add IPConfig failed: ";
+      LOG(ERROR) << "Add IPConfig failed for device path " << device_path
+                 << " and type " << type_str;
       return false;
     }
     return true;

chrome/browser/chromeos/cros/network_ip_config.cc

@@ -4,10 +4,30 @@
 
 #include "chrome/browser/chromeos/cros/network_ip_config.h"
 
+#include "base/logging.h"
 #include "base/string_tokenizer.h"
 
 namespace chromeos {
 
+namespace {
+#define ENUM_CASE(x) case x: return std::string(#x)
+std::string IPConfigTypeAsString(IPConfigType type) {
+  switch (type) {
+    ENUM_CASE(IPCONFIG_TYPE_UNKNOWN);
+    ENUM_CASE(IPCONFIG_TYPE_IPV4);
+    ENUM_CASE(IPCONFIG_TYPE_IPV6);
+    ENUM_CASE(IPCONFIG_TYPE_DHCP);
+    ENUM_CASE(IPCONFIG_TYPE_BOOTP);
+    ENUM_CASE(IPCONFIG_TYPE_ZEROCONF);
+    ENUM_CASE(IPCONFIG_TYPE_DHCP6);
+    ENUM_CASE(IPCONFIG_TYPE_PPP);
+  }
+  NOTREACHED() << "Unhandled enum value " << type;
+  return std::string();
+}
+#undef ENUM_CASE
+}  // namespace
+
 NetworkIPConfig::NetworkIPConfig(
     const std::string& device_path, IPConfigType type,
     const std::string& address, const std::string& netmask,
@@ -22,6 +42,15 @@ NetworkIPConfig::NetworkIPConfig(
 
 NetworkIPConfig::~NetworkIPConfig() {}
 
+std::string NetworkIPConfig::ToString() const {
+  return std::string("path: ") + device_path
+      + " type: " + IPConfigTypeAsString(type)
+      + " address: " + address
+      + " netmask: " + netmask
+      + " gateway: " + gateway
+      + " name_servers: " + name_servers;
+}
+
 int32 NetworkIPConfig::GetPrefixLength() const {
   int count = 0;
   int prefixlen = 0;

chrome/browser/chromeos/cros/network_ip_config.h

@@ -19,6 +19,8 @@ struct NetworkIPConfig {
                   const std::string& gateway, const std::string& name_servers);
   ~NetworkIPConfig();
 
+  std::string ToString() const;
+
   // Gets the PrefixLength for an IPv4 netmask.
   // For example, "255.255.255.0" => 24
   // If the netmask is invalid, this will return -1;

chrome/browser/chromeos/cros/network_library_impl_cros.cc

@@ -515,6 +515,8 @@ void NetworkLibraryImplCros::SetIPConfig(const NetworkIPConfig& ipconfig) {
   if (ipconfig.device_path.empty())
     return;
 
+  VLOG(1) << "Setting IPConfig: " << ipconfig.ToString();
+
   NetworkIPConfig* ipconfig_dhcp = NULL;
   std::string ipconfig_dhcp_path;
   NetworkIPConfig* ipconfig_static = NULL;
@@ -536,17 +538,24 @@ void NetworkLibraryImplCros::SetIPConfig(const NetworkIPConfig& ipconfig) {
   NetworkIPConfigVector ipconfigs2;
   std::vector<std::string> ipconfig_paths2;
   if (ipconfig.type == chromeos::IPCONFIG_TYPE_DHCP) {
-    // If switching from static to dhcp, create new dhcp ip config.
-    if (!ipconfig_dhcp)
-      CrosAddIPConfig(ipconfig.device_path, chromeos::IPCONFIG_TYPE_DHCP);
-    // User wants DHCP now. So delete the static ip config.
+    // If switching from static to DHCP, create new DHCP IPConfig.
+    if (!ipconfig_dhcp &&
+        !CrosAddIPConfig(ipconfig.device_path, chromeos::IPCONFIG_TYPE_DHCP)) {
+      LOG(ERROR) << "Unable to add new DHCP IPConfig";
+      return;
+    }
+    // User wants DHCP now. So delete the static IPConfig.
     if (ipconfig_static)
       CrosRemoveIPConfig(ipconfig_static_path);
   } else if (ipconfig.type == chromeos::IPCONFIG_TYPE_IPV4) {
-    // If switching from dhcp to static, create new static ip config.
+    // If switching from DHCP to static, create new static IPConfig.
     if (!ipconfig_static) {
-      CrosAddIPConfig(ipconfig.device_path, chromeos::IPCONFIG_TYPE_IPV4);
-      // Now find the newly created IP config.
+      if (!CrosAddIPConfig(ipconfig.device_path,
+                           chromeos::IPCONFIG_TYPE_IPV4)) {
+        LOG(ERROR) << "Unable to add new static IPConfig";
+        return;
+      }
+      // Now find the newly created IPConfig.
       if (CrosListIPConfigs(ipconfig.device_path, &ipconfigs2,
                             &ipconfig_paths2, NULL)) {
         for (size_t i = 0; i < ipconfigs2.size(); ++i) {
@@ -556,39 +565,42 @@ void NetworkLibraryImplCros::SetIPConfig(const NetworkIPConfig& ipconfig) {
           }
         }
       }
-    }
-    if (ipconfig_static) {
-      // Save any changed details.
-      if (ipconfig.address != ipconfig_static->address) {
-        base::StringValue value(ipconfig.address);
-        CrosSetNetworkIPConfigProperty(ipconfig_static->device_path,
-                                       flimflam::kAddressProperty, value);
-      }
-      if (ipconfig.netmask != ipconfig_static->netmask) {
-        int prefixlen = ipconfig.GetPrefixLength();
-        if (prefixlen == -1) {
-          VLOG(1) << "IP config prefixlen is invalid for netmask "
-                  << ipconfig.netmask;
-        } else {
-          base::FundamentalValue value(prefixlen);
-          CrosSetNetworkIPConfigProperty(ipconfig_static->device_path,
-                                         flimflam::kPrefixlenProperty, value);
-        }
-      }
-      if (ipconfig.gateway != ipconfig_static->gateway) {
-        base::StringValue value(ipconfig.gateway);
-        CrosSetNetworkIPConfigProperty(ipconfig_static->device_path,
-                                       flimflam::kGatewayProperty, value);
+      if (!ipconfig_static) {
+        LOG(ERROR) << "Unable to find newly added static IPConfig";
+        return;
       }
-      if (ipconfig.name_servers != ipconfig_static->name_servers) {
-        base::StringValue value(ipconfig.name_servers);
-        CrosSetNetworkIPConfigProperty(ipconfig_static->device_path,
-                                       flimflam::kNameServersProperty, value);
+    }
+
+    // Save any changed details.
+    if (ipconfig.address != ipconfig_static->address) {
+      base::StringValue value(ipconfig.address);
+      CrosSetNetworkIPConfigProperty(ipconfig_static_path,
+                                     flimflam::kAddressProperty, value);
+    }
+    if (ipconfig.netmask != ipconfig_static->netmask) {
+      int prefixlen = ipconfig.GetPrefixLength();
+      if (prefixlen == -1) {
+        VLOG(1) << "IPConfig prefix length is invalid for netmask "
+            << ipconfig.netmask;
+      } else {
+        base::FundamentalValue value(prefixlen);
+        CrosSetNetworkIPConfigProperty(ipconfig_static_path,
+                                       flimflam::kPrefixlenProperty, value);
       }
-      // Remove dhcp ip config if there is one.
-      if (ipconfig_dhcp)
-        CrosRemoveIPConfig(ipconfig_dhcp_path);
     }
+    if (ipconfig.gateway != ipconfig_static->gateway) {
+      base::StringValue value(ipconfig.gateway);
+      CrosSetNetworkIPConfigProperty(ipconfig_static_path,
+                                     flimflam::kGatewayProperty, value);
+    }
+    if (ipconfig.name_servers != ipconfig_static->name_servers) {
+      base::StringValue value(ipconfig.name_servers);
+      CrosSetNetworkIPConfigProperty(ipconfig_static_path,
+                                     flimflam::kNameServersProperty, value);
+    }
+    // Remove DHCP IPConfig if there is one.
+    if (ipconfig_dhcp)
+      CrosRemoveIPConfig(ipconfig_dhcp_path);
   }
 }