Code refactor/cleanup 70/35370/1
authorPiotr Bartosiewicz <p.bartosiewi@partner.samsung.com>
Thu, 12 Feb 2015 16:22:01 +0000 (17:22 +0100)
committerPiotr Bartosiewicz <p.bartosiewi@partner.samsung.com>
Thu, 12 Feb 2015 16:22:01 +0000 (17:22 +0100)
[Bug/Feature]   N/A
[Cause]         N/A
[Solution]      N/A
[Verification]  Build, run tests

Change-Id: I30a1358f9163c78afa44f6ea0731003171f0f9a7

server/dynamic-config-scheme.hpp [new file with mode: 0644]
server/exception.hpp
server/zone-connection-transport.cpp
server/zone.cpp
server/zones-manager.cpp
server/zones-manager.hpp
tests/unit_tests/lxc/templates/minimal-dbus.sh
tests/unit_tests/server/ut-server.cpp
tests/unit_tests/server/ut-zone-admin.cpp
tests/unit_tests/server/ut-zone.cpp
tests/unit_tests/server/ut-zones-manager.cpp

diff --git a/server/dynamic-config-scheme.hpp b/server/dynamic-config-scheme.hpp
new file mode 100644 (file)
index 0000000..90c80d5
--- /dev/null
@@ -0,0 +1,51 @@
+/*
+ *  Copyright (c) 2015 Samsung Electronics Co., Ltd All Rights Reserved
+ *
+ *  Contact: Piotr Bartosiewicz <p.bartosiewi@partner.samsung.com>
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License
+ */
+
+/**
+ * @file
+ * @author  Piotr Bartosiewicz (p.bartosiewi@partner.samsung.com)
+ * @brief   Dynamic config helper functions
+ */
+
+
+#ifndef SERVER_DYNAMIC_CONFIG_SCHEME_HPP
+#define SERVER_DYNAMIC_CONFIG_SCHEME_HPP
+
+#include <string>
+
+namespace vasum {
+
+/**
+ * Gets db prefix for vasum config
+ */
+inline std::string getVasumDbPrefix()
+{
+    return "vasum";
+}
+
+/**
+ * Gets db prefix for zone config
+ */
+inline std::string getZoneDbPrefix(const std::string& id)
+{
+    return "zone." + id;
+}
+
+} // namespace vasum
+
+#endif // SERVER_DYNAMIC_CONFIG_SCHEME_HPP
index 09eaaef..cb358b1 100644 (file)
@@ -50,6 +50,14 @@ struct ZoneOperationException: public ServerException {
 };
 
 /**
+ * Invalid zone id
+ */
+struct InvalidZoneIdException : public ServerException {
+
+    InvalidZoneIdException(const std::string& error = "") : ServerException(error) {}
+};
+
+/**
  * Exception during performing an operation on a zone connection
  */
 struct ZoneConnectionException: public ServerException {
index 0757241..0a8f8a0 100644 (file)
@@ -74,6 +74,7 @@ ZoneConnectionTransport::ZoneConnectionTransport(const std::string& runMountPoin
             LOGE("Initialization failed: could not mount " << runMountPoint);
             throw ZoneConnectionException("Could not mount: " + runMountPoint);
         }
+        LOGI("Mounted: " << runMountPoint);
     }
 
     // if there is no systemd in the zone this dir won't be created automatically
@@ -95,6 +96,7 @@ ZoneConnectionTransport::~ZoneConnectionTransport()
             if (!utils::umount(mRunMountPoint)) {
                 LOGE("Deinitialization failed: could not umount " << mRunMountPoint);
             }
+            LOGI("Unmounted: " << mRunMountPoint);
         }
     }
 }
index 1617bd6..0c2eca7 100644 (file)
@@ -25,6 +25,7 @@
 #include "config.hpp"
 
 #include "zone.hpp"
+#include "dynamic-config-scheme.hpp"
 #include "base-exception.hpp"
 
 #include "logger/logger.hpp"
@@ -48,7 +49,6 @@ typedef std::lock_guard<std::recursive_mutex> Lock;
 // TODO: move constants to the config file when default values are implemented there
 const int RECONNECT_RETRIES = 15;
 const int RECONNECT_DELAY = 1 * 1000;
-const std::string DB_PREFIX = "zone.";
 
 } // namespace
 
@@ -77,7 +77,7 @@ Zone::Zone(const utils::Worker::Pointer& worker,
 
     const fs::path zonePath = fs::path(zonesPath) / mAdmin->getId();
     mRootPath = (zonePath / fs::path("rootfs")).string();
-    const std::string dbPrefix = DB_PREFIX + mAdmin->getId();
+    const std::string dbPrefix = getZoneDbPrefix(mAdmin->getId());
 
     config::loadFromKVStoreWithJsonFile(dbPath, zoneConfigPath, mDynamicConfig, dbPrefix);
     mProvision.reset(new ZoneProvision(mRootPath, zoneConfigPath, dbPath, dbPrefix, mConfig.validLinkPrefixes));
index 6acc128..3baa2a2 100644 (file)
@@ -27,6 +27,7 @@
 #include "host-dbus-definitions.hpp"
 #include "common-dbus-definitions.hpp"
 #include "zone-dbus-definitions.hpp"
+#include "dynamic-config-scheme.hpp"
 #include "zones-manager.hpp"
 #include "zone-admin.hpp"
 #include "lxc/cgroup.hpp"
@@ -64,7 +65,6 @@ bool regexMatchVector(const std::string& str, const std::vector<boost::regex>& v
     return false;
 }
 
-const std::string DB_PREFIX = "daemon";
 const std::string HOST_ID = "host";
 const std::string ENABLED_FILE_NAME = "enabled";
 
@@ -124,7 +124,10 @@ ZonesManager::ZonesManager(const std::string& configPath)
     LOGD("Instantiating ZonesManager object...");
 
     config::loadFromJsonFile(configPath, mConfig);
-    config::loadFromKVStoreWithJsonFile(mConfig.dbPath, configPath, mDynamicConfig, DB_PREFIX);
+    config::loadFromKVStoreWithJsonFile(mConfig.dbPath,
+                                        configPath,
+                                        mDynamicConfig,
+                                        getVasumDbPrefix());
 
     mProxyCallPolicy.reset(new ProxyCallPolicy(mConfig.proxyCallRules));
 
@@ -187,7 +190,7 @@ ZonesManager::ZonesManager(const std::string& configPath)
                                                  this, _1, _2, _3));
 
     for (const auto& zoneConfig : mDynamicConfig.zoneConfigs) {
-        createZone(utils::createFilePath(mConfig.zoneNewConfigPrefix, zoneConfig));
+        insertZone(utils::createFilePath(mConfig.zoneNewConfigPrefix, zoneConfig));
     }
 
     updateDefaultId();
@@ -233,14 +236,14 @@ Zone& ZonesManager::getZone(const std::string& id)
 {
     auto iter = findZone(id);
     if (iter == mZones.end()) {
-        throw std::out_of_range("id not found");
+        throw InvalidZoneIdException("Zone id not found");
     }
     return get(iter);
 }
 
 void ZonesManager::saveDynamicConfig()
 {
-    config::saveToKVStore(mConfig.dbPath, mDynamicConfig, DB_PREFIX);
+    config::saveToKVStore(mConfig.dbPath, mDynamicConfig, getVasumDbPrefix());
 }
 
 void ZonesManager::updateDefaultId()
@@ -266,7 +269,7 @@ void ZonesManager::updateDefaultId()
     saveDynamicConfig();
 }
 
-void ZonesManager::createZone(const std::string& zoneConfigPath)
+void ZonesManager::insertZone(const std::string& zoneConfigPath)
 {
     LOGT("Creating Zone " << zoneConfigPath);
     std::unique_ptr<Zone> zone(new Zone(mWorker->createSubWorker(),
@@ -277,7 +280,10 @@ void ZonesManager::createZone(const std::string& zoneConfigPath)
                                         mConfig.runMountPointPrefix));
     const std::string id = zone->getId();
     if (id == HOST_ID) {
-        throw ZoneOperationException("Cannot use reserved zone ID");
+        throw InvalidZoneIdException("Cannot use reserved zone ID");
+    }
+    if (findZone(id) != mZones.end()) {
+        throw InvalidZoneIdException("Zone already exists");
     }
 
     using namespace std::placeholders;
@@ -296,11 +302,6 @@ void ZonesManager::createZone(const std::string& zoneConfigPath)
     zone->setDbusStateChangedCallback(bind(&ZonesManager::handleDbusStateChanged,
                                            this, id, _1));
 
-    Lock lock(mMutex);
-
-    if (findZone(id) != mZones.end()) {
-        throw ZoneOperationException("Zone already exists");
-    }
     mZones.push_back(std::move(zone));
 
     // after zone is created successfully, put a file informing that zones are enabled
@@ -319,7 +320,7 @@ void ZonesManager::destroyZone(const std::string& zoneId)
     auto iter = findZone(zoneId);
     if (iter == mZones.end()) {
         LOGE("Failed to destroy zone " << zoneId << ": no such zone");
-        throw ZoneOperationException("No such zone");
+        throw InvalidZoneIdException("No such zone");
     }
 
     get(iter).setDestroyOnExit();
@@ -444,7 +445,7 @@ bool ZonesManager::isPaused(const std::string& zoneId)
     auto iter = findZone(zoneId);
     if (iter == mZones.end()) {
         LOGE("No such zone id: " << zoneId);
-        throw ZoneOperationException("No such zone");
+        throw InvalidZoneIdException("No such zone");
     }
 
     return get(iter).isPaused();
@@ -457,7 +458,7 @@ bool ZonesManager::isRunning(const std::string& zoneId)
     auto iter = findZone(zoneId);
     if (iter == mZones.end()) {
         LOGE("No such zone id: " << zoneId);
-        throw ZoneOperationException("No such zone");
+        throw InvalidZoneIdException("No such zone");
     }
     return get(iter).isRunning();
 }
@@ -803,7 +804,7 @@ void ZonesManager::handleDeclareFileCall(const std::string& zone,
 
         const std::string id = getZone(zone).declareFile(type, path, flags, mode);
         result->set(g_variant_new("(s)", id.c_str()));
-    } catch (const std::out_of_range&) {
+    } catch (const InvalidZoneIdException&) {
         LOGE("No zone with id=" << zone);
         result->setError(api::ERROR_INVALID_ID, "No such zone id");
     } catch (const config::ConfigException& ex) {
@@ -827,7 +828,7 @@ void ZonesManager::handleDeclareMountCall(const std::string& source,
 
         const std::string id = getZone(zone).declareMount(source, target, type, flags, data);
         result->set(g_variant_new("(s)", id.c_str()));
-    } catch (const std::out_of_range&) {
+    } catch (const InvalidZoneIdException&) {
         LOGE("No zone with id=" << zone);
         result->setError(api::ERROR_INVALID_ID, "No such zone id");
     } catch (const config::ConfigException& ex) {
@@ -847,7 +848,7 @@ void ZonesManager::handleDeclareLinkCall(const std::string& source,
 
         const std::string id = getZone(zone).declareLink(source, target);
         result->set(g_variant_new("(s)", id.c_str()));
-    } catch (const std::out_of_range&) {
+    } catch (const InvalidZoneIdException&) {
         LOGE("No zone with id=" << zone);
         result->setError(api::ERROR_INVALID_ID, "No such zone id");
     } catch (const config::ConfigException& ex) {
@@ -874,7 +875,7 @@ void ZonesManager::handleGetDeclarationsCall(const std::string& zone,
                                               out.data(),
                                               out.size());
         result->set(g_variant_new("(@as)", array));
-    } catch (const std::out_of_range&) {
+    } catch (const InvalidZoneIdException&) {
         LOGE("No zone with id=" << zone);
         result->setError(api::ERROR_INVALID_ID, "No such zone id");
     } catch (const VasumException& ex) {
@@ -895,7 +896,7 @@ void ZonesManager::handleRemoveDeclarationCall(const std::string& zone,
         getZone(zone).removeDeclaration(declarationId);
 
         result->setVoid();
-    } catch (const std::out_of_range&) {
+    } catch (const InvalidZoneIdException&) {
         LOGE("No zone with id=" << zone);
         result->setError(api::ERROR_INVALID_ID, "No such zone id");
     } catch (const VasumException& ex) {
@@ -997,14 +998,12 @@ int ZonesManager::getVTForNewZone()
     return *candidates.begin();
 }
 
-void ZonesManager::handleCreateZoneCall(const std::string& id,
-                                        const std::string& templateName,
-                                        dbus::MethodResultBuilder::Pointer result)
+void ZonesManager::createZone(const std::string& id,
+                              const std::string& templateName)
 {
-    if (id.empty()) {
+    if (id.empty()) { // TODO validate id (no spaces, slashes etc)
         LOGE("Failed to add zone - invalid name.");
-        result->setError(api::ERROR_INVALID_ID, "Invalid name");
-        return;
+        throw InvalidZoneIdException("Invalid name");
     }
 
     LOGI("Creating zone " << id);
@@ -1018,8 +1017,7 @@ void ZonesManager::handleCreateZoneCall(const std::string& id,
     // check if zone does not exist
     if (findZone(id) != mZones.end()) {
         LOGE("Cannot create " << id << " zone - already exists!");
-        result->setError(api::ERROR_INVALID_ID, "Already exists");
-        return;
+        throw InvalidZoneIdException("Already exists");
     }
 
     const std::string zonePathStr = utils::createFilePath(mConfig.zonesPath, id, "/");
@@ -1033,8 +1031,7 @@ void ZonesManager::handleCreateZoneCall(const std::string& id,
 
         if (!utils::launchAsRoot(copyImageContentsWrapper)) {
             LOGE("Failed to copy zone image.");
-            result->setError(api::ERROR_INTERNAL, "Failed to copy zone image.");
-            return;
+            throw ZoneOperationException("Failed to copy zone image.");
         }
     }
 
@@ -1062,25 +1059,35 @@ void ZonesManager::handleCreateZoneCall(const std::string& id,
     } catch (VasumException& e) {
         LOGE("Generate config failed: " << e.what());
         utils::launchAsRoot(std::bind(removeAllWrapper, zonePathStr));
-        result->setError(api::ERROR_INTERNAL, "Failed to generate config");
-        return;
+        throw e;
     }
 
     LOGT("Creating new zone");
     try {
-        createZone(newConfigPath);
+        insertZone(newConfigPath);
     } catch (VasumException& e) {
         LOGE("Creating new zone failed: " << e.what());
         utils::launchAsRoot(std::bind(removeAllWrapper, zonePathStr));
-        result->setError(api::ERROR_INTERNAL, "Failed to create zone");
-        return;
+        throw e;
     }
 
     mDynamicConfig.zoneConfigs.push_back(newConfigName);
     saveDynamicConfig();
     updateDefaultId();
+}
 
-    result->setVoid();
+void ZonesManager::handleCreateZoneCall(const std::string& id,
+                                        const std::string& templateName,
+                                        dbus::MethodResultBuilder::Pointer result)
+{
+    try {
+        createZone(id, templateName);
+        result->setVoid();
+    } catch (const InvalidZoneIdException& e) {
+        result->setError(api::ERROR_INVALID_ID, "Existing or invalid zone id");
+    } catch (const VasumException& e) {
+        result->setError(api::ERROR_INTERNAL, "Failed to create zone");
+    }
 }
 
 void ZonesManager::handleDestroyZoneCall(const std::string& id,
@@ -1091,7 +1098,7 @@ void ZonesManager::handleDestroyZoneCall(const std::string& id,
             LOGI("Destroying zone " << id);
 
             destroyZone(id);
-        } catch (const ZoneOperationException& e) {
+        } catch (const InvalidZoneIdException&) {
             LOGE("Failed to destroy zone - no such zone id: " << id);
             result->setError(api::ERROR_INVALID_ID, "No such zone id");
         } catch (const VasumException& e) {
index 99dfa4b..529ef8b 100644 (file)
@@ -49,9 +49,10 @@ public:
     /**
      * Create new zone.
      *
-     * @param zoneConfigPath config of new zone
+     * @param zoneId id of new zone
+     * @param templateName a template name
      */
-    void createZone(const std::string& zoneConfigPath);
+    void createZone(const std::string& zoneId, const std::string& templateName);
 
     /**
      * Destroy zone.
@@ -137,6 +138,7 @@ private:
                            const std::string& templatePath,
                            const std::string& resultPath);
     int getVTForNewZone();
+    void insertZone(const std::string& zoneConfigPath);
 
     void notifyActiveZoneHandler(const std::string& caller,
                                  const std::string& appliaction,
index fb61eb3..a8ff5b9 100755 (executable)
@@ -56,6 +56,9 @@ lxc.haltsignal = SIGTERM
 lxc.pts = 256
 lxc.tty = 0
 
+#lxc.loglevel = TRACE
+#lxc.logfile = /tmp/${name}.log
+
 lxc.cgroup.devices.deny = a
 
 lxc.mount.auto = proc sys cgroup
index 3ed7319..3cd3e51 100644 (file)
 #include <future>
 
 namespace {
+
+const std::string CONFIG_DIR = VSM_TEST_CONFIG_INSTALL_DIR "/server/ut-server";
+const std::string TEST_CONFIG_PATH = CONFIG_DIR + "/test-daemon.conf";
+const std::string BUGGY_CONFIG_PATH = CONFIG_DIR + "/buggy-daemon.conf";
+const std::string MISSING_CONFIG_PATH = CONFIG_DIR + "/missing-daemon.conf";
+
 const std::string ZONES_PATH = "/tmp/ut-zones"; // the same as in daemon.conf
 const bool AS_ROOT = true;
 
@@ -52,11 +58,6 @@ BOOST_FIXTURE_TEST_SUITE(ServerSuite, Fixture)
 using namespace vasum;
 using namespace config;
 
-const std::string TEST_CONFIG_PATH = VSM_TEST_CONFIG_INSTALL_DIR "/server/ut-server/test-daemon.conf";
-const std::string BUGGY_CONFIG_PATH = VSM_TEST_CONFIG_INSTALL_DIR "/server/ut-server/buggy-daemon.conf";
-const std::string MISSING_CONFIG_PATH = "/this/is/a/missing/file/path/missing-daemon.conf";
-
-
 BOOST_AUTO_TEST_CASE(ConstructorDestructorTest)
 {
     std::unique_ptr<Server> s;
index 8dc3eb8..ba21ea9 100644 (file)
@@ -37,10 +37,11 @@ using namespace vasum;
 
 namespace {
 
-const std::string TEST_CONFIG_PATH = VSM_TEST_CONFIG_INSTALL_DIR "/server/ut-zone-admin/zones/test.conf";
-const std::string TEST_NO_SHUTDOWN_CONFIG_PATH = VSM_TEST_CONFIG_INSTALL_DIR "/server/ut-zone-admin/zones/test-no-shutdown.conf";
-const std::string BUGGY_CONFIG_PATH = VSM_TEST_CONFIG_INSTALL_DIR "/server/ut-zone-admin/zones/buggy.conf";
-const std::string MISSING_CONFIG_PATH = VSM_TEST_CONFIG_INSTALL_DIR "/server/ut-zone-admin/zones/missing.conf";
+const std::string ZONES_CONFIG_DIR = VSM_TEST_CONFIG_INSTALL_DIR "/server/ut-zone-admin/zones";
+const std::string TEST_CONFIG_PATH = ZONES_CONFIG_DIR + "/test.conf";
+const std::string TEST_NO_SHUTDOWN_CONFIG_PATH = ZONES_CONFIG_DIR + "/test-no-shutdown.conf";
+const std::string BUGGY_CONFIG_PATH = ZONES_CONFIG_DIR + "/buggy.conf";
+const std::string MISSING_CONFIG_PATH = ZONES_CONFIG_DIR + "/missing.conf";
 const std::string ZONES_PATH = "/tmp/ut-zones";
 const std::string LXC_TEMPLATES_PATH = VSM_TEST_LXC_TEMPLATES_INSTALL_DIR;
 
index 783fbf8..646a0a8 100644 (file)
@@ -45,10 +45,11 @@ using namespace config;
 
 namespace {
 
-const std::string TEST_CONFIG_PATH = VSM_TEST_CONFIG_INSTALL_DIR "/server/ut-zone/zones/test.conf";
-const std::string TEST_DBUS_CONFIG_PATH = VSM_TEST_CONFIG_INSTALL_DIR "/server/ut-zone/zones/test-dbus.conf";
-const std::string BUGGY_CONFIG_PATH = VSM_TEST_CONFIG_INSTALL_DIR "/server/ut-zone/zones/buggy.conf";
-const std::string MISSING_CONFIG_PATH = "/this/is/a/missing/file/path/config.conf";
+const std::string ZONES_CONFIG_DIR = VSM_TEST_CONFIG_INSTALL_DIR "/server/ut-zone/zones";
+const std::string TEST_CONFIG_PATH = ZONES_CONFIG_DIR + "/test.conf";
+const std::string TEST_DBUS_CONFIG_PATH = ZONES_CONFIG_DIR + "/test-dbus.conf";
+const std::string BUGGY_CONFIG_PATH = ZONES_CONFIG_DIR + "/buggy.conf";
+const std::string MISSING_CONFIG_PATH = ZONES_CONFIG_DIR + "/missing.conf";
 const std::string ZONES_PATH = "/tmp/ut-zones";
 const std::string LXC_TEMPLATES_PATH = VSM_TEST_LXC_TEMPLATES_INSTALL_DIR;
 const std::string DB_PATH = ZONES_PATH + "/vasum.db";
index b0ab2e2..a32aaa7 100644 (file)
@@ -61,12 +61,12 @@ using namespace dbus;
 
 namespace {
 
-const std::string TEST_CONFIG_PATH = VSM_TEST_CONFIG_INSTALL_DIR "/server/ut-zones-manager/test-daemon.conf";
-const std::string TEST_DBUS_CONFIG_PATH = VSM_TEST_CONFIG_INSTALL_DIR "/server/ut-zones-manager/test-dbus-daemon.conf";
-const std::string EMPTY_DBUS_CONFIG_PATH = VSM_TEST_CONFIG_INSTALL_DIR "/server/ut-zones-manager/empty-dbus-daemon.conf";
-const std::string BUGGY_CONFIG_PATH = VSM_TEST_CONFIG_INSTALL_DIR "/server/ut-zones-manager/buggy-daemon.conf";
-const std::string TEST_ZONE_CONF_PATH = VSM_TEST_CONFIG_INSTALL_DIR "/server/ut-zones-manager/zones/";
-const std::string MISSING_CONFIG_PATH = "/this/is/a/missing/file/path/missing-daemon.conf";
+const std::string CONFIG_DIR = VSM_TEST_CONFIG_INSTALL_DIR "/server/ut-zones-manager";
+const std::string TEST_CONFIG_PATH = CONFIG_DIR + "/test-daemon.conf";
+const std::string TEST_DBUS_CONFIG_PATH = CONFIG_DIR + "/test-dbus-daemon.conf";
+const std::string EMPTY_DBUS_CONFIG_PATH = CONFIG_DIR + "/empty-dbus-daemon.conf";
+const std::string BUGGY_CONFIG_PATH = CONFIG_DIR + "/buggy-daemon.conf";
+const std::string MISSING_CONFIG_PATH = CONFIG_DIR + "/missing-daemon.conf";
 const int EVENT_TIMEOUT = 5000;
 const int UNEXPECTED_EVENT_TIMEOUT = EVENT_TIMEOUT / 5;
 const int TEST_DBUS_CONNECTION_ZONES_COUNT = 3;