lxcpp: fix network config serialization

[Feature/Bug]   addr not serialized at all, type as int
[Cause]         no json support for the types
[Solution]      now json supports enums, added support for uint8_t type
[Verification]  Build, install and run tests

Change-Id: I09ec80e3ad143ccf9574c51d4f774c21b498a2aa

diff --git a/libs/config/from-json-visitor.hpp b/libs/config/from-json-visitor.hpp
index b599084..23f5369 100644 (file)
--- a/libs/config/from-json-visitor.hpp
+++ b/libs/config/from-json-visitor.hpp
@@ -87,6 +87,16 @@ private:
         }
     }
 
+    static void fromJsonObject(json_object* object, std::uint8_t& value)
+    {
+        checkType(object, json_type_int);
+        std::int64_t value64 = json_object_get_int64(object);
+        if (value64 > UINT8_MAX || value64 < 0) {
+            throw ConfigException("Value out of range");
+        }
+        value = static_cast<std::int8_t>(value64);
+    }
+
     static void fromJsonObject(json_object* object, std::int32_t& value)
     {
         checkType(object, json_type_int);

diff --git a/libs/config/manager.hpp b/libs/config/manager.hpp
index d1df907..447a204 100644 (file)
--- a/libs/config/manager.hpp
+++ b/libs/config/manager.hpp
@@ -102,6 +102,7 @@
 #include "config/from-fdstore-visitor.hpp"
 #include "config/fs-utils.hpp"
 #include "config/is-union.hpp"
+#include "logger/logger.hpp"
 
 namespace config {
 
@@ -180,12 +181,16 @@ void loadFromJsonFile(const std::string& filename, Config& config)
 {
     std::string content;
     if (!fsutils::readFileContent(filename, content)) {
-        throw ConfigException("Could not load " + filename);
+        const std::string& msg = "Could not load " + filename;
+        LOGE(msg);
+        throw ConfigException(msg);
     }
     try {
         loadFromJsonString(content, config);
     } catch (ConfigException& e) {
-        throw ConfigException("Error in " + filename + ": " + e.what());
+        const std::string& msg = "Error in " + filename + ": " + e.what();
+        LOGE(msg);
+        throw ConfigException(msg);
     }
 }
 

diff --git a/libs/config/to-json-visitor.hpp b/libs/config/to-json-visitor.hpp
index ba0d6ed..b8a4f1d 100644 (file)
--- a/libs/config/to-json-visitor.hpp
+++ b/libs/config/to-json-visitor.hpp
@@ -79,6 +79,11 @@ private:
         return ret;
     }
 
+    static json_object* toJsonObject(std::uint8_t value)
+    {
+        return json_object_new_int(value);
+    }
+
     static json_object* toJsonObject(std::int32_t value)
     {
         return json_object_new_int(value);

diff --git a/libs/lxcpp/network.cpp b/libs/lxcpp/network.cpp
index 7b9c0b3..58a86a5 100644 (file)
--- a/libs/lxcpp/network.cpp
+++ b/libs/lxcpp/network.cpp
@@ -127,9 +127,9 @@ void getAddressList(pid_t pid, std::vector<InetAddr>& addrs, int family, const s
         if (addrmsg.ifa_index == index) {
             InetAddr a;
             if (addrmsg.ifa_family == AF_INET6) {
-                a.setType(InetAddrType::IPV6);
+                a.type = InetAddrType::IPV6;
             } else if (addrmsg.ifa_family == AF_INET) {
-                a.setType(InetAddrType::IPV4);
+                a.type = InetAddrType::IPV4;
             } else {
                 const std::string msg = "Unsupported inet family";
                 LOGE(msg);
@@ -265,10 +265,10 @@ void fromString(const std::string& s, in6_addr& addr)
 
 std::string toString(const InetAddr& a) {
     std::string opts = "/" + std::to_string(a.prefix);
-    if (a.getType() == InetAddrType::IPV6) {
+    if (a.type == InetAddrType::IPV6) {
         return toString(a.getAddr<in6_addr>()) + opts;
     }
-    if (a.getType() == InetAddrType::IPV4) {
+    if (a.type == InetAddrType::IPV4) {
         return toString(a.getAddr<in_addr>()) + opts;
     }
     return "";
@@ -279,10 +279,10 @@ InetAddr::InetAddr(const std::string& a, unsigned p, uint32_t f) :
     flags(f)
 {
     if (a.find(":") != std::string::npos) {
-        setType(InetAddrType::IPV6);
+        type = InetAddrType::IPV6;
         fromString(a, getAddr<in6_addr>());
     } else {
-        setType(InetAddrType::IPV4);
+        type = InetAddrType::IPV4;
         fromString(a, getAddr<in_addr>());
     }
 }
@@ -551,15 +551,15 @@ void NetworkInterface::addInetAddr(const InetAddr& addr)
     NetlinkMessage nlm(RTM_NEWADDR, NLM_F_CREATE | NLM_F_REQUEST | NLM_F_ACK);
     ifaddrmsg infoAddr = utils::make_clean<ifaddrmsg>();
     infoAddr.ifa_index = getInterfaceIndex(mContainerPid, mIfname);
-    infoAddr.ifa_family = addr.getType() == InetAddrType::IPV4 ? AF_INET : AF_INET6;
+    infoAddr.ifa_family = addr.type == InetAddrType::IPV4 ? AF_INET : AF_INET6;
     infoAddr.ifa_prefixlen = addr.prefix;
     infoAddr.ifa_flags = addr.flags;
     nlm.put(infoAddr);
 
-    if (addr.getType() == InetAddrType::IPV6) {
+    if (addr.type == InetAddrType::IPV6) {
         nlm.put(IFA_ADDRESS, addr.getAddr<in6_addr>());
         nlm.put(IFA_LOCAL, addr.getAddr<in6_addr>());
-    } else if (addr.getType() == InetAddrType::IPV4) {
+    } else if (addr.type == InetAddrType::IPV4) {
         nlm.put(IFA_ADDRESS, addr.getAddr<in_addr>());
         nlm.put(IFA_LOCAL, addr.getAddr<in_addr>());
     }
@@ -572,15 +572,15 @@ void NetworkInterface::delInetAddr(const InetAddr& addr)
     NetlinkMessage nlm(RTM_DELADDR, NLM_F_REQUEST | NLM_F_ACK);
     ifaddrmsg infoAddr = utils::make_clean<ifaddrmsg>();
     infoAddr.ifa_index = getInterfaceIndex(mContainerPid, mIfname);
-    infoAddr.ifa_family = addr.getType() == InetAddrType::IPV4 ? AF_INET : AF_INET6;
+    infoAddr.ifa_family = addr.type == InetAddrType::IPV4 ? AF_INET : AF_INET6;
     infoAddr.ifa_prefixlen = addr.prefix;
     infoAddr.ifa_flags = addr.flags;
     nlm.put(infoAddr);
 
-    if (addr.getType() == InetAddrType::IPV6) {
+    if (addr.type == InetAddrType::IPV6) {
         nlm.put(IFA_ADDRESS, addr.getAddr<in6_addr>());
         nlm.put(IFA_LOCAL, addr.getAddr<in6_addr>());
-    } else if (addr.getType() == InetAddrType::IPV4) {
+    } else if (addr.type == InetAddrType::IPV4) {
         nlm.put(IFA_ADDRESS, addr.getAddr<in_addr>());
         nlm.put(IFA_LOCAL, addr.getAddr<in_addr>());
     }
@@ -632,8 +632,8 @@ static RoutingTable getRoutingTable(unsigned tbl)
 
 void NetworkInterface::addRoute(const Route& route, const RoutingTable rt)
 {
-    InetAddrType type = route.dst.getType();
-    if (route.src.getType() != type) {
+    InetAddrType type = route.dst.type;
+    if (route.src.type != type) {
         const std::string msg = "Family type must be the same";
         LOGE(msg);
         throw NetworkException(msg);
@@ -683,8 +683,8 @@ void NetworkInterface::addRoute(const Route& route, const RoutingTable rt)
 
 void NetworkInterface::delRoute(const Route& route, const RoutingTable rt)
 {
-    InetAddrType type = route.dst.getType();
-    if (route.src.getType() != type) {
+    InetAddrType type = route.dst.type;
+    if (route.src.type != type) {
         const std::string msg = "Family type must be the same";
         LOGE(msg);
         throw NetworkException(msg);
@@ -753,11 +753,11 @@ static std::vector<Route> getRoutesImpl(pid_t pid, rt_class_t tbl, const std::st
         route.table = getRoutingTable(rt.rtm_table);
 
         if (rt.rtm_family == AF_INET6) {
-            route.dst.setType(InetAddrType::IPV6);
-            route.src.setType(InetAddrType::IPV6);
+            route.dst.type = InetAddrType::IPV6;
+            route.src.type = InetAddrType::IPV6;
         } else if (rt.rtm_family == AF_INET) {
-            route.dst.setType(InetAddrType::IPV4);
-            route.src.setType(InetAddrType::IPV4);
+            route.dst.type = InetAddrType::IPV4;
+            route.src.type = InetAddrType::IPV4;
         } else {
             const std::string msg = "Unsupported inet family";
             LOGE(msg);
@@ -773,7 +773,7 @@ static std::vector<Route> getRoutesImpl(pid_t pid, rt_class_t tbl, const std::st
             switch (attrType) {
             case RTA_DST:    // 1
             case RTA_GATEWAY:// 5
-                if (route.dst.getType() == InetAddrType::IPV6) {
+                if (route.dst.type == InetAddrType::IPV6) {
                     response.fetch(attrType, route.dst.getAddr<in6_addr>());
                 } else {
                     response.fetch(attrType, route.dst.getAddr<in_addr>());
@@ -781,7 +781,7 @@ static std::vector<Route> getRoutesImpl(pid_t pid, rt_class_t tbl, const std::st
                 break;
             case RTA_SRC:    // 2
             case RTA_PREFSRC:// 7
-                if (route.src.getType() == InetAddrType::IPV6) {
+                if (route.src.type == InetAddrType::IPV6) {
                     response.fetch(attrType, route.src.getAddr<in6_addr>());
                     route.src.prefix = 128;
                 } else {

diff --git a/libs/lxcpp/network.hpp b/libs/lxcpp/network.hpp
index eeb5897..1a6bd51 100644 (file)
--- a/libs/lxcpp/network.hpp
+++ b/libs/lxcpp/network.hpp
@@ -30,6 +30,7 @@
 #include <cstring>
 #include <string>
 #include <vector>
+#include <array>
 #include <ostream>
 
 #include <arpa/inet.h>
@@ -57,26 +58,18 @@ public:
     InetAddr() = default;
     InetAddr(const std::string& addr, unsigned prefix, uint32_t flags=0);
 
-    InetAddrType getType() const {
-        return static_cast<InetAddrType>(type);
-    }
-    void setType(InetAddrType t) {
-        type = static_cast<int>(t);
-    }
-
     template<typename T>
     T& getAddr() {
-        //FIXME return union field after fix of addr type
-        char *v = addr;
+        std::uint8_t *v = addr.data();
         return *(reinterpret_cast<T*>(v));
     }
     template<typename T>
     const T& getAddr() const {
-        //FIXME return union field after fix of addr type
-        const char *v = addr;
+        const std::uint8_t *v = addr.data();
         return *(reinterpret_cast<const T*>(v));
     }
 
+    InetAddrType type;
     unsigned prefix;
     uint32_t flags;
 
@@ -84,16 +77,12 @@ public:
     (
         type,
         flags,
-        prefix
-        //FIXME add when visitor can serialize char[SIZE]
-        //addr
+        prefix,
+        addr
     )
 
 private:
- //FIXME change to union when visitor can serialize type by istream ostream operators
-    char addr[sizeof(in6_addr)];
- //FIXME: change to enum when visitor can serialize type by istream ostream operators
-    int type;
+    std::array<std::uint8_t,sizeof(in6_addr)> addr;
 };
 
 static inline bool operator==(const in_addr& a, const in_addr& b)
@@ -108,8 +97,8 @@ static inline bool operator==(const in6_addr& a, const in6_addr& b)
 
 static inline bool operator==(const InetAddr& a, const InetAddr& b)
 {
-    if (a.getType() == b.getType() && a.prefix == b.prefix) {
-        if (a.getType() == InetAddrType::IPV6)
+    if (a.type == b.type && a.prefix == b.prefix) {
+        if (a.type == InetAddrType::IPV6)
             return a.getAddr<in6_addr>() == b.getAddr<in6_addr>();
         else
             return a.getAddr<in_addr>() == b.getAddr<in_addr>();

diff --git a/tests/unit_tests/lxcpp/ut-cgroups.cpp b/tests/unit_tests/lxcpp/ut-cgroups.cpp
index 5568599..4e26bd1 100644 (file)
--- a/tests/unit_tests/lxcpp/ut-cgroups.cpp
+++ b/tests/unit_tests/lxcpp/ut-cgroups.cpp
@@ -141,7 +141,7 @@ BOOST_AUTO_TEST_CASE(ModifyCGroupParams)
     BOOST_CHECK_NO_THROW(memg.destroy());
 }
 
-BOOST_AUTO_TEST_CASE(LisDevicesPermissions)
+BOOST_AUTO_TEST_CASE(ListDevicesPermissions)
 {
     DevicesCGroup devgrp("/tmp");
 

diff --git a/tests/unit_tests/lxcpp/ut-network.cpp b/tests/unit_tests/lxcpp/ut-network.cpp
index 2776be6..1f55976 100644 (file)
--- a/tests/unit_tests/lxcpp/ut-network.cpp
+++ b/tests/unit_tests/lxcpp/ut-network.cpp
@@ -139,6 +139,8 @@ BOOST_AUTO_TEST_CASE(NetworkListInterfaces)
 BOOST_AUTO_TEST_CASE(NetworkConfigSerialization)
 {
     std::string tmpConfigFile = "/tmp/netconfig.conf";
+    ::unlink(tmpConfigFile.c_str());
+
     NetworkConfig cfg;
     BOOST_CHECK_NO_THROW(config::saveToJsonString(cfg));
 
@@ -153,8 +155,10 @@ BOOST_AUTO_TEST_CASE(NetworkConfigSerialization)
     NetworkConfig cfg2;
     BOOST_CHECK_NO_THROW(config::loadFromJsonFile(tmpConfigFile, cfg2));
 
-    int ifnum = cfg.getInterfaces().size();
-    for (int i = 0; i < ifnum; ++i) {
+    int ifn1 = cfg.getInterfaces().size();
+    int ifn2 = cfg2.getInterfaces().size();
+    BOOST_CHECK_EQUAL(ifn1, ifn2);
+    for (int i = 0; i < ifn2; ++i) {
         const NetworkInterfaceConfig& ni1 = cfg.getInterface(i);
         const NetworkInterfaceConfig& ni2 = cfg2.getInterface(i);