From 4d271cedce6a6768015ebdf90a553e3a4f16be20 Mon Sep 17 00:00:00 2001 From: Piotr Bartosiewicz Date: Fri, 25 Apr 2014 16:04:31 +0200 Subject: [PATCH 01/16] Split ContainerConnection into two classes [Bug/Feature] ContainerConnection had two separate functionalities [Cause] N/A [Solution] Extract transport class from connection class [Verification] Build, install, run tests, run server Change-Id: I165089d861a40e94f13bba31d61bce3b7571ff4e --- server/container-connection-transport.cpp | 91 +++++++++++++++++++++++++++ server/container-connection-transport.hpp | 57 +++++++++++++++++ server/container-connection.cpp | 55 ++-------------- server/container-connection.hpp | 17 +---- server/container.cpp | 6 +- server/container.hpp | 2 + unit_tests/server/ut-container-connection.cpp | 36 +++++------ 7 files changed, 178 insertions(+), 86 deletions(-) create mode 100644 server/container-connection-transport.cpp create mode 100644 server/container-connection-transport.hpp diff --git a/server/container-connection-transport.cpp b/server/container-connection-transport.cpp new file mode 100644 index 0000000..e1012eb --- /dev/null +++ b/server/container-connection-transport.cpp @@ -0,0 +1,91 @@ +/* + * Copyright (c) 2014 Samsung Electronics Co., Ltd All Rights Reserved + * + * Contact: Piotr Bartosiewicz + * + * 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 Implementation of a class for communication transport between container and server + */ + +#include "container-connection-transport.hpp" +#include "exception.hpp" + +#include "utils/file-wait.hpp" +#include "utils/fs.hpp" +#include "log/logger.hpp" + + +namespace security_containers { + +namespace { + +// Timeout in ms for waiting for dbus transport. +// Should be very long to ensure dbus in container is ready. +const unsigned int TRANSPORT_READY_TIMEOUT = 2 * 60 * 1000; + +} // namespace + + +ContainerConnectionTransport::ContainerConnectionTransport(const std::string& runMountPoint) + : mRunMountPoint(runMountPoint) +{ + if (runMountPoint.empty()) { + return; + } + if (!utils::createDirectories(runMountPoint, 0755)) { + LOGE("Initialization failed: could not create " << runMountPoint); + throw ContainerConnectionException("Could not create: " + runMountPoint); + } + + // try to umount if already mounted + utils::umount(runMountPoint); + + if (!utils::mountTmpfs(runMountPoint)) { + LOGE("Initialization failed: could not mount " << runMountPoint); + throw ContainerConnectionException("Could not mount: " + runMountPoint); + } +} + + +ContainerConnectionTransport::~ContainerConnectionTransport() +{ + if (!mRunMountPoint.empty()) { + if (!utils::umount(mRunMountPoint)) { + LOGE("Deinitialization failed: could not umount " << mRunMountPoint); + } + } +} + + +std::string ContainerConnectionTransport::acquireAddress() +{ + if (mRunMountPoint.empty()) { + return std::string(); + } + + const std::string dbusPath = mRunMountPoint + "/dbus/system_bus_socket"; + + // TODO This should be done asynchronously. + LOGT("Waiting for " << dbusPath); + utils::waitForFile(dbusPath, TRANSPORT_READY_TIMEOUT); + + return "unix:path=" + dbusPath; +} + + +} // namespace security_containers diff --git a/server/container-connection-transport.hpp b/server/container-connection-transport.hpp new file mode 100644 index 0000000..989ba96 --- /dev/null +++ b/server/container-connection-transport.hpp @@ -0,0 +1,57 @@ +/* + * Copyright (c) 2014 Samsung Electronics Co., Ltd All Rights Reserved + * + * Contact: Piotr Bartosiewicz + * + * 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 Declaration of a class for communication transport between container and server + */ + + +#ifndef SERVER_CONTAINER_CONNECTION_TRANSPORT_HPP +#define SERVER_CONTAINER_CONNECTION_TRANSPORT_HPP + +#include "dbus/connection.hpp" + + +namespace security_containers { + + +/** + * This class provides a communication transport between container and server. + * The object lifecycle should cover lifecycle of a container. + */ +class ContainerConnectionTransport { +public: + ContainerConnectionTransport(const std::string& runMountPoint); + ~ContainerConnectionTransport(); + + /** + * Gets dbus addres. Will block until address is available. + */ + std::string acquireAddress(); + +private: + std::string mRunMountPoint; +}; + + +} // namespace security_containers + + +#endif // SERVER_CONTAINER_CONNECTION_TRANSPORT_HPP diff --git a/server/container-connection.cpp b/server/container-connection.cpp index 7a3e973..ed6bdd8 100644 --- a/server/container-connection.cpp +++ b/server/container-connection.cpp @@ -19,16 +19,13 @@ /** * @file * @author Piotr Bartosiewicz (p.bartosiewi@partner.samsung.com) - * @brief Implementation of class for communication between container and server + * @brief Implementation of a class for communication between container and server */ #include "container-connection.hpp" #include "container-dbus-definitions.hpp" #include "exception.hpp" -#include "utils/file-wait.hpp" -#include "utils/fs.hpp" -#include "utils/paths.hpp" #include "log/logger.hpp" @@ -36,10 +33,6 @@ namespace security_containers { namespace { -// Timeout in ms for waiting for dbus transport. -// Should be very long to ensure dbus in container is ready. -const unsigned int TRANSPORT_READY_TIMEOUT = 2 * 60 * 1000; - // Timeout in ms for waiting for dbus name. // Can happen if glib loop is busy or not present. const unsigned int NAME_ACQUIRED_TIMEOUT = 5 * 1000; @@ -56,56 +49,18 @@ ContainerConnection::ContainerConnection() ContainerConnection::~ContainerConnection() { - deinitialize(); -} - - -void ContainerConnection::initialize(const std::string& runMountPoint) -{ - if (runMountPoint.empty()) { - return; - } - if (!utils::createDirectories(runMountPoint, 0755)) { - LOGE("Initialization failed: could not create " << runMountPoint); - throw ContainerConnectionException("Could not create: " + runMountPoint); - } - - // try to umount if already mounted - utils::umount(runMountPoint); - - if (!utils::mountTmpfs(runMountPoint)) { - LOGE("Initialization failed: could not mount " << runMountPoint); - throw ContainerConnectionException("Could not mount: " + runMountPoint); - } - - mRunMountPoint = runMountPoint; -} - -void ContainerConnection::deinitialize() -{ - if (!mRunMountPoint.empty()) { - if (!utils::umount(mRunMountPoint)) { - LOGE("Deinitialization failed: could not umount " << mRunMountPoint); - } - mRunMountPoint.clear(); - } } -void ContainerConnection::connect() +void ContainerConnection::connect(const std::string& address) { - if (mRunMountPoint.empty()) { + if (address.empty()) { LOGW("The connection to the container is disabled"); return; } - const std::string dbusPath = mRunMountPoint + "/dbus/system_bus_socket"; - - // TODO This should be done asynchronously. - LOGT("Waiting for " << dbusPath); - utils::waitForFile(dbusPath, TRANSPORT_READY_TIMEOUT); - LOGT("Connecting to DBUS"); - mDbusConnection = dbus::DbusConnection::create("unix:path=" + dbusPath); + LOGT("Connecting to DBUS on " << address); + mDbusConnection = dbus::DbusConnection::create(address); LOGT("Setting DBUS name"); mDbusConnection->setName(api::BUS_NAME, diff --git a/server/container-connection.hpp b/server/container-connection.hpp index e825be2..f3c6f1d 100644 --- a/server/container-connection.hpp +++ b/server/container-connection.hpp @@ -19,7 +19,7 @@ /** * @file * @author Piotr Bartosiewicz (p.bartosiewi@partner.samsung.com) - * @brief Declaration of the class for communication between container and server + * @brief Declaration of a class for communication between container and server */ @@ -41,22 +41,10 @@ public: ContainerConnection(); ~ContainerConnection(); - //TODO Move initialize, deinitialize and related stuff to separate class - - /** - * Should be called every time just before container is created. - */ - void initialize(const std::string& runMountPoint); - - /** - * Should be called every time after container is destroyed. - */ - void deinitialize(); - /** * Connect to container. */ - void connect(); + void connect(const std::string& address); /** * Disconnect from container. @@ -82,7 +70,6 @@ public: const std::string& message); private: - std::string mRunMountPoint; dbus::DbusConnection::Pointer mDbusConnection; std::mutex mNameMutex; std::condition_variable mNameCondition; diff --git a/server/container.cpp b/server/container.cpp index 28ed3a6..6474fb1 100644 --- a/server/container.cpp +++ b/server/container.cpp @@ -65,9 +65,9 @@ int Container::getPrivilege() void Container::start() { - mConnection.initialize(mConfig.runMountPoint); + mConnectionTransport.reset(new ContainerConnectionTransport(mConfig.runMountPoint)); mAdmin->start(); - mConnection.connect(); + mConnection.connect(mConnectionTransport->acquireAddress()); // Send to the background only after we're connected, // otherwise it'd take ages. @@ -79,7 +79,7 @@ void Container::stop() { mConnection.disconnect(); mAdmin->stop(); - mConnection.deinitialize(); + mConnectionTransport.reset(); } void Container::goForeground() diff --git a/server/container.hpp b/server/container.hpp index 3774c6b..179a3bb 100644 --- a/server/container.hpp +++ b/server/container.hpp @@ -29,6 +29,7 @@ #include "container-config.hpp" #include "container-admin.hpp" #include "container-connection.hpp" +#include "container-connection-transport.hpp" #include #include @@ -98,6 +99,7 @@ public: private: ContainerConfig mConfig; std::unique_ptr mAdmin; + std::unique_ptr mConnectionTransport; ContainerConnection mConnection; }; diff --git a/unit_tests/server/ut-container-connection.cpp b/unit_tests/server/ut-container-connection.cpp index 679cf60..95203b0 100644 --- a/unit_tests/server/ut-container-connection.cpp +++ b/unit_tests/server/ut-container-connection.cpp @@ -26,6 +26,7 @@ #include "ut.hpp" #include "container-connection.hpp" +#include "container-connection-transport.hpp" #include "container-dbus-definitions.hpp" #include "dbus/connection.hpp" @@ -51,19 +52,24 @@ const char* const DBUS_DAEMON_ARGS[] = { NULL }; -// TODO fix destruction order - move transport stuff to separate raii class const std::string TRANSPORT_MOUNT_POINT = "/tmp/ut-container-connection"; -const std::string DBUS_ADDRESS = "unix:path=" + TRANSPORT_MOUNT_POINT + "/dbus/system_bus_socket"; const int EVENT_TIMEOUT = 1000; -class ScopedDbusDaemon : public ScopedDaemon { +class ScopedDbusDaemon { public: - void start() + ScopedDbusDaemon() + : mTransport(TRANSPORT_MOUNT_POINT) { utils::createDirectory(TRANSPORT_MOUNT_POINT + "/dbus", 0755); mDaemon.start(DBUS_DAEMON_PROC, DBUS_DAEMON_ARGS); } + + std::string acquireAddress() + { + return mTransport.acquireAddress(); + } private: + ContainerConnectionTransport mTransport; ScopedDaemon mDaemon; }; @@ -77,28 +83,24 @@ BOOST_AUTO_TEST_CASE(ConstructorDestructorTest) BOOST_AUTO_TEST_CASE(ConnectTest) { - ScopedDbusDaemon dbus; ScopedGlibLoop loop; + ScopedDbusDaemon dbus; ContainerConnection connection; - connection.initialize(TRANSPORT_MOUNT_POINT); - dbus.start(); - BOOST_REQUIRE_NO_THROW(connection.connect()); + BOOST_REQUIRE_NO_THROW(connection.connect(dbus.acquireAddress())); BOOST_REQUIRE_NO_THROW(connection.disconnect()); } BOOST_AUTO_TEST_CASE(NotifyActiveContainerApiTest) { - ScopedDbusDaemon dbus; ScopedGlibLoop loop; + ScopedDbusDaemon dbus; Latch notifyCalled; ContainerConnection connection; - connection.initialize(TRANSPORT_MOUNT_POINT); - dbus.start(); - BOOST_REQUIRE_NO_THROW(connection.connect()); + BOOST_REQUIRE_NO_THROW(connection.connect(dbus.acquireAddress())); auto callback = [&](const std::string& application, const std::string& message) { if (application == "testapp" && message == "testmessage") { @@ -107,7 +109,7 @@ BOOST_AUTO_TEST_CASE(NotifyActiveContainerApiTest) }; connection.setNotifyActiveContainerCallback(callback); - DbusConnection::Pointer client = DbusConnection::create(DBUS_ADDRESS); + DbusConnection::Pointer client = DbusConnection::create(dbus.acquireAddress()); client->callMethod(api::BUS_NAME, api::OBJECT_PATH, api::INTERFACE, @@ -119,17 +121,15 @@ BOOST_AUTO_TEST_CASE(NotifyActiveContainerApiTest) BOOST_AUTO_TEST_CASE(SignalNotificationApiTest) { - ScopedDbusDaemon dbus; ScopedGlibLoop loop; + ScopedDbusDaemon dbus; Latch signalEmitted; ContainerConnection connection; - connection.initialize(TRANSPORT_MOUNT_POINT); - dbus.start(); - BOOST_REQUIRE_NO_THROW(connection.connect()); + BOOST_REQUIRE_NO_THROW(connection.connect(dbus.acquireAddress())); - DbusConnection::Pointer client = DbusConnection::create(DBUS_ADDRESS); + DbusConnection::Pointer client = DbusConnection::create(dbus.acquireAddress()); auto handler = [&](const std::string& /*senderBusName*/, const std::string& objectPath, -- 2.7.4 From eadaa94e279d2ad0dda07968c4a7ca47925f48f6 Mon Sep 17 00:00:00 2001 From: Piotr Bartosiewicz Date: Mon, 28 Apr 2014 13:11:32 +0200 Subject: [PATCH 02/16] Don't use deprecated json api [Bug/Feature] Compilation ploblems (-Werror=deprecated-declarations) [Cause] N/A [Solution] N/A [Verification] Build, install, run tests Change-Id: Ied61192ca331485c9ff579853053faa8caf959b3 --- common/config/configuration.hpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/common/config/configuration.hpp b/common/config/configuration.hpp index e429b27..f614b7f 100644 --- a/common/config/configuration.hpp +++ b/common/config/configuration.hpp @@ -141,9 +141,9 @@ protected: template static void readValue(json_object* jsonObj, T& val, const char* name) { - json_object* obj = json_object_object_get(jsonObj, name); - if (NULL == obj) { - LOGE("json_object_object_get failed"); + json_object* obj = NULL; + if (!json_object_object_get_ex(jsonObj, name, &obj)) { + LOGE("json_object_object_get_ex() failed, name = " << name); throw ConfigException(); } val = getValueFromJsonObj(obj); @@ -161,9 +161,9 @@ protected: static void readValue(json_object* jsonObj, std::vector& val, const char* name) { val.clear(); - json_object* array = json_object_object_get(jsonObj, name); - if (NULL == array) { - LOGE("json_object_object_get() failed, name = " << name); + json_object* array = NULL; + if (!json_object_object_get_ex(jsonObj, name, &array)) { + LOGE("json_object_object_get_ex() failed, name = " << name); throw ConfigException(); } int len = json_object_array_length(array); -- 2.7.4 From 01abb5759a2654f963b0c11df5fbf16a45dd627c Mon Sep 17 00:00:00 2001 From: Piotr Bartosiewicz Date: Mon, 28 Apr 2014 14:05:46 +0200 Subject: [PATCH 03/16] Include system headers as system [Bug/Feature] Warnings in system headers should be silenced [Cause] N/A [Solution] N/A [Verification] Build project Change-Id: Id966ef78e736e98963d156d6d45e399bfe55a982 --- server/CMakeLists.txt | 3 ++- unit_tests/CMakeLists.txt | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/server/CMakeLists.txt b/server/CMakeLists.txt index 5e24fef..b1ce9f8 100644 --- a/server/CMakeLists.txt +++ b/server/CMakeLists.txt @@ -31,7 +31,8 @@ ADD_EXECUTABLE(${SERVER_CODENAME} ${project_SRCS} ${common_SRCS}) FIND_PACKAGE (Boost COMPONENTS program_options REQUIRED) PKG_CHECK_MODULES(SERVER_DEPS REQUIRED libvirt json gio-2.0 libsystemd-journal) -INCLUDE_DIRECTORIES(${COMMON_FOLDER} ${SERVER_DEPS_INCLUDE_DIRS} ${Boost_INCLUDE_DIRS}) +INCLUDE_DIRECTORIES(${COMMON_FOLDER}) +INCLUDE_DIRECTORIES(SYSTEM ${SERVER_DEPS_INCLUDE_DIRS} ${Boost_INCLUDE_DIRS}) TARGET_LINK_LIBRARIES(${SERVER_CODENAME} ${SERVER_DEPS_LIBRARIES} ${Boost_LIBRARIES}) diff --git a/unit_tests/CMakeLists.txt b/unit_tests/CMakeLists.txt index 36d5036..87cb4c8 100644 --- a/unit_tests/CMakeLists.txt +++ b/unit_tests/CMakeLists.txt @@ -35,8 +35,8 @@ ADD_EXECUTABLE(${UT_SERVER_CODENAME} ${project_SRCS} ${common_SRCS} ${server_SRC FIND_PACKAGE (Boost COMPONENTS unit_test_framework REQUIRED) PKG_CHECK_MODULES(UT_SERVER_DEPS REQUIRED libvirt json gio-2.0 libsystemd-journal) -INCLUDE_DIRECTORIES(${COMMON_FOLDER} ${SERVER_FOLDER} ${UNIT_TESTS_FOLDER} - ${UT_SERVER_DEPS_INCLUDE_DIRS} ${Boost_INCLUDE_DIRS}) +INCLUDE_DIRECTORIES(${COMMON_FOLDER} ${SERVER_FOLDER} ${UNIT_TESTS_FOLDER}) +INCLUDE_DIRECTORIES(SYSTEM ${UT_SERVER_DEPS_INCLUDE_DIRS} ${Boost_INCLUDE_DIRS}) TARGET_LINK_LIBRARIES(${UT_SERVER_CODENAME} ${UT_SERVER_DEPS_LIBRARIES} ${Boost_LIBRARIES}) -- 2.7.4 From 23f6cbe08170570e98882acc5596a13286b9d606 Mon Sep 17 00:00:00 2001 From: Lukasz Pawelczyk Date: Mon, 5 May 2014 14:16:42 +0200 Subject: [PATCH 04/16] Minor changes to ContainerConnection [Bug/Feature] ContainerConnection lifecycle is tied with a connection's lifecycle, OnNameLost callback added. [Cause] Be consistent, OnNameLost callback will be required to handle disconnections [Solution] N/A [Verification] Build, install, run tests Change-Id: Ie43eda2a4774ef003bee9ed877b6caab041035ba Signed-off-by: Lukasz Pawelczyk --- server/container-connection.cpp | 37 ++++++++++----------------- server/container-connection.hpp | 17 ++++-------- server/container.cpp | 10 ++++++-- server/container.hpp | 4 ++- unit_tests/server/ut-container-connection.cpp | 24 ++++++----------- 5 files changed, 37 insertions(+), 55 deletions(-) diff --git a/server/container-connection.cpp b/server/container-connection.cpp index ed6bdd8..80d4be5 100644 --- a/server/container-connection.cpp +++ b/server/container-connection.cpp @@ -40,20 +40,10 @@ const unsigned int NAME_ACQUIRED_TIMEOUT = 5 * 1000; } // namespace -ContainerConnection::ContainerConnection() +ContainerConnection::ContainerConnection(const std::string& address, const OnNameLostCallback& callback) : mNameAcquired(false) , mNameLost(false) { -} - - -ContainerConnection::~ContainerConnection() -{ -} - - -void ContainerConnection::connect(const std::string& address) -{ if (address.empty()) { LOGW("The connection to the container is disabled"); return; @@ -61,15 +51,14 @@ void ContainerConnection::connect(const std::string& address) LOGT("Connecting to DBUS on " << address); mDbusConnection = dbus::DbusConnection::create(address); - LOGT("Setting DBUS name"); + LOGT("Setting DBUS name"); mDbusConnection->setName(api::BUS_NAME, std::bind(&ContainerConnection::onNameAcquired, this), std::bind(&ContainerConnection::onNameLost, this)); - if (!waitForName(NAME_ACQUIRED_TIMEOUT)) { + if (!waitForNameAndSetCallback(NAME_ACQUIRED_TIMEOUT, callback)) { LOGE("Could not acquire dbus name: " << api::BUS_NAME); - disconnect(); throw ContainerConnectionException("Could not acquire dbus name: " + api::BUS_NAME); } @@ -87,18 +76,11 @@ void ContainerConnection::connect(const std::string& address) LOGD("Connected"); } - -void ContainerConnection::disconnect() +ContainerConnection::~ContainerConnection() { - LOGD("Disconnecting"); - mDbusConnection.reset(); - - std::unique_lock lock(mNameMutex); - mNameAcquired = false; - mNameLost = false; } -bool ContainerConnection::waitForName(const unsigned int timeoutMs) +bool ContainerConnection::waitForNameAndSetCallback(const unsigned int timeoutMs, const OnNameLostCallback& callback) { std::unique_lock lock(mNameMutex); mNameCondition.wait_for(lock, @@ -106,6 +88,10 @@ bool ContainerConnection::waitForName(const unsigned int timeoutMs) [this] { return mNameAcquired || mNameLost; }); + if(mNameAcquired) { + mOnNameLostCallback = callback; + } + return mNameAcquired; } @@ -121,7 +107,10 @@ void ContainerConnection::onNameLost() std::unique_lock lock(mNameMutex); mNameLost = true; mNameCondition.notify_one(); - //TODO some callback? + + if(mOnNameLostCallback) { + mOnNameLostCallback(); + } } void ContainerConnection::setNotifyActiveContainerCallback( diff --git a/server/container-connection.hpp b/server/container-connection.hpp index f3c6f1d..4ddac0e 100644 --- a/server/container-connection.hpp +++ b/server/container-connection.hpp @@ -38,18 +38,10 @@ namespace security_containers { class ContainerConnection { public: - ContainerConnection(); - ~ContainerConnection(); - - /** - * Connect to container. - */ - void connect(const std::string& address); + typedef std::function OnNameLostCallback; - /** - * Disconnect from container. - */ - void disconnect(); + ContainerConnection(const std::string& address, const OnNameLostCallback& callback); + ~ContainerConnection(); // ------------- API -------------- @@ -75,11 +67,12 @@ private: std::condition_variable mNameCondition; bool mNameAcquired; bool mNameLost; + OnNameLostCallback mOnNameLostCallback; NotifyActiveContainerCallback mNotifyActiveContainerCallback; void onNameAcquired(); void onNameLost(); - bool waitForName(const unsigned int timeoutMs); + bool waitForNameAndSetCallback(const unsigned int timeoutMs, const OnNameLostCallback& callback); void onMessageCall(const std::string& objectPath, const std::string& interface, diff --git a/server/container.cpp b/server/container.cpp index 6474fb1..afa9ac8 100644 --- a/server/container.cpp +++ b/server/container.cpp @@ -67,7 +67,8 @@ void Container::start() { mConnectionTransport.reset(new ContainerConnectionTransport(mConfig.runMountPoint)); mAdmin->start(); - mConnection.connect(mConnectionTransport->acquireAddress()); + mConnection.reset(new ContainerConnection(mConnectionTransport->acquireAddress(), + std::bind(&Container::onNameLostCallback, this))); // Send to the background only after we're connected, // otherwise it'd take ages. @@ -77,7 +78,7 @@ void Container::start() void Container::stop() { - mConnection.disconnect(); + mConnection.reset(); mAdmin->stop(); mConnectionTransport.reset(); } @@ -107,5 +108,10 @@ bool Container::isPaused() return mAdmin->isPaused(); } +void Container::onNameLostCallback() +{ + // TODO: try to reconnect +} + } // namespace security_containers diff --git a/server/container.hpp b/server/container.hpp index 179a3bb..89ef44e 100644 --- a/server/container.hpp +++ b/server/container.hpp @@ -100,7 +100,9 @@ private: ContainerConfig mConfig; std::unique_ptr mAdmin; std::unique_ptr mConnectionTransport; - ContainerConnection mConnection; + std::unique_ptr mConnection; + + void onNameLostCallback(); }; diff --git a/unit_tests/server/ut-container-connection.cpp b/unit_tests/server/ut-container-connection.cpp index 95203b0..7721a3a 100644 --- a/unit_tests/server/ut-container-connection.cpp +++ b/unit_tests/server/ut-container-connection.cpp @@ -76,20 +76,12 @@ private: } // namespace -BOOST_AUTO_TEST_CASE(ConstructorDestructorTest) -{ - BOOST_REQUIRE_NO_THROW(ContainerConnection()); -} - -BOOST_AUTO_TEST_CASE(ConnectTest) +BOOST_AUTO_TEST_CASE(ConstructorDestructorConnectTest) { ScopedGlibLoop loop; ScopedDbusDaemon dbus; - ContainerConnection connection; - - BOOST_REQUIRE_NO_THROW(connection.connect(dbus.acquireAddress())); - BOOST_REQUIRE_NO_THROW(connection.disconnect()); + BOOST_REQUIRE_NO_THROW(ContainerConnection(dbus.acquireAddress(), nullptr)); } BOOST_AUTO_TEST_CASE(NotifyActiveContainerApiTest) @@ -98,16 +90,16 @@ BOOST_AUTO_TEST_CASE(NotifyActiveContainerApiTest) ScopedDbusDaemon dbus; Latch notifyCalled; - ContainerConnection connection; + std::unique_ptr connection; - BOOST_REQUIRE_NO_THROW(connection.connect(dbus.acquireAddress())); + BOOST_REQUIRE_NO_THROW(connection.reset(new ContainerConnection(dbus.acquireAddress(), nullptr))); auto callback = [&](const std::string& application, const std::string& message) { if (application == "testapp" && message == "testmessage") { notifyCalled.set(); } }; - connection.setNotifyActiveContainerCallback(callback); + connection->setNotifyActiveContainerCallback(callback); DbusConnection::Pointer client = DbusConnection::create(dbus.acquireAddress()); client->callMethod(api::BUS_NAME, @@ -125,9 +117,9 @@ BOOST_AUTO_TEST_CASE(SignalNotificationApiTest) ScopedDbusDaemon dbus; Latch signalEmitted; - ContainerConnection connection; + std::unique_ptr connection; - BOOST_REQUIRE_NO_THROW(connection.connect(dbus.acquireAddress())); + BOOST_REQUIRE_NO_THROW(connection.reset(new ContainerConnection(dbus.acquireAddress(), nullptr))); DbusConnection::Pointer client = DbusConnection::create(dbus.acquireAddress()); @@ -154,7 +146,7 @@ BOOST_AUTO_TEST_CASE(SignalNotificationApiTest) }; client->signalSubscribe(handler, api::BUS_NAME); - connection.sendNotification("testcontainer", "testapp", "testmessage"); + connection->sendNotification("testcontainer", "testapp", "testmessage"); BOOST_CHECK(signalEmitted.wait(EVENT_TIMEOUT)); } -- 2.7.4 From af45e8595ecbca512030e96ef8951c1161cd4cc4 Mon Sep 17 00:00:00 2001 From: Piotr Bartosiewicz Date: Thu, 8 May 2014 12:37:20 +0200 Subject: [PATCH 05/16] Fix config type of cpu quota [Bug/Feature] Type in config was double instead of int64 [Cause] N/A [Solution] N/A [Verification] Build, install, run tests Change-Id: I77ee8fd87faf798a21d2327cb6955be3482e78a0 --- common/config/configuration.cpp | 10 ++++++++++ common/config/configuration.hpp | 8 ++++---- server/container-config.hpp | 4 ++-- unit_tests/config/ut-configuration.cpp | 5 +++++ 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/common/config/configuration.cpp b/common/config/configuration.cpp index d8dd143..fe22aa1 100644 --- a/common/config/configuration.cpp +++ b/common/config/configuration.cpp @@ -95,6 +95,11 @@ template<> json_object* ConfigurationBase::getJsonObjFromValue(const int& val) return json_object_new_int(val); } +template<> json_object* ConfigurationBase::getJsonObjFromValue(const std::int64_t& val) +{ + return json_object_new_int64(val); +} + template<> json_object* ConfigurationBase::getJsonObjFromValue(const bool& val) { return json_object_new_boolean(val); @@ -119,6 +124,11 @@ template<> int ConfigurationBase::getValueFromJsonObj(json_object* obj) return json_object_get_int(obj); } +template<> std::int64_t ConfigurationBase::getValueFromJsonObj(json_object* obj) +{ + return json_object_get_int64(obj); +} + template<> bool ConfigurationBase::getValueFromJsonObj(json_object* obj) { return json_object_get_boolean(obj); diff --git a/common/config/configuration.hpp b/common/config/configuration.hpp index f614b7f..810721a 100644 --- a/common/config/configuration.hpp +++ b/common/config/configuration.hpp @@ -167,9 +167,9 @@ protected: throw ConfigException(); } int len = json_object_array_length(array); - val.resize(len); + val.resize(static_cast(len)); for (int i = 0; i < len; ++i) { - val[i] = getValueFromJsonObj(json_object_array_get_idx(array, i)); + val[static_cast(i)] = getValueFromJsonObj(json_object_array_get_idx(array, i)); } } @@ -227,10 +227,10 @@ protected: } int len = json_object_array_length(obj); - val.resize(len); + val.resize(static_cast(len)); for (int i = 0; i < len; ++i) { json_object* arrayObj = json_object_array_get_idx(obj, i); - val[i].process(arrayObj, ConfigProcessMode::Read); + val[static_cast(i)].process(arrayObj, ConfigProcessMode::Read); } } diff --git a/server/container-config.hpp b/server/container-config.hpp index 1e35ca7..f34b929 100644 --- a/server/container-config.hpp +++ b/server/container-config.hpp @@ -50,12 +50,12 @@ struct ContainerConfig : public config::ConfigurationBase { /** * Container's CFS quota in us when it's in the foreground */ - double cpuQuotaForeground; + std::int64_t cpuQuotaForeground; /** * Container's CFS quota in us when it's in the background */ - double cpuQuotaBackground; + std::int64_t cpuQuotaBackground; /** * Path to containers dbus unix socket diff --git a/unit_tests/config/ut-configuration.cpp b/unit_tests/config/ut-configuration.cpp index 657d586..cbb84eb 100644 --- a/unit_tests/config/ut-configuration.cpp +++ b/unit_tests/config/ut-configuration.cpp @@ -47,6 +47,7 @@ struct TestConfig : public ConfigurationBase { }; int intVal; + std::int64_t int64Val; std::string stringVal; double floatVal; bool boolVal; @@ -60,6 +61,7 @@ struct TestConfig : public ConfigurationBase { CONFIG_REGISTER { CONFIG_VALUE(intVal) + CONFIG_VALUE(int64Val) CONFIG_VALUE(stringVal) CONFIG_VALUE(floatVal) CONFIG_VALUE(boolVal) @@ -75,6 +77,7 @@ struct TestConfig : public ConfigurationBase { bool operator== (const TestConfig& rhs) const { return (rhs.intVal == intVal && + rhs.int64Val == int64Val && rhs.stringVal == stringVal && rhs.floatVal == floatVal && rhs.boolVal == boolVal && @@ -91,6 +94,7 @@ struct TestConfig : public ConfigurationBase { */ const std::string json_test_string = "{ \"intVal\": 12345, " + "\"int64Val\": -1234567890123456789, " "\"stringVal\": \"blah\", " "\"floatVal\": -1.234, " "\"boolVal\": true, " @@ -111,6 +115,7 @@ BOOST_AUTO_TEST_CASE(SimpleTypesTest) BOOST_REQUIRE_NO_THROW(testConfig.parseStr(json_test_string)); BOOST_CHECK_EQUAL(12345, testConfig.intVal); + BOOST_CHECK_EQUAL(-1234567890123456789ll, testConfig.int64Val); BOOST_CHECK_CLOSE(-1.234, testConfig.floatVal, max_float_error); BOOST_CHECK_EQUAL("blah", testConfig.stringVal); BOOST_CHECK_EQUAL(true, testConfig.boolVal); -- 2.7.4 From 59e71108eaba98ea9eb7c024c5aa7bcbaf0f0ade Mon Sep 17 00:00:00 2001 From: Lukasz Pawelczyk Date: Tue, 6 May 2014 12:42:28 +0200 Subject: [PATCH 06/16] Handle reconnecting to the dbus, add glib helper [Bug/Feature] Handle reconnecting to the dbus in case of the connection loss. Also add a Glib helper for scheduling a timer function to the glib loop. Style cosmetics. [Cause] In case the DBUS daemon gets restarted we loose connection, we need to handle that case. [Solution] Detect nameLost event and react appropriately. [Verification] Built, installed, run tests. The reconnect has been tested by hand as follows: 1. Run the security-containers-server 2. Make sure it started properly 3. Enter the container and restart dbus 4. See SCS logs, make sure it reconnected 5. Enter the container stop dbus and dbus.socket 6. See SCS logs, make sure the container stopped Change-Id: I1185d8d46e0ace8e96b4d4136fbca20bd603bea9 Signed-off-by: Lukasz Pawelczyk --- common/dbus/connection.cpp | 2 +- common/log/backend-stderr.cpp | 2 +- common/utils/file-wait.cpp | 5 +- common/utils/fs.cpp | 2 +- common/utils/glib-loop.cpp | 27 ++++++++- common/utils/glib-loop.hpp | 29 ++++++++- common/utils/latch.cpp | 23 ++++++-- common/utils/latch.hpp | 24 +++++++- server/container-connection-transport.cpp | 8 +++ server/container-connection.cpp | 1 + server/container.cpp | 41 ++++++++++++- server/container.hpp | 5 +- server/containers-manager.cpp | 2 +- unit_tests/server/configs/CMakeLists.txt | 9 +++ .../configs/ut-container/containers/test-dbus.conf | 7 +++ .../ut-container/libvirt-config/test-dbus.xml.in | 19 ++++++ .../server/configs/ut-container/ut-dbus.conf | 18 ++++++ unit_tests/server/ut-container-connection.cpp | 1 - unit_tests/server/ut-container.cpp | 27 +++++++++ unit_tests/utils/ut-glib-loop.cpp | 68 ++++++++++++++++++++++ 20 files changed, 297 insertions(+), 23 deletions(-) create mode 100644 unit_tests/server/configs/ut-container/containers/test-dbus.conf create mode 100644 unit_tests/server/configs/ut-container/libvirt-config/test-dbus.xml.in create mode 100644 unit_tests/server/configs/ut-container/ut-dbus.conf create mode 100644 unit_tests/utils/ut-glib-loop.cpp diff --git a/common/dbus/connection.cpp b/common/dbus/connection.cpp index 7d5f0bf..af48230 100644 --- a/common/dbus/connection.cpp +++ b/common/dbus/connection.cpp @@ -185,7 +185,7 @@ void DbusConnection::onNameAcquired(GDBusConnection*, const gchar* name, gpointe void DbusConnection::onNameLost(GDBusConnection*, const gchar* name, gpointer userData) { - LOGE("Name lost " << name); + LOGD("Name lost " << name); const NameCallbacks& callbacks = utils::getCallbackFromPointer(userData); if (callbacks.nameLost) { callbacks.nameLost(); diff --git a/common/log/backend-stderr.cpp b/common/log/backend-stderr.cpp index 4d0aaa7..e91c12d 100644 --- a/common/log/backend-stderr.cpp +++ b/common/log/backend-stderr.cpp @@ -46,7 +46,7 @@ void StderrBackend::log(LogLevel logLevel, const std::string defaultColor = LogFormatter::getDefaultConsoleColor(); const std::string header = LogFormatter::getHeader(logLevel, file, line, func); tokenizer tokens(message, charSeparator("\n")); - for(const auto& line : tokens) { + for (const auto& line : tokens) { if (!line.empty()) { fprintf(stderr, "%s%s%s%s\n", diff --git a/common/utils/file-wait.cpp b/common/utils/file-wait.cpp index c903780..cc468e3 100644 --- a/common/utils/file-wait.cpp +++ b/common/utils/file-wait.cpp @@ -22,6 +22,7 @@ * @brief Wait for file utility function */ +#include "utils/exception.hpp" #include "utils/file-wait.hpp" #include @@ -42,11 +43,11 @@ void waitForFile(const std::string& filename, const unsigned int timeoutMs) unsigned int loops = 0; while (stat(filename.c_str(), &s) == -1) { if (errno != ENOENT) { - throw std::runtime_error("file access error: " + filename); + throw UtilsException("file access error: " + filename); } ++ loops; if (loops * GRANULARITY > timeoutMs) { - throw std::runtime_error("timeout on waiting for: " + filename); + throw UtilsException("timeout on waiting for: " + filename); } usleep(GRANULARITY * 1000); } diff --git a/common/utils/fs.cpp b/common/utils/fs.cpp index 2b0b681..0446630 100644 --- a/common/utils/fs.cpp +++ b/common/utils/fs.cpp @@ -101,7 +101,7 @@ bool createDirectories(const std::string& path, mode_t mode) bool mountTmpfs(const std::string& path) { - if (::mount("tmpfs", path.c_str(), "tmpfs", MS_NOSUID|MS_NODEV, "mode=755") != 0) { + if (::mount("tmpfs", path.c_str(), "tmpfs", MS_NOSUID | MS_NODEV, "mode=755") != 0) { LOGD("Mount failed for '" << path << "': " << strerror(errno)); return false; } diff --git a/common/utils/glib-loop.cpp b/common/utils/glib-loop.cpp index 7946c05..193a25f 100644 --- a/common/utils/glib-loop.cpp +++ b/common/utils/glib-loop.cpp @@ -23,8 +23,7 @@ */ #include "utils/glib-loop.hpp" - -#include +#include "utils/callback-wrapper.hpp" namespace security_containers { @@ -34,7 +33,9 @@ namespace utils { ScopedGlibLoop::ScopedGlibLoop() : mLoop(g_main_loop_new(NULL, FALSE), g_main_loop_unref) { - mLoopThread = std::thread([this] {g_main_loop_run(mLoop.get());}); + mLoopThread = std::thread([this] { + g_main_loop_run(mLoop.get()); + }); } ScopedGlibLoop::~ScopedGlibLoop() @@ -48,6 +49,26 @@ ScopedGlibLoop::~ScopedGlibLoop() mLoopThread.join(); } +void Glib::addTimerEvent(const unsigned int intervalMs, + const OnTimerEventCallback& callback, + const CallbackGuard& guard) +{ + g_timeout_add_full(G_PRIORITY_DEFAULT, + intervalMs, + &Glib::onTimerEvent, + utils::createCallbackWrapper(callback, guard.spawn()), + &utils::deleteCallbackWrapper); +} + +gboolean Glib::onTimerEvent(gpointer data) +{ + const OnTimerEventCallback& callback = getCallbackFromPointer(data); + if (callback) { + return (gboolean)callback(); + } + return FALSE; +} + } // namespace utils } // namespace security_containers diff --git a/common/utils/glib-loop.hpp b/common/utils/glib-loop.hpp index a31099a..38d7020 100644 --- a/common/utils/glib-loop.hpp +++ b/common/utils/glib-loop.hpp @@ -25,11 +25,11 @@ #ifndef COMMON_UTILS_GLIB_LOOP_HPP #define COMMON_UTILS_GLIB_LOOP_HPP +#include "utils/callback-guard.hpp" + #include #include - -struct _GMainLoop; -typedef struct _GMainLoop GMainLoop; +#include namespace security_containers { @@ -56,6 +56,29 @@ private: std::thread mLoopThread; }; +/** + * Miscellaneous helpers for the Glib library + */ +class Glib { +public: + /** + * A user provided function that will be called succesively after an interval has passed. + * + * @return true if the callback is supposed to be called further, + * false if the callback is not to be called anymore and be destroyed. + */ + typedef std::function OnTimerEventCallback; + + /** + * Adds a timer event to the glib main loop. + */ + static void addTimerEvent(const unsigned int intervalMs, + const OnTimerEventCallback& callback, + const CallbackGuard& guard); + +private: + static gboolean onTimerEvent(gpointer data); +}; } // namespace utils } // namespace security_containers diff --git a/common/utils/latch.cpp b/common/utils/latch.cpp index 5dc7da1..a35aee5 100644 --- a/common/utils/latch.cpp +++ b/common/utils/latch.cpp @@ -24,6 +24,8 @@ #include "utils/latch.hpp" +#include + namespace security_containers { namespace utils { @@ -43,22 +45,33 @@ void Latch::set() void Latch::wait() { - std::unique_lock lock(mMutex); - mCondition.wait(lock, [this] {return mCount > 0;}); - --mCount; + waitForN(1); } bool Latch::wait(const unsigned int timeoutMs) { + return waitForN(1, timeoutMs); +} + +void Latch::waitForN(const unsigned int n) +{ + std::unique_lock lock(mMutex); + mCondition.wait(lock, [this, &n] {return mCount >= n;}); + mCount -= n; +} + +bool Latch::waitForN(const unsigned int n, const unsigned int timeoutMs) +{ std::unique_lock lock(mMutex); if (!mCondition.wait_for(lock, std::chrono::milliseconds(timeoutMs), - [this] {return mCount > 0;})) { + [this, &n] {return mCount >= n;})) { return false; } - --mCount; + mCount -= n; return true; } + bool Latch::empty() { std::unique_lock lock(mMutex); diff --git a/common/utils/latch.hpp b/common/utils/latch.hpp index 54239fc..2621d49 100644 --- a/common/utils/latch.hpp +++ b/common/utils/latch.hpp @@ -54,19 +54,37 @@ public: void wait(); /** - * Waits with timeout. - * @return false on timeout + * Waits for a single occurence of event with timeout. + * + * @param timeoutMs timeout in ms to wait for + * @return false on timeout */ bool wait(const unsigned int timeoutMs); /** + * Waits for @ref n occurrences of event. + * + * @param n number of occurences to wait for + */ + void waitForN(const unsigned int n); + + /** + * Waits for @ref n occurences of event with timeout. + * + * @param n number of occurences to wait for + * @param timeoutMs timeout in ms to wait for + * @return false on timeout + */ + bool waitForN(const unsigned int n, const unsigned int timeoutMs); + + /** * Check if there are no pending events. */ bool empty(); private: std::mutex mMutex; std::condition_variable mCondition; - int mCount; + unsigned int mCount; }; diff --git a/server/container-connection-transport.cpp b/server/container-connection-transport.cpp index e1012eb..5842c81 100644 --- a/server/container-connection-transport.cpp +++ b/server/container-connection-transport.cpp @@ -59,6 +59,14 @@ ContainerConnectionTransport::ContainerConnectionTransport(const std::string& ru LOGE("Initialization failed: could not mount " << runMountPoint); throw ContainerConnectionException("Could not mount: " + runMountPoint); } + + // if there is no systemd in the container this dir won't be created automatically + // TODO: will require chown with USER namespace enabled + std::string dbusDirectory = runMountPoint + "/dbus"; + if (!utils::createDirectories(dbusDirectory, 0755)) { + LOGE("Initialization failed: could not create " << dbusDirectory); + throw ContainerConnectionException("Could not create: " + dbusDirectory); + } } diff --git a/server/container-connection.cpp b/server/container-connection.cpp index 80d4be5..8d06803 100644 --- a/server/container-connection.cpp +++ b/server/container-connection.cpp @@ -45,6 +45,7 @@ ContainerConnection::ContainerConnection(const std::string& address, const OnNam , mNameLost(false) { if (address.empty()) { + // TODO: this should throw. Don't return cleanly unless the object is fully usable. LOGW("The connection to the container is disabled"); return; } diff --git a/server/container.cpp b/server/container.cpp index afa9ac8..36d0c34 100644 --- a/server/container.cpp +++ b/server/container.cpp @@ -23,10 +23,12 @@ */ #include "container.hpp" + #include "log/logger.hpp" #include "utils/paths.hpp" #include +#include namespace security_containers { @@ -51,6 +53,14 @@ Container::Container(const std::string& containerConfigPath) Container::~Container() { + // Make sure all OnNameLostCallbacks get finished and no new will + // get called before proceeding further. This guarantees no race + // condition on the mReconnectThread. + mConnection.reset(); + + if (mReconnectThread.joinable()) { + mReconnectThread.join(); + } } std::string Container::getId() @@ -110,7 +120,36 @@ bool Container::isPaused() void Container::onNameLostCallback() { - // TODO: try to reconnect + LOGI(getId() << ": A connection to the DBUS server has been lost, reconnecting..."); + + if (mReconnectThread.joinable()) { + mReconnectThread.join(); + } + mReconnectThread = std::thread(std::bind(&Container::reconnectHandler, this)); +} + +void Container::reconnectHandler() +{ + std::string address; + + mConnection.reset(); + + try { + address = mConnectionTransport->acquireAddress(); + } catch (SecurityContainersException& e) { + LOGE(getId() << "The socket does not exist anymore, something went terribly wrong, stopping the container"); + stop(); // TODO: shutdownOrStop() + return; + } + + try { + mConnection.reset(new ContainerConnection(address, std::bind(&Container::onNameLostCallback, this))); + LOGI(getId() << ": Reconnected"); + } catch (SecurityContainersException& e) { + LOGE(getId() << ": Reconnecting to the DBUS has failed, stopping the container"); + stop(); // TODO: shutdownOrStop() + return; + } } diff --git a/server/container.hpp b/server/container.hpp index 89ef44e..871b802 100644 --- a/server/container.hpp +++ b/server/container.hpp @@ -33,6 +33,7 @@ #include #include +#include namespace security_containers { @@ -98,11 +99,13 @@ public: private: ContainerConfig mConfig; - std::unique_ptr mAdmin; std::unique_ptr mConnectionTransport; + std::unique_ptr mAdmin; std::unique_ptr mConnection; + std::thread mReconnectThread; void onNameLostCallback(); + void reconnectHandler(); }; diff --git a/server/containers-manager.cpp b/server/containers-manager.cpp index 3af8276..2623ba5 100644 --- a/server/containers-manager.cpp +++ b/server/containers-manager.cpp @@ -65,7 +65,7 @@ ContainersManager::~ContainersManager() { LOGD("Destroying ContainersManager object..."); try { - stopAll(); + stopAll(); // TODO: shutdownOrStop() } catch (ServerException& e) { LOGE("Failed to stop all of the containers"); } diff --git a/unit_tests/server/configs/CMakeLists.txt b/unit_tests/server/configs/CMakeLists.txt index 6847cac..e427b68 100644 --- a/unit_tests/server/configs/CMakeLists.txt +++ b/unit_tests/server/configs/CMakeLists.txt @@ -27,6 +27,7 @@ FILE(GLOB manager_manager_CONF ut-containers-manager/*.conf) FILE(GLOB manager_container_CONF ut-containers-manager/containers/*.conf) FILE(GLOB manager_admin_CONF ut-containers-manager/libvirt-config/*.xml) +FILE(GLOB container_CONF ut-container/*.conf) FILE(GLOB container_container_CONF ut-container/containers/*.conf) FILE(GLOB container_admin_CONF ut-container/libvirt-config/*.xml) @@ -43,6 +44,10 @@ CONFIGURE_FILE(ut-container-admin/containers/test.conf.in ${CMAKE_BINARY_DIR}/ut-container-admin/containers/test.conf @ONLY) FILE(GLOB admin_container_CONF_GEN ${CMAKE_BINARY_DIR}/ut-container-admin/containers/*.conf) +CONFIGURE_FILE(ut-container/libvirt-config/test-dbus.xml.in + ${CMAKE_BINARY_DIR}/ut-container/libvirt-config/test-dbus.xml @ONLY) +FILE(GLOB container_admin_CONF_GEN ${CMAKE_BINARY_DIR}/ut-container/libvirt-config/*.xml) + ## Install ##################################################################### INSTALL(FILES ${server_manager_CONF} @@ -59,10 +64,14 @@ INSTALL(FILES ${manager_container_CONF} INSTALL(FILES ${manager_admin_CONF} DESTINATION ${SC_TEST_CONFIG_INSTALL_DIR}/server/ut-containers-manager/libvirt-config) +INSTALL(FILES ${container_CONF} + DESTINATION ${SC_TEST_CONFIG_INSTALL_DIR}/server/ut-container) INSTALL(FILES ${container_container_CONF} DESTINATION ${SC_TEST_CONFIG_INSTALL_DIR}/server/ut-container/containers) INSTALL(FILES ${container_admin_CONF} DESTINATION ${SC_TEST_CONFIG_INSTALL_DIR}/server/ut-container/libvirt-config) +INSTALL(FILES ${container_admin_CONF_GEN} + DESTINATION ${SC_TEST_CONFIG_INSTALL_DIR}/server/ut-container/libvirt-config) INSTALL(FILES ${admin_container_CONF} DESTINATION ${SC_TEST_CONFIG_INSTALL_DIR}/server/ut-container-admin/containers) diff --git a/unit_tests/server/configs/ut-container/containers/test-dbus.conf b/unit_tests/server/configs/ut-container/containers/test-dbus.conf new file mode 100644 index 0000000..a39e267 --- /dev/null +++ b/unit_tests/server/configs/ut-container/containers/test-dbus.conf @@ -0,0 +1,7 @@ +{ + "privilege" : 10, + "config" : "../libvirt-config/test-dbus.xml", + "cpuQuotaForeground" : -1, + "cpuQuotaBackground" : 1000, + "runMountPoint" : "/tmp/ut-container" +} diff --git a/unit_tests/server/configs/ut-container/libvirt-config/test-dbus.xml.in b/unit_tests/server/configs/ut-container/libvirt-config/test-dbus.xml.in new file mode 100644 index 0000000..a4b2d73 --- /dev/null +++ b/unit_tests/server/configs/ut-container/libvirt-config/test-dbus.xml.in @@ -0,0 +1,19 @@ + + ut-container-test-dbus + 35bb7989-f222-4b63-b0b1-facbdb05b495 + 102400 + destroy + + exe + /bin/dbus-daemon + --config-file=@SC_TEST_CONFIG_INSTALL_DIR@/server/ut-container/ut-dbus.conf + --nofork + + + + + + + + + diff --git a/unit_tests/server/configs/ut-container/ut-dbus.conf b/unit_tests/server/configs/ut-container/ut-dbus.conf new file mode 100644 index 0000000..e52a825 --- /dev/null +++ b/unit_tests/server/configs/ut-container/ut-dbus.conf @@ -0,0 +1,18 @@ + + + + + + custom + unix:path=/tmp/ut-container/dbus/system_bus_socket + + + + + + + + + + diff --git a/unit_tests/server/ut-container-connection.cpp b/unit_tests/server/ut-container-connection.cpp index 7721a3a..5f485f4 100644 --- a/unit_tests/server/ut-container-connection.cpp +++ b/unit_tests/server/ut-container-connection.cpp @@ -60,7 +60,6 @@ public: ScopedDbusDaemon() : mTransport(TRANSPORT_MOUNT_POINT) { - utils::createDirectory(TRANSPORT_MOUNT_POINT + "/dbus", 0755); mDaemon.start(DBUS_DAEMON_PROC, DBUS_DAEMON_ARGS); } diff --git a/unit_tests/server/ut-container.cpp b/unit_tests/server/ut-container.cpp index 43fa4f0..69d1a4a 100644 --- a/unit_tests/server/ut-container.cpp +++ b/unit_tests/server/ut-container.cpp @@ -29,6 +29,7 @@ #include "exception.hpp" #include "utils/exception.hpp" +#include "utils/glib-loop.hpp" #include #include @@ -38,11 +39,16 @@ BOOST_AUTO_TEST_SUITE(ContainerSuite) using namespace security_containers; using namespace security_containers::config; +using namespace security_containers::utils; + +namespace { const std::string TEST_CONFIG_PATH = SC_TEST_CONFIG_INSTALL_DIR "/server/ut-container/containers/test.conf"; +const std::string TEST_DBUS_CONFIG_PATH = SC_TEST_CONFIG_INSTALL_DIR "/server/ut-container/containers/test-dbus.conf"; const std::string BUGGY_CONFIG_PATH = SC_TEST_CONFIG_INSTALL_DIR "/server/ut-container/containers/buggy.conf"; const std::string MISSING_CONFIG_PATH = "/this/is/a/missing/file/path/config.conf"; +} // namespace BOOST_AUTO_TEST_CASE(ConstructorTest) { @@ -65,5 +71,26 @@ BOOST_AUTO_TEST_CASE(MissingConfigTest) BOOST_REQUIRE_THROW(Container c(MISSING_CONFIG_PATH), ConfigException); } +BOOST_AUTO_TEST_CASE(StartStopTest) +{ + ScopedGlibLoop loop; + + std::unique_ptr c(new Container(TEST_CONFIG_PATH)); + BOOST_REQUIRE_NO_THROW(c->start()); + BOOST_REQUIRE_NO_THROW(c->stop()); +} + +BOOST_AUTO_TEST_CASE(DbusConnectionTest) +{ + ScopedGlibLoop loop; + + std::unique_ptr c; + BOOST_REQUIRE_NO_THROW(c.reset(new Container(TEST_DBUS_CONFIG_PATH))); + BOOST_REQUIRE_NO_THROW(c->start()); + BOOST_REQUIRE_NO_THROW(c->stop()); +} + +// TODO: DbusReconnectionTest + BOOST_AUTO_TEST_SUITE_END() diff --git a/unit_tests/utils/ut-glib-loop.cpp b/unit_tests/utils/ut-glib-loop.cpp new file mode 100644 index 0000000..f34fdbc --- /dev/null +++ b/unit_tests/utils/ut-glib-loop.cpp @@ -0,0 +1,68 @@ +/* + * Copyright (c) 2014 Samsung Electronics Co., Ltd All Rights Reserved + * + * Contact: Lukasz Pawelczyk + * + * 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 Lukasz Pawelczyk (l.pawelczyk@partner.samsung.com) + * @brief Unit tests of glib-loop + */ + +#include "ut.hpp" + +#include "utils/latch.hpp" +#include "utils/glib-loop.hpp" + +#include + +BOOST_AUTO_TEST_SUITE(UtilsGlibLoopSuite) + +using namespace security_containers; +using namespace security_containers::utils; + + +namespace { + +const unsigned int TIMER_INTERVAL_MS = 10; +const unsigned int TIMER_NUMBER = 5; +const unsigned int TIMER_WAIT_FOR = 2 * TIMER_NUMBER * TIMER_INTERVAL_MS; + +} // namespace + +BOOST_AUTO_TEST_CASE(GlibTimerEventTest) +{ + ScopedGlibLoop loop; + CallbackGuard guard; + Latch latch; + + Glib::OnTimerEventCallback callback = [&] { + static unsigned int counter = 0; + latch.set(); + if (++counter >= TIMER_NUMBER) { + return false; + } + return true; + }; + + Glib::addTimerEvent(TIMER_INTERVAL_MS, callback, guard); + + BOOST_REQUIRE(latch.waitForN(TIMER_NUMBER, TIMER_WAIT_FOR)); + BOOST_REQUIRE(latch.wait(TIMER_WAIT_FOR) == false); +} + +BOOST_AUTO_TEST_SUITE_END() -- 2.7.4 From c61b669a7ba1b614aaf617888b26e2dce4bfbd5f Mon Sep 17 00:00:00 2001 From: Piotr Bartosiewicz Date: Thu, 8 May 2014 12:16:17 +0200 Subject: [PATCH 07/16] Remove some minor clang warnings [Bug/Feature] N/A [Cause] N/A [Solution] N/A [Verification] Build, install, run tests Change-Id: I0afb654f036c46a11946faadaa62dc5bb831d4d6 --- common/libvirt/helpers.cpp | 1 + common/log/backend-stderr.cpp | 6 +++--- common/utils/fs.cpp | 2 +- common/utils/glib-loop.hpp | 4 ++-- server/container-admin.cpp | 6 +++--- server/container-connection.cpp | 2 +- server/container.cpp | 8 ++++---- server/container.hpp | 4 ++-- server/containers-manager.cpp | 2 +- unit_tests/log/ut-logger.cpp | 4 ++++ unit_tests/server/ut-container-admin.cpp | 4 ++++ 11 files changed, 26 insertions(+), 17 deletions(-) diff --git a/common/libvirt/helpers.cpp b/common/libvirt/helpers.cpp index 87c4705..b8f5230 100644 --- a/common/libvirt/helpers.cpp +++ b/common/libvirt/helpers.cpp @@ -22,6 +22,7 @@ * @brief A function helpers for the libvirt library */ +#include "libvirt/helpers.hpp" #include "log/logger.hpp" #include diff --git a/common/log/backend-stderr.cpp b/common/log/backend-stderr.cpp index e91c12d..fe1c5bc 100644 --- a/common/log/backend-stderr.cpp +++ b/common/log/backend-stderr.cpp @@ -46,13 +46,13 @@ void StderrBackend::log(LogLevel logLevel, const std::string defaultColor = LogFormatter::getDefaultConsoleColor(); const std::string header = LogFormatter::getHeader(logLevel, file, line, func); tokenizer tokens(message, charSeparator("\n")); - for (const auto& line : tokens) { - if (!line.empty()) { + for (const auto& messageLine : tokens) { + if (!messageLine.empty()) { fprintf(stderr, "%s%s%s%s\n", logColor.c_str(), header.c_str(), - line.c_str(), + messageLine.c_str(), defaultColor.c_str()); } } diff --git a/common/utils/fs.cpp b/common/utils/fs.cpp index 0446630..b3e15cf 100644 --- a/common/utils/fs.cpp +++ b/common/utils/fs.cpp @@ -52,7 +52,7 @@ std::string readFileContent(const std::string& path) std::string content; t.seekg(0, std::ios::end); - content.reserve(t.tellg()); + content.reserve(static_cast(t.tellg())); t.seekg(0, std::ios::beg); content.assign((std::istreambuf_iterator(t)), std::istreambuf_iterator()); diff --git a/common/utils/glib-loop.hpp b/common/utils/glib-loop.hpp index 38d7020..ca1bd88 100644 --- a/common/utils/glib-loop.hpp +++ b/common/utils/glib-loop.hpp @@ -64,8 +64,8 @@ public: /** * A user provided function that will be called succesively after an interval has passed. * - * @return true if the callback is supposed to be called further, - * false if the callback is not to be called anymore and be destroyed. + * Return true if the callback is supposed to be called further, + * false if the callback is not to be called anymore and be destroyed. */ typedef std::function OnTimerEventCallback; diff --git a/server/container-admin.cpp b/server/container-admin.cpp index 5f8d653..f499326 100644 --- a/server/container-admin.cpp +++ b/server/container-admin.cpp @@ -73,12 +73,12 @@ ContainerAdmin::~ContainerAdmin() LOGD(mId << ": Destroying ContainerAdmin object..."); try { shutdown(); - } catch (ServerException& e) {} + } catch (ServerException&) {} // Try to forcefully stop try { stop(); - } catch (ServerException& e) { + } catch (ServerException&) { LOGE(mId << ": Failed to destroy the container!"); } @@ -255,7 +255,7 @@ void ContainerAdmin::setSchedulerLevel(SchedulerLevel sched) mConfig.cpuQuotaBackground); break; default: - assert(!"Unknown sched parameter value"); + assert(0 && "Unknown sched parameter value"); } } diff --git a/server/container-connection.cpp b/server/container-connection.cpp index 8d06803..d3176e5 100644 --- a/server/container-connection.cpp +++ b/server/container-connection.cpp @@ -109,7 +109,7 @@ void ContainerConnection::onNameLost() mNameLost = true; mNameCondition.notify_one(); - if(mOnNameLostCallback) { + if (mOnNameLostCallback) { mOnNameLostCallback(); } } diff --git a/server/container.cpp b/server/container.cpp index 36d0c34..443b356 100644 --- a/server/container.cpp +++ b/server/container.cpp @@ -63,12 +63,12 @@ Container::~Container() } } -std::string Container::getId() +const std::string& Container::getId() const { return mAdmin->getId(); } -int Container::getPrivilege() +int Container::getPrivilege() const { return mConfig.privilege; } @@ -136,7 +136,7 @@ void Container::reconnectHandler() try { address = mConnectionTransport->acquireAddress(); - } catch (SecurityContainersException& e) { + } catch (SecurityContainersException&) { LOGE(getId() << "The socket does not exist anymore, something went terribly wrong, stopping the container"); stop(); // TODO: shutdownOrStop() return; @@ -145,7 +145,7 @@ void Container::reconnectHandler() try { mConnection.reset(new ContainerConnection(address, std::bind(&Container::onNameLostCallback, this))); LOGI(getId() << ": Reconnected"); - } catch (SecurityContainersException& e) { + } catch (SecurityContainersException&) { LOGE(getId() << ": Reconnecting to the DBUS has failed, stopping the container"); stop(); // TODO: shutdownOrStop() return; diff --git a/server/container.hpp b/server/container.hpp index 871b802..e464ffb 100644 --- a/server/container.hpp +++ b/server/container.hpp @@ -49,12 +49,12 @@ public: /** * Get the container id */ - std::string getId(); + const std::string& getId() const; /** * Get the container privilege */ - int getPrivilege(); + int getPrivilege() const; /** * Boot the container to the background. diff --git a/server/containers-manager.cpp b/server/containers-manager.cpp index 2623ba5..000b014 100644 --- a/server/containers-manager.cpp +++ b/server/containers-manager.cpp @@ -66,7 +66,7 @@ ContainersManager::~ContainersManager() LOGD("Destroying ContainersManager object..."); try { stopAll(); // TODO: shutdownOrStop() - } catch (ServerException& e) { + } catch (ServerException&) { LOGE("Failed to stop all of the containers"); } LOGD("ContainersManager object destroyed"); diff --git a/unit_tests/log/ut-logger.cpp b/unit_tests/log/ut-logger.cpp index 1f4f000..66c698d 100644 --- a/unit_tests/log/ut-logger.cpp +++ b/unit_tests/log/ut-logger.cpp @@ -34,6 +34,8 @@ BOOST_AUTO_TEST_SUITE(LogSuite) using namespace security_containers::log; +namespace { + class StubbedBackend : public LogBackend { public: StubbedBackend(std::ostringstream& s) : mLogStream(s) {} @@ -90,6 +92,8 @@ void exampleTestLogs(void) LOGT("test log trace " << "5"); } +} // namespace + BOOST_AUTO_TEST_CASE(LogLevelSetandGet) { Logger::setLogLevel(LogLevel::TRACE); diff --git a/unit_tests/server/ut-container-admin.cpp b/unit_tests/server/ut-container-admin.cpp index 2e59e63..1beb30d 100644 --- a/unit_tests/server/ut-container-admin.cpp +++ b/unit_tests/server/ut-container-admin.cpp @@ -42,6 +42,8 @@ BOOST_AUTO_TEST_SUITE(ContainerAdminSuite) using namespace security_containers; using namespace security_containers::config; +namespace { + const std::string TEST_CONFIG_PATH = SC_TEST_CONFIG_INSTALL_DIR "/server/ut-container-admin/containers/test.conf"; const std::string BUGGY_CONFIG_PATH = SC_TEST_CONFIG_INSTALL_DIR "/server/ut-container-admin/containers/buggy.conf"; const std::string MISSING_CONFIG_PATH = SC_TEST_CONFIG_INSTALL_DIR "/server/ut-container-admin/containers/missing.conf"; @@ -52,6 +54,8 @@ void ensureStarted() std::this_thread::sleep_for(std::chrono::milliseconds(200)); } +} // namespace + BOOST_AUTO_TEST_CASE(ConstructorTest) { -- 2.7.4 From 07d874514c1b9a3208158b4de8d6a804f43b657e Mon Sep 17 00:00:00 2001 From: Jan Olszak Date: Thu, 8 May 2014 16:00:07 +0200 Subject: [PATCH 08/16] Introduced Container Daemon [Bug/Feature] No way to run arbitrary code in a container [Cause] N/A [Solution] Introduced Container Daemon that provides his API in system dbus of the container. [Verification] Build, install run security-containers-container-daemon dbus-send --system --dest=com.samsung.container.daemon \ --type=method_call --print-reply \ /com/samsung/container/daemon \ com.samsung.container.daemon.GainFocus dbus-send --system --dest=com.samsung.container.daemon \ --type=method_call --print-reply \ /com/samsung/container/daemon \ com.samsung.container.daemon.LoseFocus Change-Id: I557ca0b283f8c542d45238ec0183ee953a277d5e --- CMakeLists.txt | 2 + container-daemon/CMakeLists.txt | 44 ++++++ .../configs/com.samsung.container.daemon.conf | 14 ++ container-daemon/daemon-connection.cpp | 140 +++++++++++++++++++ container-daemon/daemon-connection.hpp | 79 +++++++++++ container-daemon/daemon-dbus-definitions.hpp | 58 ++++++++ container-daemon/daemon.cpp | 64 +++++++++ container-daemon/daemon.hpp | 54 ++++++++ container-daemon/exception.hpp | 47 +++++++ container-daemon/main.cpp | 153 +++++++++++++++++++++ container-daemon/runner.cpp | 88 ++++++++++++ container-daemon/runner.hpp | 56 ++++++++ packaging/security-containers.spec | 17 +++ 13 files changed, 816 insertions(+) create mode 100644 container-daemon/CMakeLists.txt create mode 100644 container-daemon/configs/com.samsung.container.daemon.conf create mode 100644 container-daemon/daemon-connection.cpp create mode 100644 container-daemon/daemon-connection.hpp create mode 100644 container-daemon/daemon-dbus-definitions.hpp create mode 100644 container-daemon/daemon.cpp create mode 100644 container-daemon/daemon.hpp create mode 100644 container-daemon/exception.hpp create mode 100644 container-daemon/main.cpp create mode 100644 container-daemon/runner.cpp create mode 100644 container-daemon/runner.hpp diff --git a/CMakeLists.txt b/CMakeLists.txt index d228123..964ba66 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -58,6 +58,7 @@ ADD_DEFINITIONS(-DPROJECT_SOURCE_DIR="${PROJECT_SOURCE_DIR}") SET(COMMON_FOLDER ${PROJECT_SOURCE_DIR}/common) SET(CLIENT_FOLDER ${PROJECT_SOURCE_DIR}/client) SET(SERVER_FOLDER ${PROJECT_SOURCE_DIR}/server) +SET(CONTAINER_DAEMON_FOLDER ${PROJECT_SOURCE_DIR}/container-daemon) SET(UNIT_TESTS_FOLDER ${PROJECT_SOURCE_DIR}/unit_tests) IF(NOT DEFINED SYSCONF_INSTALL_DIR) @@ -85,5 +86,6 @@ SET(SC_DATA_INSTALL_DIR ${SHARE_INSTALL_PREFIX}/security-containers) ADD_SUBDIRECTORY(${CLIENT_FOLDER}) ADD_SUBDIRECTORY(${SERVER_FOLDER}) +ADD_SUBDIRECTORY(${CONTAINER_DAEMON_FOLDER}) ADD_SUBDIRECTORY(${UNIT_TESTS_FOLDER}) diff --git a/container-daemon/CMakeLists.txt b/container-daemon/CMakeLists.txt new file mode 100644 index 0000000..5cce2ad --- /dev/null +++ b/container-daemon/CMakeLists.txt @@ -0,0 +1,44 @@ +# Copyright (c) 2014 Samsung Electronics Co., Ltd All Rights Reserved +# +# 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 CMakeLists.txt +# @author Jan Olszak (j.olszak@samsung.com) +# + +MESSAGE(STATUS "Generating makefile for the Container Daemon...") +FILE(GLOB project_SRCS *.cpp *.hpp) +FILE(GLOB common_SRCS ${COMMON_FOLDER}/dbus/*.cpp ${COMMON_FOLDER}/dbus/*.hpp + ${COMMON_FOLDER}/log/*.cpp ${COMMON_FOLDER}/log/*.hpp + ${COMMON_FOLDER}/utils/*.cpp ${COMMON_FOLDER}/utils/*.hpp) + +## Setup target ################################################################ +SET(CONTAINER_DAEMON_CODENAME "${PROJECT_NAME}-container-daemon") +ADD_EXECUTABLE(${CONTAINER_DAEMON_CODENAME} ${project_SRCS} ${common_SRCS}) + + +## Link libraries ############################################################## +FIND_PACKAGE (Boost COMPONENTS program_options REQUIRED) + +PKG_CHECK_MODULES(CONTAINER_DAEMON_DEPS REQUIRED gio-2.0 libsystemd-journal) +INCLUDE_DIRECTORIES(${COMMON_FOLDER}) +INCLUDE_DIRECTORIES(SYSTEM ${CONTAINER_DAEMON_DEPS_INCLUDE_DIRS} ${Boost_INCLUDE_DIRS}) +TARGET_LINK_LIBRARIES(${CONTAINER_DAEMON_CODENAME} ${CONTAINER_DAEMON_DEPS_LIBRARIES} ${Boost_LIBRARIES}) + + +## Install ##################################################################### +INSTALL(TARGETS ${CONTAINER_DAEMON_CODENAME} DESTINATION bin) + +INSTALL(FILES configs/com.samsung.container.daemon.conf + DESTINATION /etc/dbus-1/system.d/) diff --git a/container-daemon/configs/com.samsung.container.daemon.conf b/container-daemon/configs/com.samsung.container.daemon.conf new file mode 100644 index 0000000..26dbddd --- /dev/null +++ b/container-daemon/configs/com.samsung.container.daemon.conf @@ -0,0 +1,14 @@ + + + + + + + + + + + + + diff --git a/container-daemon/daemon-connection.cpp b/container-daemon/daemon-connection.cpp new file mode 100644 index 0000000..714d987 --- /dev/null +++ b/container-daemon/daemon-connection.cpp @@ -0,0 +1,140 @@ +/* + * Copyright (c) 2014 Samsung Electronics Co., Ltd All Rights Reserved + * + * Contact: Jan Olszak + * + * 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 Jan Olszak (j.olszak@samsung.com) + * @brief Dbus API for the Container Daemon + */ + +#include "daemon-connection.hpp" +#include "daemon-dbus-definitions.hpp" +#include "exception.hpp" + +#include "log/logger.hpp" + + +namespace security_containers { +namespace container_daemon { + +namespace { + +// Timeout in ms for waiting for dbus name. +// Can happen if glib loop is busy or not present. +const unsigned int NAME_ACQUIRED_TIMEOUT = 5 * 1000; + +} // namespace + + +DaemonConnection::DaemonConnection(const NameLostCallback& nameLostCallback, + const GainFocusCallback& gainFocusCallback, + const LoseFocusCallback& loseFocusCallback) + : mNameAcquired(false) + , mNameLost(false) +{ + LOGD("Connecting to DBUS on system bus"); + mDbusConnection = dbus::DbusConnection::createSystem(); + + LOGD("Setting DBUS name"); + mDbusConnection->setName(container_daemon::api::BUS_NAME, + std::bind(&DaemonConnection::onNameAcquired, this), + std::bind(&DaemonConnection::onNameLost, this)); + + if (!waitForNameAndSetCallback(NAME_ACQUIRED_TIMEOUT, nameLostCallback)) { + LOGE("Could not acquire dbus name: " << container_daemon::api::BUS_NAME); + throw ContainerDaemonException("Could not acquire dbus name: " + container_daemon::api::BUS_NAME); + } + + LOGD("Setting callbacks"); + mGainFocusCallback = gainFocusCallback; + mLoseFocusCallback = loseFocusCallback; + + + LOGD("Registering DBUS interface"); + using namespace std::placeholders; + mDbusConnection->registerObject(container_daemon::api::OBJECT_PATH, + container_daemon::api::DEFINITION, + std::bind(&DaemonConnection::onMessageCall, + this, _1, _2, _3, _4, _5)); + LOGD("Connected"); +} + +DaemonConnection::~DaemonConnection() +{ +} + +bool DaemonConnection::waitForNameAndSetCallback(const unsigned int timeoutMs, + const NameLostCallback& nameLostCallback) +{ + std::unique_lock lock(mNameMutex); + mNameCondition.wait_for(lock, + std::chrono::milliseconds(timeoutMs), + [this] { + return mNameAcquired || mNameLost; + }); + + if (mNameAcquired) { + mNameLostCallback = nameLostCallback; + } + + return mNameAcquired; +} + +void DaemonConnection::onNameAcquired() +{ + std::unique_lock lock(mNameMutex); + mNameAcquired = true; + mNameCondition.notify_one(); +} + +void DaemonConnection::onNameLost() +{ + std::unique_lock lock(mNameMutex); + mNameLost = true; + mNameCondition.notify_one(); + + if (mNameLostCallback) { + mNameLostCallback(); + } +} + +void DaemonConnection::onMessageCall(const std::string& objectPath, + const std::string& interface, + const std::string& methodName, + GVariant* /*parameters*/, + dbus::MethodResultBuilder& result) +{ + if (objectPath != api::OBJECT_PATH || interface != api::INTERFACE) { + return; + } + + if (methodName == api::METHOD_GAIN_FOCUS) { + if (mGainFocusCallback) { + mGainFocusCallback(); + result.setVoid(); + } + } else if (methodName == api::METHOD_LOSE_FOCUS) { + if (mLoseFocusCallback) { + mLoseFocusCallback(); + result.setVoid(); + } + } +} + +} // namespace container_daemon +} // namespace security_containers diff --git a/container-daemon/daemon-connection.hpp b/container-daemon/daemon-connection.hpp new file mode 100644 index 0000000..e2e5154 --- /dev/null +++ b/container-daemon/daemon-connection.hpp @@ -0,0 +1,79 @@ +/* + * Copyright (c) 2014 Samsung Electronics Co., Ltd All Rights Reserved + * + * Contact: Jan Olszak + * + * 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 Jan Olszak (j.olszak@samsung.com) + * @brief Declaration of a class for communication between container and server + */ + + +#ifndef CONTAINER_DAEMON_DAEMON_CONNECTION_HPP +#define CONTAINER_DAEMON_DAEMON_CONNECTION_HPP + +#include "dbus/connection.hpp" + +#include +#include + + +namespace security_containers { +namespace container_daemon { + + +class DaemonConnection { + +public: + typedef std::function NameLostCallback; + typedef std::function GainFocusCallback; + typedef std::function LoseFocusCallback; + + DaemonConnection(const NameLostCallback& nameLostCallback, + const GainFocusCallback& gainFocusCallback, + const LoseFocusCallback& loseFocusCallback); + ~DaemonConnection(); + + +private: + dbus::DbusConnection::Pointer mDbusConnection; + std::mutex mNameMutex; + std::condition_variable mNameCondition; + bool mNameAcquired; + bool mNameLost; + + NameLostCallback mNameLostCallback; + GainFocusCallback mGainFocusCallback; + LoseFocusCallback mLoseFocusCallback; + + void onNameAcquired(); + void onNameLost(); + bool waitForNameAndSetCallback(const unsigned int timeoutMs, const NameLostCallback& callback); + + void onMessageCall(const std::string& objectPath, + const std::string& interface, + const std::string& methodName, + GVariant* parameters, + dbus::MethodResultBuilder& result); +}; + + +} // namespace container_daemon +} // namespace security_containers + + +#endif // CONTAINER_DAEMON_DAEMON_CONNECTION_HPP diff --git a/container-daemon/daemon-dbus-definitions.hpp b/container-daemon/daemon-dbus-definitions.hpp new file mode 100644 index 0000000..785a9cb --- /dev/null +++ b/container-daemon/daemon-dbus-definitions.hpp @@ -0,0 +1,58 @@ +/* + * Copyright (c) 2014 Samsung Electronics Co., Ltd All Rights Reserved + * + * Contact: Jan Olszak + * + * 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 Jan Olszak (j.olszak@samsung.com) + * @brief container-daemon dbus api definitions + */ + +#ifndef CONTAINER_DAEMON_DAEMON_DBUS_DEFINITIONS_HPP +#define CONTAINER_DAEMON_DAEMON_DBUS_DEFINITIONS_HPP + +#include + + +namespace security_containers { +namespace container_daemon { +namespace api { + +const std::string BUS_NAME = "com.samsung.container.daemon"; +const std::string OBJECT_PATH = "/com/samsung/container/daemon"; +const std::string INTERFACE = "com.samsung.container.daemon"; + +const std::string METHOD_GAIN_FOCUS = "GainFocus"; +const std::string METHOD_LOSE_FOCUS = "LoseFocus"; + +const std::string DEFINITION = + "" + " " + " " + " " + " " + " " + " " + ""; + + +} // namespace api +} // namespace container_daemon +} // namespace security_containers + + +#endif // CONTAINER_DAEMON_DAEMON_DBUS_DEFINITIONS_HPP diff --git a/container-daemon/daemon.cpp b/container-daemon/daemon.cpp new file mode 100644 index 0000000..9dd1c7b --- /dev/null +++ b/container-daemon/daemon.cpp @@ -0,0 +1,64 @@ +/* + * Copyright (c) 2014 Samsung Electronics Co., Ltd All Rights Reserved + * + * Contact: Jan Olszak (j.olszak@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 Jan Olszak (j.olszak@samsung.com) + * @brief Definition of the Daemon, implementation of the logic of the daemon. + */ + +#include "daemon.hpp" + +#include "log/logger.hpp" + + +namespace security_containers { +namespace container_daemon { + + +Daemon::Daemon() +{ + mConnectionPtr.reset(new DaemonConnection(std::bind(&Daemon::onNameLostCallback, this), + std::bind(&Daemon::onGainFocusCallback, this), + std::bind(&Daemon::onLoseFocusCallback, this))); + +} + +Daemon::~Daemon() +{ +} + +void Daemon::onNameLostCallback() +{ + //TODO: Try to reconnect or close the daemon. + LOGE("Dbus name lost"); +} + +void Daemon::onGainFocusCallback() +{ + LOGD("Gained Focus"); +} + +void Daemon::onLoseFocusCallback() +{ + LOGD("Lost Focus"); + +} + +} // namespace container_daemon +} // namespace security_containers diff --git a/container-daemon/daemon.hpp b/container-daemon/daemon.hpp new file mode 100644 index 0000000..1731f6a --- /dev/null +++ b/container-daemon/daemon.hpp @@ -0,0 +1,54 @@ +/* + * Copyright (c) 2014 Samsung Electronics Co., Ltd All Rights Reserved + * + * Contact: Jan Olszak + * + * 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 Jan Olszak (j.olszak@samsung.com) + * @brief Declaration of the Daemon, implementation of the logic of the daemon. + */ + + +#ifndef CONTAINER_DAEMON_DAEMON_HPP +#define CONTAINER_DAEMON_DAEMON_HPP + +#include "daemon-connection.hpp" + +#include + + +namespace security_containers { +namespace container_daemon { + +class Daemon { +public: + Daemon(); + virtual ~Daemon(); + +private: + void onNameLostCallback(); + void onGainFocusCallback(); + void onLoseFocusCallback(); + std::unique_ptr mConnectionPtr; +}; + + +} // namespace container_daemon +} // namespace security_containers + + +#endif // CONTAINER_DAEMON_DAEMON_HPP diff --git a/container-daemon/exception.hpp b/container-daemon/exception.hpp new file mode 100644 index 0000000..2cd4d12 --- /dev/null +++ b/container-daemon/exception.hpp @@ -0,0 +1,47 @@ +/* + * Copyright (c) 2014 Samsung Electronics Co., Ltd All Rights Reserved + * + * Contact: Jan Olszak + * + * 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 Jan Olszak (j.olszak@samsung.com) + * @brief Exceptions for the container-daemon + */ + + +#ifndef CONTAINER_DAEMON_EXCEPTION_HPP +#define CONTAINER_DAEMON_EXCEPTION_HPP + +#include "base-exception.hpp" + + +namespace security_containers { +namespace container_daemon { + +/** + * Base class for exceptions in Security Containers Container Daemon + */ +struct ContainerDaemonException: public SecurityContainersException { + using SecurityContainersException::SecurityContainersException; +}; + + +} // container_daemon +} // security_containers + + +#endif // CONTAINER_DAEMON_EXCEPTION_HPP diff --git a/container-daemon/main.cpp b/container-daemon/main.cpp new file mode 100644 index 0000000..40e7fd9 --- /dev/null +++ b/container-daemon/main.cpp @@ -0,0 +1,153 @@ +/* + * Copyright (c) 2014 Samsung Electronics Co., Ltd All Rights Reserved + * + * Contact: Jan Olszak + * + * 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 Jan Olszak (j.olszak@samsung.com) + * @brief Main file for the Security Containers Daemon + */ + +// Always log to console in DEBUG mode +#if !defined(LOG_TO_CONSOLE) && !defined(NDEBUG) +#define LOG_TO_CONSOLE +#endif + +#include "log/logger.hpp" +#include "log/backend-stderr.hpp" +#include "log/backend-journal.hpp" +#include "utils/typeinfo.hpp" + +#include "exception.hpp" +#include "runner.hpp" + +#include +#include +#include + +using namespace security_containers::log; +using namespace security_containers; + +namespace po = boost::program_options; + + +namespace { + +const std::string PROGRAM_NAME_AND_VERSION = + "Security Containers Containers Daemon " PROGRAM_VERSION; + +/** + * TODO: This is a copied function, move to common/log + * Resolve if given log severity level is valid + * + * @param s log severity level + * @return LogLevel when valid, + * otherwise exception po::validation_error::invalid_option_value thrown + */ +LogLevel validateLogLevel(const std::string& s) +{ + std::string s_capitalized = boost::to_upper_copy(s); + + if (s_capitalized == "ERROR") { + return LogLevel::ERROR; + } else if (s_capitalized == "WARN") { + return LogLevel::WARN; + } else if (s_capitalized == "INFO") { + return LogLevel::INFO; + } else if (s_capitalized == "DEBUG") { + return LogLevel::DEBUG; + } else if (s_capitalized == "TRACE") { + return LogLevel::TRACE; + } else { + throw po::validation_error(po::validation_error::invalid_option_value); + } +} + +} // namespace + + +int main(int argc, char* argv[]) +{ + std::string configPath ; + + try { + po::options_description desc("Allowed options"); + + desc.add_options() + ("help,h", "print this help") + ("version,v", "show application version") + ("log-level,l", po::value()->default_value("DEBUG"), "set log level") + ; + + po::variables_map vm; + po::basic_parsed_options< char > parsed = + po::command_line_parser(argc, argv).options(desc).allow_unregistered().run(); + + std::vector unrecognized_options = + po::collect_unrecognized(parsed.options, po::include_positional); + + if (!unrecognized_options.empty()) { + std::cerr << "Unrecognized options: "; + + for (auto& uo : unrecognized_options) { + std::cerr << ' ' << uo; + } + + std::cerr << std::endl << std::endl; + std::cerr << desc << std::endl; + + return 1; + } + + po::store(parsed, vm); + po::notify(vm); + + if (vm.count("help")) { + std::cout << desc << std::endl; + return 0; + } else if (vm.count("version")) { + std::cout << PROGRAM_NAME_AND_VERSION << std::endl; + return 0; + } + + LogLevel level = validateLogLevel(vm["log-level"].as()); + Logger::setLogLevel(level); +#ifdef LOG_TO_CONSOLE + Logger::setLogBackend(new StderrBackend()); +#else + Logger::setLogBackend(new SystemdJournalBackend()); +#endif + + + } catch (std::exception& e) { + std::cerr << e.what() << std::endl; + return 1; + } + + try { + container_daemon::Runner daemon; + daemon.run(); + + } catch (std::exception& e) { + LOGE("Unexpected: " << utils::getTypeName(e) << ": " << e.what()); + return 1; + } + + return 0; +} + diff --git a/container-daemon/runner.cpp b/container-daemon/runner.cpp new file mode 100644 index 0000000..5c5f9b0 --- /dev/null +++ b/container-daemon/runner.cpp @@ -0,0 +1,88 @@ +/* + * Copyright (c) 2014 Samsung Electronics Co., Ltd All Rights Reserved + * + * Contact: Jan Olszak (j.olszak@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 Jan Olszak (j.olszak@samsung.com) + * @brief Definition of the Runner class, that manages daemon's lifetime + */ + +#include "runner.hpp" +#include "daemon.hpp" + +#include "log/logger.hpp" +#include "utils/glib-loop.hpp" +#include "utils/latch.hpp" + +#include + + +namespace security_containers { +namespace container_daemon { + + +Runner::Runner() +{ +} + + +Runner::~Runner() +{ +} + + +namespace { +utils::Latch gSignalLatch; + +void signalHandler(const int sig) +{ + LOGI("Got signal " << sig); + gSignalLatch.set(); +} +} // namespace + + +void Runner::run() +{ + signal(SIGINT, signalHandler); + signal(SIGTERM, signalHandler); + + LOGI("Starting Container Daemon..."); + { + utils::ScopedGlibLoop loop; + LOGI("Container Daemon started"); + + // Connects to dbus and registers API + container_daemon::Daemon daemon; + + gSignalLatch.wait(); + + LOGI("Stopping Container Daemon..."); + + } + LOGI("Daemon stopped"); +} + +void Runner::terminate() +{ + LOGI("Terminating Container Daemon"); + gSignalLatch.set(); +} + +} // namespace container_daemon +} // namespace security_containers diff --git a/container-daemon/runner.hpp b/container-daemon/runner.hpp new file mode 100644 index 0000000..cdc0913 --- /dev/null +++ b/container-daemon/runner.hpp @@ -0,0 +1,56 @@ +/* + * Copyright (c) 2014 Samsung Electronics Co., Ltd All Rights Reserved + * + * Contact: Jan Olszak + * + * 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 Jan Olszak (j.olszak@samsung.com) + * @brief Declaration of the Runner class, that manages daemon's lifetime + */ + + +#ifndef CONTAINER_DAEMON_RUNNER_HPP +#define CONTAINER_DAEMON_RUNNER_HPP + + +namespace security_containers { +namespace container_daemon { + + +class Runner { +public: + Runner(); + virtual ~Runner(); + + /** + * Starts all the daemon and blocks until SIGINT or SIGTERM + */ + void run(); + + /** + * Terminates the daemon. + * Equivalent of sending SIGINT or SIGTERM signal + */ + void terminate(); +}; + + +} // namespace container_daemon +} // namespace security_containers + + +#endif // CONTAINER_DAEMON_RUNNER_HPP diff --git a/packaging/security-containers.spec b/packaging/security-containers.spec index a3a042e..d30b3a4 100644 --- a/packaging/security-containers.spec +++ b/packaging/security-containers.spec @@ -83,6 +83,23 @@ Development package including the header files for the client library %{_libdir}/pkgconfig/* +## Container Daemon Package #################################################### +%package container-daemon +Summary: Security Containers Containers Daemon +Group: Security/Other +Requires: security-containers = %{version}-%{release} +BuildRequires: pkgconfig(glib-2.0) +BuildRequires: pkgconfig(libsystemd-journal) + +%description container-daemon +Daemon running inside every container. + +%files container-daemon +%defattr(644,root,root,755) +%attr(755,root,root) %{_bindir}/security-containers-container-daemon +/etc/dbus-1/system.d/com.samsung.container.daemon.conf + + ## Test Package ################################################################ %package unit-tests Summary: Security Containers Unit Tests -- 2.7.4 From 44ce3715eb382aa06dd94da3dcfacef7cda9aada Mon Sep 17 00:00:00 2001 From: Jan Olszak Date: Wed, 14 May 2014 16:41:27 +0200 Subject: [PATCH 09/16] Parsing LogLevel in the Logger [Bug/Feature] No way to initialize LogLevel with a string value [Cause] N/A [Solution] Added parsing of LogLevel from std::string [Verification] Build, install and run tests Change-Id: I1f1dada51d0a131d13aa21d6e49e6c4d37ee0f25 --- common/log/backend-journal.cpp | 2 +- common/log/formatter.cpp | 18 ----------- common/log/formatter.hpp | 1 - common/log/level.cpp | 69 ++++++++++++++++++++++++++++++++++++++++++ common/log/level.hpp | 17 ++++++++++- common/log/logger.cpp | 7 ++++- common/log/logger.hpp | 3 +- container-daemon/main.cpp | 31 +------------------ server/main.cpp | 30 +----------------- unit_tests/log/ut-logger.cpp | 26 +++++++++++++++- 10 files changed, 121 insertions(+), 83 deletions(-) create mode 100644 common/log/level.cpp diff --git a/common/log/backend-journal.cpp b/common/log/backend-journal.cpp index 9a00470..032ab9b 100644 --- a/common/log/backend-journal.cpp +++ b/common/log/backend-journal.cpp @@ -37,7 +37,7 @@ void SystemdJournalBackend::log(LogLevel logLevel, const std::string& message) { #define SD_JOURNAL_SUPPRESS_LOCATION - sd_journal_send("PRIORITY=%s", LogFormatter::toString(logLevel).c_str(), + sd_journal_send("PRIORITY=%s", toString(logLevel).c_str(), "CODE_FILE=%s", file.c_str(), "CODE_LINE=%d", line, "CODE_FUNC=%s", func.c_str(), diff --git a/common/log/formatter.cpp b/common/log/formatter.cpp index a8f6384..8ac3581 100644 --- a/common/log/formatter.cpp +++ b/common/log/formatter.cpp @@ -77,24 +77,6 @@ std::string LogFormatter::getDefaultConsoleColor(void) return getConsoleEscapeSequence(Attributes::DEFAULT, Color::DEFAULT); } -std::string LogFormatter::toString(LogLevel logLevel) -{ - switch (logLevel) { - case LogLevel::ERROR: - return "ERROR"; - case LogLevel::WARN: - return "WARN"; - case LogLevel::INFO: - return "INFO"; - case LogLevel::DEBUG: - return "DEBUG"; - case LogLevel::TRACE: - return "TRACE"; - default: - return "UNKNOWN"; - } -} - std::string LogFormatter::stripProjectDir(const std::string& file) { const std::string SOURCE_DIR = PROJECT_SOURCE_DIR "/"; diff --git a/common/log/formatter.hpp b/common/log/formatter.hpp index 6cc783d..3667ecd 100644 --- a/common/log/formatter.hpp +++ b/common/log/formatter.hpp @@ -37,7 +37,6 @@ public: static std::string getCurrentTime(void); static std::string getConsoleColor(LogLevel logLevel); static std::string getDefaultConsoleColor(void); - static std::string toString(LogLevel logLevel); static std::string stripProjectDir(const std::string& file); static std::string getHeader(LogLevel logLevel, const std::string& file, diff --git a/common/log/level.cpp b/common/log/level.cpp new file mode 100644 index 0000000..5fc3c2a --- /dev/null +++ b/common/log/level.cpp @@ -0,0 +1,69 @@ +/* + * Copyright (c) 2014 Samsung Electronics Co., Ltd All Rights Reserved + * + * Contact: Jan Olszak + * + * 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 Jan Olszak (j.olszak@samsung.com) + * @brief Functions to handle LogLevel + */ + + +#include "log/level.hpp" + +#include +#include + +namespace security_containers { +namespace log { + +LogLevel parseLogLevel(const std::string& level) +{ + if (boost::iequals(level, "ERROR")) { + return LogLevel::ERROR; + } else if (boost::iequals(level, "WARN")) { + return LogLevel::WARN; + } else if (boost::iequals(level, "INFO")) { + return LogLevel::INFO; + } else if (boost::iequals(level, "DEBUG")) { + return LogLevel::DEBUG; + } else if (boost::iequals(level, "TRACE")) { + return LogLevel::TRACE; + } else { + throw std::runtime_error("Invalid LogLevel to parse"); + } +} + +std::string toString(const LogLevel logLevel) +{ + switch (logLevel) { + case LogLevel::ERROR: + return "ERROR"; + case LogLevel::WARN: + return "WARN"; + case LogLevel::INFO: + return "INFO"; + case LogLevel::DEBUG: + return "DEBUG"; + case LogLevel::TRACE: + return "TRACE"; + default: + return "UNKNOWN"; + } +} +} // namespace log +} // namespace security_containers diff --git a/common/log/level.hpp b/common/log/level.hpp index 60d8adf..6a54cda 100644 --- a/common/log/level.hpp +++ b/common/log/level.hpp @@ -25,6 +25,9 @@ #ifndef COMMON_LOG_LEVEL_HPP #define COMMON_LOG_LEVEL_HPP +#include + + namespace security_containers { namespace log { @@ -36,8 +39,20 @@ enum class LogLevel { ERROR }; +/** + * @param logLevel LogLevel + * @return std::sting representation of the LogLevel value + */ +std::string toString(const LogLevel logLevel); + +/** + * @param level string representation of log level + * @return parsed LogLevel value + */ +LogLevel parseLogLevel(const std::string& level); + + } // namespace log } // namespace security_containers #endif // COMMON_LOG_LEVEL_HPP - diff --git a/common/log/logger.cpp b/common/log/logger.cpp index 3bb58b3..f834557 100644 --- a/common/log/logger.cpp +++ b/common/log/logger.cpp @@ -59,11 +59,16 @@ void Logger::logMessage(const std::string& message) gLogBackendPtr->log(mLogLevel, mFile, mLine, mFunc, message); } -void Logger::setLogLevel(LogLevel level) +void Logger::setLogLevel(const LogLevel level) { gLogLevel = level; } +void Logger::setLogLevel(const std::string& level) +{ + gLogLevel = parseLogLevel(level); +} + LogLevel Logger::getLogLevel(void) { return gLogLevel; diff --git a/common/log/logger.hpp b/common/log/logger.hpp index 91a4578..45c5f26 100644 --- a/common/log/logger.hpp +++ b/common/log/logger.hpp @@ -46,7 +46,8 @@ public: void logMessage(const std::string& message); - static void setLogLevel(LogLevel level); + static void setLogLevel(const LogLevel level); + static void setLogLevel(const std::string& level); static LogLevel getLogLevel(void); static void setLogBackend(LogBackend* pBackend); diff --git a/container-daemon/main.cpp b/container-daemon/main.cpp index 40e7fd9..769de69 100644 --- a/container-daemon/main.cpp +++ b/container-daemon/main.cpp @@ -36,7 +36,6 @@ #include "exception.hpp" #include "runner.hpp" -#include #include #include @@ -51,33 +50,6 @@ namespace { const std::string PROGRAM_NAME_AND_VERSION = "Security Containers Containers Daemon " PROGRAM_VERSION; -/** - * TODO: This is a copied function, move to common/log - * Resolve if given log severity level is valid - * - * @param s log severity level - * @return LogLevel when valid, - * otherwise exception po::validation_error::invalid_option_value thrown - */ -LogLevel validateLogLevel(const std::string& s) -{ - std::string s_capitalized = boost::to_upper_copy(s); - - if (s_capitalized == "ERROR") { - return LogLevel::ERROR; - } else if (s_capitalized == "WARN") { - return LogLevel::WARN; - } else if (s_capitalized == "INFO") { - return LogLevel::INFO; - } else if (s_capitalized == "DEBUG") { - return LogLevel::DEBUG; - } else if (s_capitalized == "TRACE") { - return LogLevel::TRACE; - } else { - throw po::validation_error(po::validation_error::invalid_option_value); - } -} - } // namespace @@ -125,8 +97,7 @@ int main(int argc, char* argv[]) return 0; } - LogLevel level = validateLogLevel(vm["log-level"].as()); - Logger::setLogLevel(level); + Logger::setLogLevel(vm["log-level"].as()); #ifdef LOG_TO_CONSOLE Logger::setLogBackend(new StderrBackend()); #else diff --git a/server/main.cpp b/server/main.cpp index 2b20823..d911071 100644 --- a/server/main.cpp +++ b/server/main.cpp @@ -35,7 +35,6 @@ #include "exception.hpp" #include "server.hpp" -#include #include #include @@ -50,32 +49,6 @@ namespace { const std::string PROGRAM_NAME_AND_VERSION = "Security Containers Server " PROGRAM_VERSION; -/** - * Resolve if given log severity level is valid - * - * @param s log severity level - * @return LogLevel when valid, - * otherwise exception po::validation_error::invalid_option_value thrown - */ -LogLevel validateLogLevel(const std::string& s) -{ - std::string s_capitalized = boost::to_upper_copy(s); - - if (s_capitalized == "ERROR") { - return LogLevel::ERROR; - } else if (s_capitalized == "WARN") { - return LogLevel::WARN; - } else if (s_capitalized == "INFO") { - return LogLevel::INFO; - } else if (s_capitalized == "DEBUG") { - return LogLevel::DEBUG; - } else if (s_capitalized == "TRACE") { - return LogLevel::TRACE; - } else { - throw po::validation_error(po::validation_error::invalid_option_value); - } -} - } // namespace @@ -124,8 +97,7 @@ int main(int argc, char* argv[]) return 0; } - LogLevel level = validateLogLevel(vm["log-level"].as()); - Logger::setLogLevel(level); + Logger::setLogLevel(vm["log-level"].as()); #ifdef LOG_TO_CONSOLE Logger::setLogBackend(new StderrBackend()); #else diff --git a/unit_tests/log/ut-logger.cpp b/unit_tests/log/ut-logger.cpp index 66c698d..696bda5 100644 --- a/unit_tests/log/ut-logger.cpp +++ b/unit_tests/log/ut-logger.cpp @@ -29,6 +29,7 @@ #include "log/backend.hpp" #include "log/backend-stderr.hpp" +#include BOOST_AUTO_TEST_SUITE(LogSuite) @@ -47,7 +48,7 @@ public: const std::string& func, const std::string& message) override { - mLogStream << '[' + LogFormatter::toString(logLevel) + ']' + ' ' + mLogStream << '[' + toString(logLevel) + ']' + ' ' << file + ':' + std::to_string(line) + ' ' + func + ':' << message << std::endl; } @@ -112,6 +113,29 @@ BOOST_AUTO_TEST_CASE(LogLevelSetandGet) BOOST_CHECK(LogLevel::ERROR == Logger::getLogLevel()); } +BOOST_AUTO_TEST_CASE(StringLogLevelSetandGet) +{ + Logger::setLogLevel("TRACE"); + BOOST_CHECK(LogLevel::TRACE == Logger::getLogLevel()); + + Logger::setLogLevel("traCE"); + BOOST_CHECK(LogLevel::TRACE == Logger::getLogLevel()); + + Logger::setLogLevel("DEBUG"); + BOOST_CHECK(LogLevel::DEBUG == Logger::getLogLevel()); + + Logger::setLogLevel("INFO"); + BOOST_CHECK(LogLevel::INFO == Logger::getLogLevel()); + + Logger::setLogLevel("WARN"); + BOOST_CHECK(LogLevel::WARN == Logger::getLogLevel()); + + Logger::setLogLevel("ERROR"); + BOOST_CHECK(LogLevel::ERROR == Logger::getLogLevel()); + + BOOST_REQUIRE_THROW(Logger::setLogLevel("UNKNOWN"), std::runtime_error); +} + BOOST_AUTO_TEST_CASE(TestLogsError) { TestLog tf(LogLevel::ERROR); -- 2.7.4 From e5d1822e62bf0009cb956bad151c7436877156f5 Mon Sep 17 00:00:00 2001 From: Piotr Bartosiewicz Date: Thu, 15 May 2014 12:11:18 +0200 Subject: [PATCH 10/16] Add SMACK manifests [Bug/Feature] Manifests were not present [Cause] N/A [Solution] N/A [Verification] Build, install, verity on target using command: rpm -q --qf '%{SECMANIFEST}' pkg_name | base64 -d where pkg_name is all of our security-containers* Change-Id: I134055ea328b0f8b76e090dc33b1a3152d96dd0f --- packaging/libsecurity-containers-client.manifest | 5 +++++ packaging/security-containers-container-daemon.manifest | 5 +++++ packaging/security-containers-server-unit-tests.manifest | 5 +++++ packaging/security-containers.manifest | 5 +++++ packaging/security-containers.spec | 5 +++++ 5 files changed, 25 insertions(+) create mode 100644 packaging/libsecurity-containers-client.manifest create mode 100644 packaging/security-containers-container-daemon.manifest create mode 100644 packaging/security-containers-server-unit-tests.manifest create mode 100644 packaging/security-containers.manifest diff --git a/packaging/libsecurity-containers-client.manifest b/packaging/libsecurity-containers-client.manifest new file mode 100644 index 0000000..2a0cec5 --- /dev/null +++ b/packaging/libsecurity-containers-client.manifest @@ -0,0 +1,5 @@ + + + + + diff --git a/packaging/security-containers-container-daemon.manifest b/packaging/security-containers-container-daemon.manifest new file mode 100644 index 0000000..2a0cec5 --- /dev/null +++ b/packaging/security-containers-container-daemon.manifest @@ -0,0 +1,5 @@ + + + + + diff --git a/packaging/security-containers-server-unit-tests.manifest b/packaging/security-containers-server-unit-tests.manifest new file mode 100644 index 0000000..2a0cec5 --- /dev/null +++ b/packaging/security-containers-server-unit-tests.manifest @@ -0,0 +1,5 @@ + + + + + diff --git a/packaging/security-containers.manifest b/packaging/security-containers.manifest new file mode 100644 index 0000000..2a0cec5 --- /dev/null +++ b/packaging/security-containers.manifest @@ -0,0 +1,5 @@ + + + + + diff --git a/packaging/security-containers.spec b/packaging/security-containers.spec index d30b3a4..707bdcb 100644 --- a/packaging/security-containers.spec +++ b/packaging/security-containers.spec @@ -19,6 +19,7 @@ between them. A process from inside a container can request a switch of context (display, input devices) to the other container. %files +%manifest packaging/security-containers.manifest %defattr(644,root,root,755) %attr(755,root,root) %{_bindir}/security-containers-server %dir /etc/security-containers @@ -64,6 +65,7 @@ Requires(postun): /sbin/ldconfig Library interface to the security-containers daemon %files client +%manifest packaging/libsecurity-containers-client.manifest %attr(644,root,root) %{_libdir}/libsecurity-containers-client.so @@ -78,6 +80,7 @@ Requires: security-containers-client = %{version}-%{release} Development package including the header files for the client library %files devel +%manifest packaging/security-containers.manifest %defattr(644,root,root,755) %{_includedir}/security-containers %{_libdir}/pkgconfig/* @@ -95,6 +98,7 @@ BuildRequires: pkgconfig(libsystemd-journal) Daemon running inside every container. %files container-daemon +%manifest packaging/security-containers-container-daemon.manifest %defattr(644,root,root,755) %attr(755,root,root) %{_bindir}/security-containers-container-daemon /etc/dbus-1/system.d/com.samsung.container.daemon.conf @@ -114,6 +118,7 @@ BuildRequires: boost-devel Unit tests for both: server and client. %files unit-tests +%manifest packaging/security-containers-server-unit-tests.manifest %defattr(644,root,root,755) %attr(755,root,root) %{_bindir}/security-containers-server-unit-tests %attr(755,root,root) %{script_dir}/sc_all_tests.py -- 2.7.4 From 7649171d7f1db162bb743f6b0bf2ed6801535b42 Mon Sep 17 00:00:00 2001 From: Lukasz Pawelczyk Date: Thu, 8 May 2014 12:24:42 +0200 Subject: [PATCH 11/16] Add libvirt's event listeners and use them to implement a graceful stop [Bug/Feature] Orginize container's shutdown process [Cause] Burdello [Solution] Implemented listeners for libvirt's events (lifecycle and reboot) Added libvirt-glib dependency to use glib main loop for those events. Used those listeners to implement a synchronous graceful stop of the container: "try to shutdown, if it wont in 10 seconds, destroy it". Added thread ID to the logger. Organized container related logs a little. [Verification] Built, installed, run tests and the daemon. Change-Id: I3be53a2a46cd130cf414e89b0c47eb1cce74e6b5 Signed-off-by: Lukasz Pawelczyk --- common/libvirt/helpers.cpp | 139 ++++++++++++++++++ common/libvirt/helpers.hpp | 10 ++ common/log/formatter.cpp | 20 ++- common/log/formatter.hpp | 1 + packaging/security-containers.spec | 1 + server/CMakeLists.txt | 2 +- server/container-admin.cpp | 163 +++++++++++++++++++-- server/container-admin.hpp | 100 ++++++++++++- server/container-connection-transport.cpp | 1 + server/container-connection.cpp | 1 + server/container.cpp | 6 +- server/container.hpp | 2 +- server/containers-manager.cpp | 4 +- unit_tests/CMakeLists.txt | 2 +- .../ut-container-admin/libvirt-config/test.xml | 3 +- .../ut-container/libvirt-config/test-dbus.xml.in | 7 +- .../configs/ut-container/libvirt-config/test.xml | 3 +- .../libvirt-config/console1.xml | 2 + .../libvirt-config/console2.xml | 2 + .../libvirt-config/console3.xml | 2 + .../ut-server/libvirt-config/container1.xml | 2 + .../ut-server/libvirt-config/container2.xml | 2 + .../ut-server/libvirt-config/container3.xml | 2 + unit_tests/server/ut-container-admin.cpp | 3 + unit_tests/server/ut-container.cpp | 14 +- unit_tests/server/ut-containers-manager.cpp | 14 +- 26 files changed, 466 insertions(+), 42 deletions(-) diff --git a/common/libvirt/helpers.cpp b/common/libvirt/helpers.cpp index b8f5230..36b49e5 100644 --- a/common/libvirt/helpers.cpp +++ b/common/libvirt/helpers.cpp @@ -27,6 +27,7 @@ #include #include +#include namespace security_containers { @@ -57,6 +58,7 @@ void libvirtInitialize(void) std::call_once(gInitFlag, []() { virInitialize(); virSetErrorFunc(NULL, &libvirtErrorFunction); + gvir_event_register(); }); } @@ -72,6 +74,143 @@ std::string libvirtFormatError(void) return "Libvirt error: " + std::string(error->message); } +std::string libvirtEventToString(const int eventId) +{ + switch(eventId) { + case VIR_DOMAIN_EVENT_DEFINED: + return "Defined"; + case VIR_DOMAIN_EVENT_UNDEFINED: + return "Undefined"; + case VIR_DOMAIN_EVENT_STARTED: + return "Started"; + case VIR_DOMAIN_EVENT_SUSPENDED: + return "Suspended"; + case VIR_DOMAIN_EVENT_RESUMED: + return "Resumed"; + case VIR_DOMAIN_EVENT_STOPPED: + return "Stopped"; + case VIR_DOMAIN_EVENT_SHUTDOWN: + return "Shutdown"; + case VIR_DOMAIN_EVENT_PMSUSPENDED: + return "PM Suspended"; + case VIR_DOMAIN_EVENT_CRASHED: + return "Crashed"; + default: + return "Unknown EventId"; + } +} + +std::string libvirtEventDetailToString(const int eventId, const int detailId) +{ + switch (eventId) { + case VIR_DOMAIN_EVENT_DEFINED: + switch (detailId) { + case VIR_DOMAIN_EVENT_DEFINED_ADDED: + return "Added"; + case VIR_DOMAIN_EVENT_DEFINED_UPDATED: + return "Updated"; + default: + return "Unknown detail"; + } + case VIR_DOMAIN_EVENT_UNDEFINED: + switch (detailId) { + case VIR_DOMAIN_EVENT_UNDEFINED_REMOVED: + return "Removed"; + default: + return "Unknown detail"; + } + case VIR_DOMAIN_EVENT_STARTED: + switch (detailId) { + case VIR_DOMAIN_EVENT_STARTED_BOOTED: + return "Booted"; + case VIR_DOMAIN_EVENT_STARTED_MIGRATED: + return "Migrated"; + case VIR_DOMAIN_EVENT_STARTED_RESTORED: + return "Restored"; + case VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT: + return "From Snapshot"; + case VIR_DOMAIN_EVENT_STARTED_WAKEUP: + return "Wakeup"; + default: + return "Unknown detail"; + } + case VIR_DOMAIN_EVENT_SUSPENDED: + switch (detailId) { + case VIR_DOMAIN_EVENT_SUSPENDED_PAUSED: + return "Paused"; + case VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED: + return "Migrated"; + case VIR_DOMAIN_EVENT_SUSPENDED_IOERROR: + return "IO Error"; + case VIR_DOMAIN_EVENT_SUSPENDED_WATCHDOG: + return "Watchdog"; + case VIR_DOMAIN_EVENT_SUSPENDED_RESTORED: + return "Restored"; + case VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT: + return "From Snapshot"; + case VIR_DOMAIN_EVENT_SUSPENDED_API_ERROR: + return "API Error"; + default: + return "Unknown detail"; + } + case VIR_DOMAIN_EVENT_RESUMED: + switch (detailId) { + case VIR_DOMAIN_EVENT_RESUMED_UNPAUSED: + return "Unpaused"; + case VIR_DOMAIN_EVENT_RESUMED_MIGRATED: + return "Migrated"; + case VIR_DOMAIN_EVENT_RESUMED_FROM_SNAPSHOT: + return "From Snapshot"; + default: + return "Unknown detail"; + } + case VIR_DOMAIN_EVENT_STOPPED: + switch (detailId) { + case VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN: + return "Shutdown"; + case VIR_DOMAIN_EVENT_STOPPED_DESTROYED: + return "Destroyed"; + case VIR_DOMAIN_EVENT_STOPPED_CRASHED: + return "Crashed"; + case VIR_DOMAIN_EVENT_STOPPED_MIGRATED: + return "Migrated"; + case VIR_DOMAIN_EVENT_STOPPED_SAVED: + return "Failed"; + case VIR_DOMAIN_EVENT_STOPPED_FAILED: + return "Failed"; + case VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT: + return "From Snapshot"; + default: + return "Unknown detail"; + } + case VIR_DOMAIN_EVENT_SHUTDOWN: + switch (detailId) { + case VIR_DOMAIN_EVENT_SHUTDOWN_FINISHED: + return "Finished"; + default: + return "Unknown detail"; + } + case VIR_DOMAIN_EVENT_PMSUSPENDED: + switch (detailId) { + case VIR_DOMAIN_EVENT_PMSUSPENDED_MEMORY: + return "Memory"; + case VIR_DOMAIN_EVENT_PMSUSPENDED_DISK: + return "Disk"; + default: + return "Unknown detail"; + } + case VIR_DOMAIN_EVENT_CRASHED: + switch (detailId) { + case VIR_DOMAIN_EVENT_CRASHED_PANICKED: + return "Panicked"; + default: + return "Unknown detail"; + } + default: + return "Unknown event"; + } +} + } // namespace libvirt } // namespace security_containers diff --git a/common/libvirt/helpers.hpp b/common/libvirt/helpers.hpp index 35cac9e..b68dd6e 100644 --- a/common/libvirt/helpers.hpp +++ b/common/libvirt/helpers.hpp @@ -42,6 +42,16 @@ void libvirtInitialize(void); */ std::string libvirtFormatError(void); +/** + * Converts an event ID to an event name. + */ +std::string libvirtEventToString(const int event); + +/** + * Converts an event's detail ID to an event's detail name. + */ +std::string libvirtEventDetailToString(const int event, const int detail); + } // namespace libvirt } // namespace security_containers diff --git a/common/log/formatter.cpp b/common/log/formatter.cpp index 8ac3581..1bfe905 100644 --- a/common/log/formatter.cpp +++ b/common/log/formatter.cpp @@ -29,13 +29,29 @@ #include #include #include +#include +#include namespace security_containers { namespace log { const int TIME_COLUMN_LENGTH = 12; const int SEVERITY_COLUMN_LENGTH = 8; -const int FILE_COLUMN_LENGTH = 52; +const int THREAD_COLUMN_LENGTH = 3; +const int FILE_COLUMN_LENGTH = 60; + +std::atomic gNextThreadId(1); +thread_local unsigned int gThisThreadId(0); + +unsigned int LogFormatter::getCurrentThread(void) +{ + unsigned int id = gThisThreadId; + if (id == 0) { + gThisThreadId = id = gNextThreadId++; + } + + return id; +} std::string LogFormatter::getCurrentTime(void) { @@ -93,6 +109,7 @@ std::string LogFormatter::getHeader(LogLevel logLevel, std::ostringstream logLine; logLine << getCurrentTime() << ' ' << std::left << std::setw(SEVERITY_COLUMN_LENGTH) << '[' + toString(logLevel) + ']' + << std::right << std::setw(THREAD_COLUMN_LENGTH) << getCurrentThread() << ": " << std::left << std::setw(FILE_COLUMN_LENGTH) << file + ':' + std::to_string(line) + ' ' + func + ':'; return logLine.str(); @@ -100,4 +117,3 @@ std::string LogFormatter::getHeader(LogLevel logLevel, } // namespace log } // namespace security_containers - diff --git a/common/log/formatter.hpp b/common/log/formatter.hpp index 3667ecd..7f83ebe 100644 --- a/common/log/formatter.hpp +++ b/common/log/formatter.hpp @@ -34,6 +34,7 @@ namespace log { class LogFormatter { public: + static unsigned int getCurrentThread(void); static std::string getCurrentTime(void); static std::string getConsoleColor(LogLevel logLevel); static std::string getDefaultConsoleColor(void); diff --git a/packaging/security-containers.spec b/packaging/security-containers.spec index 707bdcb..c7e2df0 100644 --- a/packaging/security-containers.spec +++ b/packaging/security-containers.spec @@ -12,6 +12,7 @@ BuildRequires: libvirt-devel BuildRequires: libjson-devel BuildRequires: pkgconfig(glib-2.0) BuildRequires: pkgconfig(libsystemd-journal) +BuildRequires: pkgconfig(libvirt-glib-1.0) %description This package provides a daemon used to manage containers - start, stop and switch diff --git a/server/CMakeLists.txt b/server/CMakeLists.txt index b1ce9f8..020724c 100644 --- a/server/CMakeLists.txt +++ b/server/CMakeLists.txt @@ -30,7 +30,7 @@ ADD_EXECUTABLE(${SERVER_CODENAME} ${project_SRCS} ${common_SRCS}) ## Link libraries ############################################################## FIND_PACKAGE (Boost COMPONENTS program_options REQUIRED) -PKG_CHECK_MODULES(SERVER_DEPS REQUIRED libvirt json gio-2.0 libsystemd-journal) +PKG_CHECK_MODULES(SERVER_DEPS REQUIRED libvirt libvirt-glib-1.0 json gio-2.0 libsystemd-journal) INCLUDE_DIRECTORIES(${COMMON_FOLDER}) INCLUDE_DIRECTORIES(SYSTEM ${SERVER_DEPS_INCLUDE_DIRS} ${Boost_INCLUDE_DIRS}) TARGET_LINK_LIBRARIES(${SERVER_CODENAME} ${SERVER_DEPS_LIBRARIES} ${Boost_LIBRARIES}) diff --git a/server/container-admin.cpp b/server/container-admin.cpp index f499326..0365ba8 100644 --- a/server/container-admin.cpp +++ b/server/container-admin.cpp @@ -28,6 +28,8 @@ #include "libvirt/helpers.hpp" #include "log/logger.hpp" #include "utils/fs.hpp" +#include "utils/latch.hpp" +#include "utils/callback-wrapper.hpp" #include #include @@ -41,6 +43,9 @@ namespace security_containers { namespace { +// TODO: this should be in container's configuration file +const int SHUTDOWN_WAIT = 10 * 1000; + std::string getDomainName(virDomainPtr dom) { assert(dom != NULL); @@ -61,25 +66,62 @@ const std::uint64_t DEFAULT_CPU_SHARES = 1024; const std::uint64_t DEFAULT_VCPU_PERIOD_MS = 100000; ContainerAdmin::ContainerAdmin(ContainerConfig& config) - : mConfig(config), mDom(utils::readFileContent(mConfig.config)), mId(getDomainName(mDom.get())) + : mConfig(config), + mDom(utils::readFileContent(mConfig.config)), + mId(getDomainName(mDom.get())), + mLifecycleCallbackId(-1), + mRebootCallbackId(-1), + mNextIdForListener(0) { LOGD(mId << ": Instantiating ContainerAdmin object"); + + // ContainerAdmin owns those callbacks + mLifecycleCallbackId = virConnectDomainEventRegisterAny(virDomainGetConnect(mDom.get()), + mDom.get(), + VIR_DOMAIN_EVENT_ID_LIFECYCLE, + VIR_DOMAIN_EVENT_CALLBACK(&ContainerAdmin::libvirtLifecycleCallback), + utils::createCallbackWrapper(this, mLibvirtGuard.spawn()), + &utils::deleteCallbackWrapper); + + if (mLifecycleCallbackId < 0) { + LOGE(mId << ": Failed to register a libvirt lifecycle callback"); + throw ContainerOperationException(mId + ": Failed to register a libvirt lifecycle callback"); + } + + mRebootCallbackId = virConnectDomainEventRegisterAny(virDomainGetConnect(mDom.get()), + mDom.get(), + VIR_DOMAIN_EVENT_ID_REBOOT, + VIR_DOMAIN_EVENT_CALLBACK(&ContainerAdmin::libvirtRebootCallback), + utils::createCallbackWrapper(this, mLibvirtGuard.spawn()), + &utils::deleteCallbackWrapper); + + if (mRebootCallbackId < 0) { + LOGE(mId << ": Failed to register a libvirt reboot callback"); + virConnectDomainEventDeregisterAny(virDomainGetConnect(mDom.get()), + mLifecycleCallbackId); + throw ContainerOperationException(mId + ": Failed to register a libvirt reboot callback"); + } } ContainerAdmin::~ContainerAdmin() { - // Try to shutdown - LOGD(mId << ": Destroying ContainerAdmin object..."); - try { - shutdown(); - } catch (ServerException&) {} + // Deregister callbacks + if (mLifecycleCallbackId >= 0) { + virConnectDomainEventDeregisterAny(virDomainGetConnect(mDom.get()), + mLifecycleCallbackId); + } + if (mRebootCallbackId >= 0) { + virConnectDomainEventDeregisterAny(virDomainGetConnect(mDom.get()), + mRebootCallbackId); + } // Try to forcefully stop + LOGD(mId << ": Destroying ContainerAdmin object..."); try { - stop(); + destroy(); } catch (ServerException&) { - LOGE(mId << ": Failed to destroy the container!"); + LOGE(mId << ": Failed to destroy the container"); } LOGD(mId << ": ContainerAdmin object destroyed"); @@ -121,22 +163,60 @@ void ContainerAdmin::stop() { assert(mDom); - LOGD(mId << ": Stopping..."); + LOGD(mId << ": Stopping procedure started..."); + if (isStopped()) { + LOGD(mId << ": Already crashed/down/off - nothing to do"); + return; + } + + utils::Latch stoppedOccured; + + LifecycleListener setStopped = [&](const int eventId, const int detailId) { + if (eventId == VIR_DOMAIN_EVENT_STOPPED) { + if (detailId != VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN) { + LOGW(mId << ": shutdown requested, but the container stopped with a different status: " + << libvirt::libvirtEventDetailToString(eventId, detailId)); + } + stoppedOccured.set(); + } + }; + + ListenerId id = registerListener(setStopped, nullptr); + shutdown(); + bool stopped = stoppedOccured.wait(SHUTDOWN_WAIT); + removeListener(id); + + if (!stopped) { + LOGD(mId << ": waiting for shutdown timed out, destroying"); + destroy(); + } + + LOGD(mId << ": Stopping procedure ended"); +} + + +void ContainerAdmin::destroy() +{ + assert(mDom); + + LOGD(mId << ": Destroying..."); if (isStopped()) { - LOGD(mId << ": Already crashed/down/off - nothing to do..."); + LOGD(mId << ": Already crashed/down/off - nothing to do"); return; } + setSchedulerLevel(SchedulerLevel::FOREGROUND); + // Forceful termination of the guest u_int flags = VIR_DOMAIN_DESTROY_DEFAULT; if (virDomainDestroyFlags(mDom.get(), flags) < 0) { - LOGE(mId << ": Error while stopping the container:\n" + LOGE(mId << ": Error while destroying the container:\n" << libvirt::libvirtFormatError()); throw ContainerOperationException(); } - LOGD(mId << ": Stopped"); + LOGD(mId << ": Destroyed"); } @@ -146,11 +226,11 @@ void ContainerAdmin::shutdown() LOGD(mId << ": Shutting down..."); if (isStopped()) { - LOGD(mId << ": Already crashed/down/off - nothing to do..."); + LOGD(mId << ": Already crashed/down/off - nothing to do"); return; } - resume(); + setSchedulerLevel(SchedulerLevel::FOREGROUND); if (virDomainShutdown(mDom.get()) < 0) { LOGE(mId << ": Error while shutting down the container:\n" @@ -158,7 +238,7 @@ void ContainerAdmin::shutdown() throw ContainerOperationException(); } - LOGD(mId << ": Shut down"); + LOGD(mId << ": Shut down initiated (async)"); } @@ -317,5 +397,58 @@ std::int64_t ContainerAdmin::getSchedulerQuota() return quota; } +int ContainerAdmin::libvirtLifecycleCallback(virConnectPtr /*con*/, + virDomainPtr /*dom*/, + int event, + int detail, + void* opaque) +{ + ContainerAdmin* thisPtr = utils::getCallbackFromPointer(opaque); + + LOGI(thisPtr->getId() + << ": Lifecycle event: " + << libvirt::libvirtEventToString(event) + << ": " + << libvirt::libvirtEventDetailToString(event, detail)); + + std::unique_lock lock(thisPtr->mListenerMutex); + for (auto& it : thisPtr->mLifecycleListeners) { + LifecycleListener f = it.second.get(); + f(event, detail); + } + + // ignored, libvirt's legacy + return 0; +} + +void ContainerAdmin::libvirtRebootCallback(virConnectPtr /*con*/, + virDomainPtr /*dom*/, + void* opaque) +{ + ContainerAdmin* thisPtr = utils::getCallbackFromPointer(opaque); + + LOGI(thisPtr->getId() << ": Reboot event"); + + std::unique_lock lock(thisPtr->mListenerMutex); + for (auto& it : thisPtr->mRebootListeners) { + RebootListener f = it.second.get(); + f(); + } +} + +template<> +ContainerAdmin::ListenerMap& +ContainerAdmin::getListenerMap() +{ + return mLifecycleListeners; +} + +template<> +ContainerAdmin::ListenerMap& +ContainerAdmin::getListenerMap() +{ + return mRebootListeners; +} + } // namespace security_containers diff --git a/server/container-admin.hpp b/server/container-admin.hpp index b01d9e0..add8f9a 100644 --- a/server/container-admin.hpp +++ b/server/container-admin.hpp @@ -28,9 +28,14 @@ #include "container-config.hpp" +#include "utils/callback-guard.hpp" +#include "utils/callback-wrapper.hpp" #include "libvirt/connection.hpp" #include "libvirt/domain.hpp" +#include +#include +#include #include #include #include @@ -47,6 +52,21 @@ enum class SchedulerLevel { class ContainerAdmin { public: + /** + * A listener ID type. + */ + typedef unsigned int ListenerId; + + /** + * A function type used for the lifecycle listener + */ + typedef std::function LifecycleListener; + + /** + * A function type used for the reboot listener + */ + typedef std::function RebootListener; + ContainerAdmin(ContainerConfig& config); virtual ~ContainerAdmin(); @@ -61,14 +81,18 @@ public: void start(); /** - * Forcefully stop the container. + * Try to shutdown the container, if failed, destroy it. */ void stop(); /** + * Forcefully stop the container. + */ + void destroy(); + + /** * Gracefully shutdown the container. - * This method will NOT block until container is shut down, - * because some configurations may ignore this. + * This method will NOT block until container is shut down. */ void shutdown(); @@ -115,6 +139,23 @@ public: */ std::int64_t getSchedulerQuota(); + /** + * Sets a listener for a specific event. + * It's a caller's responsibility to remove the listener + * prior to destroying the object. + * + * @return listener ID that can be used to remove. + */ + template + ListenerId registerListener(const Listener& listener, + const utils::CallbackGuard::Tracker& tracker); + + /** + * Remove a previously registered listener. + */ + template + void removeListener(const ListenerId id); + private: ContainerConfig& mConfig; libvirt::LibvirtDomain mDom; @@ -123,10 +164,63 @@ private: int getState(); // get the libvirt's domain state void setSchedulerParams(std::uint64_t cpuShares, std::uint64_t vcpuPeriod, std::int64_t vcpuQuota); + // for handling libvirt callbacks + utils::CallbackGuard mLibvirtGuard; + int mLifecycleCallbackId; + int mRebootCallbackId; + + // virConnectDomainEventCallback + static int libvirtLifecycleCallback(virConnectPtr con, + virDomainPtr dom, + int event, + int detail, + void* opaque); + + // virConnectDomainEventGenericCallback + static void libvirtRebootCallback(virConnectPtr con, + virDomainPtr dom, + void* opaque); + + // for handling external listeners triggered from libvirt callbacks + // TODO, the Listener type might not be unique, reimplement using proper listeners + template + using ListenerMap = std::map>; + + std::mutex mListenerMutex; + std::atomic mNextIdForListener; + ListenerMap mLifecycleListeners; + ListenerMap mRebootListeners; + + template + ListenerMap& getListenerMap(); }; +template +unsigned int ContainerAdmin::registerListener(const Listener& listener, + const utils::CallbackGuard::Tracker& tracker) +{ + ListenerMap& map = getListenerMap(); + unsigned int id = mNextIdForListener++; + utils::CallbackWrapper wrap(listener, tracker); + + std::unique_lock lock(mListenerMutex); + map.emplace(id, std::move(wrap)); + + return id; } +template +void ContainerAdmin::removeListener(const ContainerAdmin::ListenerId id) +{ + ListenerMap& map = getListenerMap(); + + std::unique_lock lock(mListenerMutex); + map.erase(id); +} + + +} // namespace security_containers + #endif // SERVER_CONTAINER_ADMIN_HPP diff --git a/server/container-connection-transport.cpp b/server/container-connection-transport.cpp index 5842c81..7f4fb5e 100644 --- a/server/container-connection-transport.cpp +++ b/server/container-connection-transport.cpp @@ -36,6 +36,7 @@ namespace { // Timeout in ms for waiting for dbus transport. // Should be very long to ensure dbus in container is ready. +// TODO: this should be in container's configuration file const unsigned int TRANSPORT_READY_TIMEOUT = 2 * 60 * 1000; } // namespace diff --git a/server/container-connection.cpp b/server/container-connection.cpp index d3176e5..b1c1df0 100644 --- a/server/container-connection.cpp +++ b/server/container-connection.cpp @@ -35,6 +35,7 @@ namespace { // Timeout in ms for waiting for dbus name. // Can happen if glib loop is busy or not present. +// TODO: this should be in container's configuration file const unsigned int NAME_ACQUIRED_TIMEOUT = 5 * 1000; } // namespace diff --git a/server/container.cpp b/server/container.cpp index 443b356..4717429 100644 --- a/server/container.cpp +++ b/server/container.cpp @@ -82,7 +82,7 @@ void Container::start() // Send to the background only after we're connected, // otherwise it'd take ages. - LOGD(getId() << ": Sending to the background"); + LOGD(getId() << ": DBUS connected, sending to the background"); goBackground(); } @@ -138,7 +138,7 @@ void Container::reconnectHandler() address = mConnectionTransport->acquireAddress(); } catch (SecurityContainersException&) { LOGE(getId() << "The socket does not exist anymore, something went terribly wrong, stopping the container"); - stop(); // TODO: shutdownOrStop() + stop(); return; } @@ -147,7 +147,7 @@ void Container::reconnectHandler() LOGI(getId() << ": Reconnected"); } catch (SecurityContainersException&) { LOGE(getId() << ": Reconnecting to the DBUS has failed, stopping the container"); - stop(); // TODO: shutdownOrStop() + stop(); return; } } diff --git a/server/container.hpp b/server/container.hpp index e464ffb..5607715 100644 --- a/server/container.hpp +++ b/server/container.hpp @@ -62,7 +62,7 @@ public: void start(); /** - * Forcefully stop the container. + * Try to shutdown the container, if failed, destroy it. */ void stop(); diff --git a/server/containers-manager.cpp b/server/containers-manager.cpp index 000b014..288bdef 100644 --- a/server/containers-manager.cpp +++ b/server/containers-manager.cpp @@ -65,7 +65,7 @@ ContainersManager::~ContainersManager() { LOGD("Destroying ContainersManager object..."); try { - stopAll(); // TODO: shutdownOrStop() + stopAll(); } catch (ServerException&) { LOGE("Failed to stop all of the containers"); } @@ -99,6 +99,7 @@ void ContainersManager::startAll() if (container.first == mConfig.foregroundId) { isForegroundFound = true; + LOGI(container.second->getId() << ": set as the foreground container"); container.second->goForeground(); } } @@ -110,6 +111,7 @@ void ContainersManager::startAll() }); mConfig.foregroundId = foregroundIterator->second->getId(); + LOGI(mConfig.foregroundId << ": no foreground container configured, setting one with highest priority"); foregroundIterator->second->goForeground(); } } diff --git a/unit_tests/CMakeLists.txt b/unit_tests/CMakeLists.txt index 87cb4c8..e865049 100644 --- a/unit_tests/CMakeLists.txt +++ b/unit_tests/CMakeLists.txt @@ -34,7 +34,7 @@ ADD_EXECUTABLE(${UT_SERVER_CODENAME} ${project_SRCS} ${common_SRCS} ${server_SRC ## Link libraries ############################################################## FIND_PACKAGE (Boost COMPONENTS unit_test_framework REQUIRED) -PKG_CHECK_MODULES(UT_SERVER_DEPS REQUIRED libvirt json gio-2.0 libsystemd-journal) +PKG_CHECK_MODULES(UT_SERVER_DEPS REQUIRED libvirt libvirt-glib-1.0 json gio-2.0 libsystemd-journal) INCLUDE_DIRECTORIES(${COMMON_FOLDER} ${SERVER_FOLDER} ${UNIT_TESTS_FOLDER}) INCLUDE_DIRECTORIES(SYSTEM ${UT_SERVER_DEPS_INCLUDE_DIRS} ${Boost_INCLUDE_DIRS}) TARGET_LINK_LIBRARIES(${UT_SERVER_CODENAME} ${UT_SERVER_DEPS_LIBRARIES} ${Boost_LIBRARIES}) diff --git a/unit_tests/server/configs/ut-container-admin/libvirt-config/test.xml b/unit_tests/server/configs/ut-container-admin/libvirt-config/test.xml index 91974d9..ac9916d 100644 --- a/unit_tests/server/configs/ut-container-admin/libvirt-config/test.xml +++ b/unit_tests/server/configs/ut-container-admin/libvirt-config/test.xml @@ -2,10 +2,11 @@ ut-container-admin-test a1299273-bce2-4d1f-8369-54b75a791279 102400 - destroy exe /bin/sh + -c + trap exit SIGTERM; read diff --git a/unit_tests/server/configs/ut-container/libvirt-config/test-dbus.xml.in b/unit_tests/server/configs/ut-container/libvirt-config/test-dbus.xml.in index a4b2d73..678cd20 100644 --- a/unit_tests/server/configs/ut-container/libvirt-config/test-dbus.xml.in +++ b/unit_tests/server/configs/ut-container/libvirt-config/test-dbus.xml.in @@ -2,12 +2,11 @@ ut-container-test-dbus 35bb7989-f222-4b63-b0b1-facbdb05b495 102400 - destroy exe - /bin/dbus-daemon - --config-file=@SC_TEST_CONFIG_INSTALL_DIR@/server/ut-container/ut-dbus.conf - --nofork + /bin/sh + -c + trap exit SIGTERM; /bin/dbus-daemon --config-file=@SC_TEST_CONFIG_INSTALL_DIR@/server/ut-container/ut-dbus.conf --fork; read diff --git a/unit_tests/server/configs/ut-container/libvirt-config/test.xml b/unit_tests/server/configs/ut-container/libvirt-config/test.xml index 06155ae..24b61fe 100644 --- a/unit_tests/server/configs/ut-container/libvirt-config/test.xml +++ b/unit_tests/server/configs/ut-container/libvirt-config/test.xml @@ -2,10 +2,11 @@ ut-container-test be2e7a5e-c59f-4264-aeab-390cedf47922 102400 - destroy exe /bin/sh + -c + trap exit SIGTERM; read diff --git a/unit_tests/server/configs/ut-containers-manager/libvirt-config/console1.xml b/unit_tests/server/configs/ut-containers-manager/libvirt-config/console1.xml index b95b2e0..f732480 100644 --- a/unit_tests/server/configs/ut-containers-manager/libvirt-config/console1.xml +++ b/unit_tests/server/configs/ut-containers-manager/libvirt-config/console1.xml @@ -5,6 +5,8 @@ exe /bin/sh + -c + trap exit SIGTERM; read diff --git a/unit_tests/server/configs/ut-containers-manager/libvirt-config/console2.xml b/unit_tests/server/configs/ut-containers-manager/libvirt-config/console2.xml index abe7e12..880d675 100644 --- a/unit_tests/server/configs/ut-containers-manager/libvirt-config/console2.xml +++ b/unit_tests/server/configs/ut-containers-manager/libvirt-config/console2.xml @@ -5,6 +5,8 @@ exe /bin/sh + -c + trap exit SIGTERM; read diff --git a/unit_tests/server/configs/ut-containers-manager/libvirt-config/console3.xml b/unit_tests/server/configs/ut-containers-manager/libvirt-config/console3.xml index b30e42a..2f5916f 100644 --- a/unit_tests/server/configs/ut-containers-manager/libvirt-config/console3.xml +++ b/unit_tests/server/configs/ut-containers-manager/libvirt-config/console3.xml @@ -5,6 +5,8 @@ exe /bin/sh + -c + trap exit SIGTERM; read diff --git a/unit_tests/server/configs/ut-server/libvirt-config/container1.xml b/unit_tests/server/configs/ut-server/libvirt-config/container1.xml index e2ea46f..0a67236 100644 --- a/unit_tests/server/configs/ut-server/libvirt-config/container1.xml +++ b/unit_tests/server/configs/ut-server/libvirt-config/container1.xml @@ -5,6 +5,8 @@ exe /bin/sh + -c + trap exit SIGTERM; read diff --git a/unit_tests/server/configs/ut-server/libvirt-config/container2.xml b/unit_tests/server/configs/ut-server/libvirt-config/container2.xml index 91bdf05..253c1ba 100644 --- a/unit_tests/server/configs/ut-server/libvirt-config/container2.xml +++ b/unit_tests/server/configs/ut-server/libvirt-config/container2.xml @@ -5,6 +5,8 @@ exe /bin/sh + -c + trap exit SIGTERM; read diff --git a/unit_tests/server/configs/ut-server/libvirt-config/container3.xml b/unit_tests/server/configs/ut-server/libvirt-config/container3.xml index 711fd49..3d65238 100644 --- a/unit_tests/server/configs/ut-server/libvirt-config/container3.xml +++ b/unit_tests/server/configs/ut-server/libvirt-config/container3.xml @@ -5,6 +5,8 @@ exe /bin/sh + -c + trap exit SIGTERM; read diff --git a/unit_tests/server/ut-container-admin.cpp b/unit_tests/server/ut-container-admin.cpp index 1beb30d..0d8cd23 100644 --- a/unit_tests/server/ut-container-admin.cpp +++ b/unit_tests/server/ut-container-admin.cpp @@ -28,6 +28,7 @@ #include "container-admin.hpp" #include "exception.hpp" +#include "utils/glib-loop.hpp" #include "utils/exception.hpp" #include "libvirt/exception.hpp" @@ -93,6 +94,8 @@ BOOST_AUTO_TEST_CASE(StartTest) BOOST_AUTO_TEST_CASE(StopTest) { + utils::ScopedGlibLoop loop; + ContainerConfig config; config.parseFile(TEST_CONFIG_PATH); ContainerAdmin ca(config); BOOST_REQUIRE_NO_THROW(ca.start()); diff --git a/unit_tests/server/ut-container.cpp b/unit_tests/server/ut-container.cpp index 69d1a4a..ee71fe0 100644 --- a/unit_tests/server/ut-container.cpp +++ b/unit_tests/server/ut-container.cpp @@ -35,11 +35,8 @@ #include -BOOST_AUTO_TEST_SUITE(ContainerSuite) - using namespace security_containers; using namespace security_containers::config; -using namespace security_containers::utils; namespace { @@ -48,8 +45,15 @@ const std::string TEST_DBUS_CONFIG_PATH = SC_TEST_CONFIG_INSTALL_DIR "/server/ut const std::string BUGGY_CONFIG_PATH = SC_TEST_CONFIG_INSTALL_DIR "/server/ut-container/containers/buggy.conf"; const std::string MISSING_CONFIG_PATH = "/this/is/a/missing/file/path/config.conf"; +struct Fixture { + utils::ScopedGlibLoop loop; +}; + } // namespace + +BOOST_FIXTURE_TEST_SUITE(ContainerSuite, Fixture) + BOOST_AUTO_TEST_CASE(ConstructorTest) { BOOST_REQUIRE_NO_THROW(Container c(TEST_CONFIG_PATH)); @@ -73,8 +77,6 @@ BOOST_AUTO_TEST_CASE(MissingConfigTest) BOOST_AUTO_TEST_CASE(StartStopTest) { - ScopedGlibLoop loop; - std::unique_ptr c(new Container(TEST_CONFIG_PATH)); BOOST_REQUIRE_NO_THROW(c->start()); BOOST_REQUIRE_NO_THROW(c->stop()); @@ -82,8 +84,6 @@ BOOST_AUTO_TEST_CASE(StartStopTest) BOOST_AUTO_TEST_CASE(DbusConnectionTest) { - ScopedGlibLoop loop; - std::unique_ptr c; BOOST_REQUIRE_NO_THROW(c.reset(new Container(TEST_DBUS_CONFIG_PATH))); BOOST_REQUIRE_NO_THROW(c->start()); diff --git a/unit_tests/server/ut-containers-manager.cpp b/unit_tests/server/ut-containers-manager.cpp index 350154d..ab2bb75 100644 --- a/unit_tests/server/ut-containers-manager.cpp +++ b/unit_tests/server/ut-containers-manager.cpp @@ -28,20 +28,30 @@ #include "containers-manager.hpp" #include "exception.hpp" +#include "utils/glib-loop.hpp" + #include #include -BOOST_AUTO_TEST_SUITE(ContainersManagerSuite) - using namespace security_containers; using namespace security_containers::config; +namespace { + const std::string TEST_CONFIG_PATH = SC_TEST_CONFIG_INSTALL_DIR "/server/ut-containers-manager/test-daemon.conf"; const std::string BUGGY_CONFIG_PATH = SC_TEST_CONFIG_INSTALL_DIR "/server/ut-containers-manager/buggy-daemon.conf"; const std::string BUGGY_FOREGROUND_CONFIG_PATH = SC_TEST_CONFIG_INSTALL_DIR "/server/ut-containers-manager/buggy-foreground-daemon.conf"; const std::string MISSING_CONFIG_PATH = "/this/is/a/missing/file/path/missing-daemon.conf"; +struct Fixture { + utils::ScopedGlibLoop loop; +}; + +} // namespace + + +BOOST_FIXTURE_TEST_SUITE(ContainersManagerSuite, Fixture) BOOST_AUTO_TEST_CASE(ConstructorTest) { -- 2.7.4 From 21fd6f9cc1f696893737980eb68014a606164acc Mon Sep 17 00:00:00 2001 From: Lukasz Pawelczyk Date: Mon, 19 May 2014 13:17:47 +0200 Subject: [PATCH 12/16] Unit tests for ContainerAdmin using and testing the listeners [Bug/Feature] Revamped tests for ContainerAdmin. [Cause] N/A [Solution] They use and test Listeners. New test for the stop() procedure. [Verification] Built, installed and run tests. Change-Id: I76c0a3871298855b8cbdcddbd21e8421887d34ed Signed-off-by: Lukasz Pawelczyk --- server/container-admin.cpp | 2 +- unit_tests/server/configs/CMakeLists.txt | 2 + .../containers/test-no-shutdown.conf.in | 7 + .../libvirt-config/test-no-shutdown.xml | 12 ++ unit_tests/server/ut-container-admin.cpp | 161 +++++++++++++++++++-- unit_tests/server/ut-container.cpp | 2 +- unit_tests/server/ut-containers-manager.cpp | 2 +- 7 files changed, 170 insertions(+), 18 deletions(-) create mode 100644 unit_tests/server/configs/ut-container-admin/containers/test-no-shutdown.conf.in create mode 100644 unit_tests/server/configs/ut-container-admin/libvirt-config/test-no-shutdown.xml diff --git a/server/container-admin.cpp b/server/container-admin.cpp index 0365ba8..808bd76 100644 --- a/server/container-admin.cpp +++ b/server/container-admin.cpp @@ -187,7 +187,7 @@ void ContainerAdmin::stop() removeListener(id); if (!stopped) { - LOGD(mId << ": waiting for shutdown timed out, destroying"); + LOGW(mId << ": Gracefull shutdown timed out, the container is still running, destroying"); destroy(); } diff --git a/unit_tests/server/configs/CMakeLists.txt b/unit_tests/server/configs/CMakeLists.txt index e427b68..37a1a06 100644 --- a/unit_tests/server/configs/CMakeLists.txt +++ b/unit_tests/server/configs/CMakeLists.txt @@ -42,6 +42,8 @@ CONFIGURE_FILE(ut-container-admin/containers/buggy.conf.in ${CMAKE_BINARY_DIR}/ut-container-admin/containers/buggy.conf @ONLY) CONFIGURE_FILE(ut-container-admin/containers/test.conf.in ${CMAKE_BINARY_DIR}/ut-container-admin/containers/test.conf @ONLY) +CONFIGURE_FILE(ut-container-admin/containers/test-no-shutdown.conf.in + ${CMAKE_BINARY_DIR}/ut-container-admin/containers/test-no-shutdown.conf @ONLY) FILE(GLOB admin_container_CONF_GEN ${CMAKE_BINARY_DIR}/ut-container-admin/containers/*.conf) CONFIGURE_FILE(ut-container/libvirt-config/test-dbus.xml.in diff --git a/unit_tests/server/configs/ut-container-admin/containers/test-no-shutdown.conf.in b/unit_tests/server/configs/ut-container-admin/containers/test-no-shutdown.conf.in new file mode 100644 index 0000000..c1d89b9 --- /dev/null +++ b/unit_tests/server/configs/ut-container-admin/containers/test-no-shutdown.conf.in @@ -0,0 +1,7 @@ +{ + "privilege" : 10, + "config" : "@SC_TEST_CONFIG_INSTALL_DIR@/server/ut-container-admin/libvirt-config/test-no-shutdown.xml", + "cpuQuotaForeground" : -1, + "cpuQuotaBackground" : 1000, + "runMountPoint" : "" +} diff --git a/unit_tests/server/configs/ut-container-admin/libvirt-config/test-no-shutdown.xml b/unit_tests/server/configs/ut-container-admin/libvirt-config/test-no-shutdown.xml new file mode 100644 index 0000000..609e155 --- /dev/null +++ b/unit_tests/server/configs/ut-container-admin/libvirt-config/test-no-shutdown.xml @@ -0,0 +1,12 @@ + + ut-container-admin-test + f6924d8d-faa4-4cd3-8b47-383f45a4c0c8 + 102400 + + exe + /bin/sh + + + + + diff --git a/unit_tests/server/ut-container-admin.cpp b/unit_tests/server/ut-container-admin.cpp index 0d8cd23..5d4d09c 100644 --- a/unit_tests/server/ut-container-admin.cpp +++ b/unit_tests/server/ut-container-admin.cpp @@ -28,8 +28,10 @@ #include "container-admin.hpp" #include "exception.hpp" +#include "utils/latch.hpp" #include "utils/glib-loop.hpp" #include "utils/exception.hpp" +#include "utils/callback-guard.hpp" #include "libvirt/exception.hpp" #include @@ -38,16 +40,17 @@ #include -BOOST_AUTO_TEST_SUITE(ContainerAdminSuite) - using namespace security_containers; using namespace security_containers::config; namespace { const std::string TEST_CONFIG_PATH = SC_TEST_CONFIG_INSTALL_DIR "/server/ut-container-admin/containers/test.conf"; +const std::string TEST_NO_SHUTDOWN_CONFIG_PATH = SC_TEST_CONFIG_INSTALL_DIR "/server/ut-container-admin/containers/test-no-shutdown.conf"; const std::string BUGGY_CONFIG_PATH = SC_TEST_CONFIG_INSTALL_DIR "/server/ut-container-admin/containers/buggy.conf"; const std::string MISSING_CONFIG_PATH = SC_TEST_CONFIG_INSTALL_DIR "/server/ut-container-admin/containers/missing.conf"; +const unsigned int WAIT_TIMEOUT = 5 * 1000; +const unsigned int WAIT_STOP_TIMEOUT = 15 * 1000; void ensureStarted() { @@ -55,9 +58,16 @@ void ensureStarted() std::this_thread::sleep_for(std::chrono::milliseconds(200)); } +struct Fixture { + utils::ScopedGlibLoop mLoop; + utils::CallbackGuard mGuard; +}; + } // namespace +BOOST_FIXTURE_TEST_SUITE(ContainerAdminSuite, Fixture) + BOOST_AUTO_TEST_CASE(ConstructorTest) { ContainerConfig config; config.parseFile(TEST_CONFIG_PATH); @@ -85,63 +95,184 @@ BOOST_AUTO_TEST_CASE(MissingConfigTest) BOOST_AUTO_TEST_CASE(StartTest) { + utils::Latch booted; + ContainerAdmin::ListenerId id; ContainerConfig config; config.parseFile(TEST_CONFIG_PATH); ContainerAdmin ca(config); + + ContainerAdmin::LifecycleListener bootedListener = [&](const int event, const int detail) { + if (event == VIR_DOMAIN_EVENT_STARTED && detail == VIR_DOMAIN_EVENT_STARTED_BOOTED) { + booted.set(); + } + }; + BOOST_REQUIRE_NO_THROW(id = ca.registerListener(bootedListener, mGuard.spawn())); + BOOST_REQUIRE_NO_THROW(ca.start()); ensureStarted(); + + BOOST_CHECK(booted.wait(WAIT_TIMEOUT)); BOOST_CHECK(ca.isRunning()); + + BOOST_REQUIRE_NO_THROW(ca.removeListener(id)); } -BOOST_AUTO_TEST_CASE(StopTest) +BOOST_AUTO_TEST_CASE(ShutdownTest) { - utils::ScopedGlibLoop loop; + utils::Latch shutdown; + ContainerAdmin::ListenerId id; + ContainerConfig config; config.parseFile(TEST_CONFIG_PATH); + ContainerAdmin ca(config); + ContainerAdmin::LifecycleListener shutdownListener = [&](const int event, const int detail) { + if (event == VIR_DOMAIN_EVENT_STOPPED && detail == VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN) { + shutdown.set(); + } + }; + + BOOST_REQUIRE_NO_THROW(ca.start()); + ensureStarted(); + BOOST_REQUIRE(ca.isRunning()); + BOOST_REQUIRE_NO_THROW(id = ca.registerListener(shutdownListener, mGuard.spawn())); + + BOOST_REQUIRE_NO_THROW(ca.shutdown()); + BOOST_CHECK(shutdown.wait(WAIT_TIMEOUT)); + BOOST_CHECK(!ca.isRunning()); + BOOST_CHECK(ca.isStopped()); + + BOOST_REQUIRE_NO_THROW(ca.removeListener(id)); +} + +BOOST_AUTO_TEST_CASE(DestroyTest) +{ + utils::Latch destroyed; + ContainerAdmin::ListenerId id; ContainerConfig config; config.parseFile(TEST_CONFIG_PATH); ContainerAdmin ca(config); + + ContainerAdmin::LifecycleListener destroyedListener = [&](const int event, const int detail) { + if (event == VIR_DOMAIN_EVENT_STOPPED && detail == VIR_DOMAIN_EVENT_STOPPED_DESTROYED) { + destroyed.set(); + } + }; + BOOST_REQUIRE_NO_THROW(ca.start()); ensureStarted(); - BOOST_CHECK(ca.isRunning()); - BOOST_REQUIRE_NO_THROW(ca.stop()) + BOOST_REQUIRE(ca.isRunning()); + BOOST_REQUIRE_NO_THROW(id = ca.registerListener(destroyedListener, mGuard.spawn())); + + BOOST_REQUIRE_NO_THROW(ca.destroy()); + BOOST_CHECK(destroyed.wait(WAIT_TIMEOUT)); BOOST_CHECK(!ca.isRunning()); BOOST_CHECK(ca.isStopped()); + + BOOST_REQUIRE_NO_THROW(ca.removeListener(id)); } -BOOST_AUTO_TEST_CASE(ShutdownTest) +BOOST_AUTO_TEST_CASE(StopShutdownTest) { + utils::Latch shutdown; + ContainerAdmin::ListenerId id; ContainerConfig config; config.parseFile(TEST_CONFIG_PATH); ContainerAdmin ca(config); - BOOST_REQUIRE_NO_THROW(ca.start()) + + ContainerAdmin::LifecycleListener shutdownListener = [&](const int event, const int detail) { + if (event == VIR_DOMAIN_EVENT_STOPPED && detail == VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN) { + shutdown.set(); + } + }; + + BOOST_REQUIRE_NO_THROW(ca.start()); ensureStarted(); - BOOST_CHECK(ca.isRunning()); - BOOST_REQUIRE_NO_THROW(ca.shutdown()) - // TODO: For this simple configuration, the shutdown signal is ignored - // BOOST_CHECK(!ca.isRunning()); - // BOOST_CHECK(ca.isStopped()); + BOOST_REQUIRE(ca.isRunning()); + BOOST_REQUIRE_NO_THROW(id = ca.registerListener(shutdownListener, mGuard.spawn())); + + BOOST_REQUIRE_NO_THROW(ca.stop()); + BOOST_CHECK(shutdown.wait(WAIT_TIMEOUT)); + BOOST_CHECK(!ca.isRunning()); + BOOST_CHECK(ca.isStopped()); + + BOOST_REQUIRE_NO_THROW(ca.removeListener(id)); +} + +// This test needs to wait for a shutdown timer in stop() method. This takes 10s+. +BOOST_AUTO_TEST_CASE(StopDestroyTest) +{ + utils::Latch destroyed; + ContainerAdmin::ListenerId id; + ContainerConfig config; config.parseFile(TEST_NO_SHUTDOWN_CONFIG_PATH); + ContainerAdmin ca(config); + + ContainerAdmin::LifecycleListener destroyedListener = [&](const int event, const int detail) { + if (event == VIR_DOMAIN_EVENT_STOPPED && detail == VIR_DOMAIN_EVENT_STOPPED_DESTROYED) { + destroyed.set(); + } + }; + + BOOST_REQUIRE_NO_THROW(ca.start()); + ensureStarted(); + BOOST_REQUIRE(ca.isRunning()); + BOOST_REQUIRE_NO_THROW(id = ca.registerListener(destroyedListener, mGuard.spawn())); + + BOOST_REQUIRE_NO_THROW(ca.stop()); + BOOST_CHECK(destroyed.wait(WAIT_STOP_TIMEOUT)); + BOOST_CHECK(!ca.isRunning()); + BOOST_CHECK(ca.isStopped()); + + BOOST_REQUIRE_NO_THROW(ca.removeListener(id)); } BOOST_AUTO_TEST_CASE(SuspendTest) { + utils::Latch paused; + ContainerAdmin::ListenerId id; ContainerConfig config; config.parseFile(TEST_CONFIG_PATH); ContainerAdmin ca(config); + + ContainerAdmin::LifecycleListener pausedListener = [&](const int event, const int detail) { + if (event == VIR_DOMAIN_EVENT_SUSPENDED && detail == VIR_DOMAIN_EVENT_SUSPENDED_PAUSED) { + paused.set(); + } + }; + BOOST_REQUIRE_NO_THROW(ca.start()) ensureStarted(); - BOOST_CHECK(ca.isRunning()); + BOOST_REQUIRE(ca.isRunning()); + BOOST_REQUIRE_NO_THROW(id = ca.registerListener(pausedListener, mGuard.spawn())); + BOOST_REQUIRE_NO_THROW(ca.suspend()); + BOOST_CHECK(paused.wait(WAIT_TIMEOUT)); BOOST_CHECK(!ca.isRunning()); BOOST_CHECK(ca.isPaused()); + + BOOST_REQUIRE_NO_THROW(ca.removeListener(id)); } BOOST_AUTO_TEST_CASE(ResumeTest) { + utils::Latch unpaused; + ContainerAdmin::ListenerId id; ContainerConfig config; config.parseFile(TEST_CONFIG_PATH); ContainerAdmin ca(config); + + ContainerAdmin::LifecycleListener unpausedListener = [&](const int event, const int detail) { + if (event == VIR_DOMAIN_EVENT_RESUMED && detail == VIR_DOMAIN_EVENT_RESUMED_UNPAUSED) { + unpaused.set(); + } + }; + BOOST_REQUIRE_NO_THROW(ca.start()); ensureStarted(); + BOOST_REQUIRE(ca.isRunning()); BOOST_REQUIRE_NO_THROW(ca.suspend()) - BOOST_CHECK(ca.isPaused()); + BOOST_REQUIRE(ca.isPaused()); + BOOST_REQUIRE_NO_THROW(id = ca.registerListener(unpausedListener, mGuard.spawn())); + BOOST_REQUIRE_NO_THROW(ca.resume()); + BOOST_CHECK(unpaused.wait(WAIT_TIMEOUT)); BOOST_CHECK(!ca.isPaused()); BOOST_CHECK(ca.isRunning()); + + BOOST_REQUIRE_NO_THROW(ca.removeListener(id)); } BOOST_AUTO_TEST_CASE(SchedulerLevelTest) diff --git a/unit_tests/server/ut-container.cpp b/unit_tests/server/ut-container.cpp index ee71fe0..09aa059 100644 --- a/unit_tests/server/ut-container.cpp +++ b/unit_tests/server/ut-container.cpp @@ -46,7 +46,7 @@ const std::string BUGGY_CONFIG_PATH = SC_TEST_CONFIG_INSTALL_DIR "/server/ut-con const std::string MISSING_CONFIG_PATH = "/this/is/a/missing/file/path/config.conf"; struct Fixture { - utils::ScopedGlibLoop loop; + utils::ScopedGlibLoop mLoop; }; } // namespace diff --git a/unit_tests/server/ut-containers-manager.cpp b/unit_tests/server/ut-containers-manager.cpp index ab2bb75..b0a96db 100644 --- a/unit_tests/server/ut-containers-manager.cpp +++ b/unit_tests/server/ut-containers-manager.cpp @@ -45,7 +45,7 @@ const std::string BUGGY_FOREGROUND_CONFIG_PATH = SC_TEST_CONFIG_INSTALL_DIR "/se const std::string MISSING_CONFIG_PATH = "/this/is/a/missing/file/path/missing-daemon.conf"; struct Fixture { - utils::ScopedGlibLoop loop; + utils::ScopedGlibLoop mLoop; }; } // namespace -- 2.7.4 From 15d22682188bd8d8f01b15941370c8ca9f8843d4 Mon Sep 17 00:00:00 2001 From: Michal Witanowski Date: Wed, 16 Apr 2014 11:26:06 +0200 Subject: [PATCH 13/16] Make Security Containers Server a systemd service [Bug/Feature] Create systemd service for SCS launching. [Cause] N/A [Solution] * Needed configs and scripts have been written. * "post" and "postun" sections of the spec file have been filled. [Verification] Build and install on a target that has: * "business" and "private" root filesystems located at /opt/usr/containers/ * libvirtd running as systemd service After the installation, verify: 1. Stopping/starting the service via "systemctl stop/start security-containers.service". 2. Restarting the device (the containers should boot automatically). 3. Killing the Security Containers Server (for example: "kill -11 `pidof security-containers-server`") - the service should restart in this situation. 4. Uninstalling the Security Containers package - the containers should keep on running since next reboot. 5. Upgrading should send SIGUSR1 to the daemon (it will be handled by another commit). Change-Id: I514cc4c447e0f100022b80e2149fc3e228aa5f8a Signed-off-by: Michal Witanowski --- packaging/security-containers.spec | 32 +++++++++++++++++++++- server/configs/CMakeLists.txt | 5 +++- server/configs/systemd/security-containers.service | 13 +++++++++ 3 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 server/configs/systemd/security-containers.service diff --git a/packaging/security-containers.spec b/packaging/security-containers.spec index c7e2df0..23b7bb3 100644 --- a/packaging/security-containers.spec +++ b/packaging/security-containers.spec @@ -28,6 +28,8 @@ between them. A process from inside a container can request a switch of context %dir /etc/security-containers/libvirt-config %config /etc/security-containers/daemon.conf %config /etc/security-containers/containers/*.conf +%{_unitdir}/security-containers.service +%{_unitdir}/multi-user.target.wants/security-containers.service %config %attr(400,root,root) /etc/security-containers/libvirt-config/*.xml /etc/security-containers/image-skel @@ -44,15 +46,43 @@ between them. A process from inside a container can request a switch of context %cmake . -DVERSION=%{version} \ -DCMAKE_BUILD_TYPE=%{build_type} \ - -DSCRIPT_INSTALL_DIR=%{script_dir} + -DSCRIPT_INSTALL_DIR=%{script_dir} \ + -DSYSTEMD_UNIT_DIR=%{_unitdir} make -k %{?jobs:-j%jobs} %install %make_install +mkdir -p %{buildroot}/%{_unitdir}/multi-user.target.wants +ln -s ../security-containers.service %{buildroot}/%{_unitdir}/multi-user.target.wants/security-containers.service %clean rm -rf %{buildroot} +%post +# Refresh systemd services list after installation +if [ $1 == 1 ]; then + systemctl daemon-reload || : +fi + +%preun +# Stop the service before uninstall +if [ $1 == 0 ]; then + systemctl stop security-containers.service || : +fi + +%postun +# Refresh systemd services list after uninstall/upgrade +systemctl daemon-reload || : +if [ $1 -ge 1 ]; then + # TODO: at this point an appropriate notification should show up + eval `systemctl show security-containers --property=MainPID` + if [ -n "$MainPID" -a "$MainPID" != "0" ]; then + kill -USR1 $MainPID + fi + echo "Security Containers updated. Reboot is required for the changes to take effect..." +else + echo "Security Containers removed. Reboot is required for the changes to take effect..." +fi ## Client Package ############################################################## %package client diff --git a/server/configs/CMakeLists.txt b/server/configs/CMakeLists.txt index 11f258e..935cc2f 100644 --- a/server/configs/CMakeLists.txt +++ b/server/configs/CMakeLists.txt @@ -21,7 +21,7 @@ MESSAGE(STATUS "Installing configs to " ${SC_CONFIG_INSTALL_DIR}) FILE(GLOB container_CONF containers/*.conf) FILE(GLOB admin_CONF libvirt-config/*.xml) - +FILE(GLOB SYSTEMD_SERVICES systemd/*.service) ## Installations ############################################################### INSTALL(FILES daemon.conf @@ -36,3 +36,6 @@ INSTALL(FILES ${container_CONF} INSTALL(FILES ${admin_CONF} DESTINATION ${SC_CONFIG_INSTALL_DIR}/libvirt-config) + +INSTALL(FILES ${SYSTEMD_SERVICES} + DESTINATION ${SYSTEMD_UNIT_DIR}) diff --git a/server/configs/systemd/security-containers.service b/server/configs/systemd/security-containers.service new file mode 100644 index 0000000..38e2fd7 --- /dev/null +++ b/server/configs/systemd/security-containers.service @@ -0,0 +1,13 @@ +[Unit] +Description=Security Containers Server +After=libvirtd.service +Requires=libvirtd.service + +[Service] +Type=simple +ExecStart=/usr/bin/security-containers-server +Restart=on-failure +ExecReload=/bin/kill -HUP $MAINPID + +[Install] +WantedBy=multi-user.target -- 2.7.4 From 6e86807780dceb3b2978bf5169fc604b27748d45 Mon Sep 17 00:00:00 2001 From: Lukasz Kostyra Date: Mon, 28 Apr 2014 13:06:41 +0200 Subject: [PATCH 14/16] Reload SCS binary when updating security-containers [Feature] Reload SCS without turning containers off when binary is updated. [Cause] When updating SCS we don't want to restart containers, only SCS itself. [Solution] Add SIGUSR1 handling which will tell SCS to keep containers alive when exiting. Add check in ContainerConnectionTransport if containers are running to skip remount of tmpfs when it is not needed. [Verification] Build, install, reboot target. Test the following when SCS is running together with containers active: * Call "systemctl stop security-containers". SCS should turn off and containers should turn off as well. Call "systemctl start security-containers", SCS and containers should start up. * Simulate update by calling "kill -USR1 `pidof security-containers-server`". SCS should properly reload, however containers should stay on. (note - the best way to check it would be by verifying logs in journalctl). Change-Id: I3a6d0fb25a4579208ad0f6d0de00e2755548230e Signed-off-by: Lukasz Kostyra --- common/utils/fs.cpp | 19 ++++++++++++++ common/utils/fs.hpp | 5 ++++ server/container-admin.cpp | 26 +++++++++++++------- server/container-admin.hpp | 6 +++++ server/container-connection-transport.cpp | 32 +++++++++++++++++------- server/container-connection-transport.hpp | 6 +++++ server/container.cpp | 6 +++++ server/container.hpp | 8 ++++++ server/containers-manager.cpp | 23 +++++++++++++---- server/containers-manager.hpp | 6 +++++ server/main.cpp | 1 + server/server.cpp | 41 +++++++++++++++++++++++++++---- server/server.hpp | 7 +++++- 13 files changed, 157 insertions(+), 29 deletions(-) diff --git a/common/utils/fs.cpp b/common/utils/fs.cpp index b3e15cf..aac496f 100644 --- a/common/utils/fs.cpp +++ b/common/utils/fs.cpp @@ -117,6 +117,25 @@ bool umount(const std::string& path) return true; } +bool isMountPoint(const std::string& path, bool& result) +{ + struct stat stat, parentStat; + std::string parentPath = dirName(path); + + if (::stat(path.c_str(), &stat)) { + LOGD("Failed to get stat of " << path << ": " << strerror(errno)); + return false; + } + + if (::stat(parentPath.c_str(), &parentStat)) { + LOGD("Failed to get stat of " << parentPath << ": " << strerror(errno)); + return false; + } + + result = (stat.st_dev != parentStat.st_dev); + return true; +} + } // namespace utils } // namespace security_containers diff --git a/common/utils/fs.hpp b/common/utils/fs.hpp index 52273dd..95ce200 100644 --- a/common/utils/fs.hpp +++ b/common/utils/fs.hpp @@ -67,6 +67,11 @@ bool mountTmpfs(const std::string& path); */ bool umount(const std::string& path); +/** + * Check if given path is a mount point + */ +bool isMountPoint(const std::string& path, bool& result); + } // namespace utils } // namespace security_containers diff --git a/server/container-admin.cpp b/server/container-admin.cpp index 808bd76..657ab4f 100644 --- a/server/container-admin.cpp +++ b/server/container-admin.cpp @@ -69,6 +69,7 @@ ContainerAdmin::ContainerAdmin(ContainerConfig& config) : mConfig(config), mDom(utils::readFileContent(mConfig.config)), mId(getDomainName(mDom.get())), + mDetachOnExit(false), mLifecycleCallbackId(-1), mRebootCallbackId(-1), mNextIdForListener(0) @@ -106,6 +107,8 @@ ContainerAdmin::ContainerAdmin(ContainerConfig& config) ContainerAdmin::~ContainerAdmin() { + LOGD(mId << ": Destroying ContainerAdmin object..."); + // Deregister callbacks if (mLifecycleCallbackId >= 0) { virConnectDomainEventDeregisterAny(virDomainGetConnect(mDom.get()), @@ -117,11 +120,12 @@ ContainerAdmin::~ContainerAdmin() } // Try to forcefully stop - LOGD(mId << ": Destroying ContainerAdmin object..."); - try { - destroy(); - } catch (ServerException&) { - LOGE(mId << ": Failed to destroy the container"); + if (!mDetachOnExit) { + try { + destroy(); + } catch (ServerException&) { + LOGE(mId << ": Failed to destroy the container"); + } } LOGD(mId << ": ContainerAdmin object destroyed"); @@ -144,10 +148,10 @@ void ContainerAdmin::start() return; } - // Autodestroyed when connection pointer released - // Any managed save file for this domain is discarded, - // and the domain boots from scratch - u_int flags = VIR_DOMAIN_START_AUTODESTROY; + // In order to update daemon without shutting down the containers + // autodestroy option must NOT be set. It's best to create domain + // without any flags. + u_int flags = VIR_DOMAIN_NONE; if (virDomainCreateWithFlags(mDom.get(), flags) < 0) { LOGE(mId << ": Failed to start the container\n" @@ -362,6 +366,10 @@ void ContainerAdmin::setSchedulerParams(std::uint64_t cpuShares, std::uint64_t v } } +void ContainerAdmin::setDetachOnExit() +{ + mDetachOnExit = true; +} std::int64_t ContainerAdmin::getSchedulerQuota() { diff --git a/server/container-admin.hpp b/server/container-admin.hpp index add8f9a..ed1e00e 100644 --- a/server/container-admin.hpp +++ b/server/container-admin.hpp @@ -134,6 +134,11 @@ public: void setSchedulerLevel(SchedulerLevel sched); /** + * Set whether container should be detached on exit. + */ + void setDetachOnExit(); + + /** * @return Scheduler CFS quota, * TODO: this function is only for UNIT TESTS */ @@ -160,6 +165,7 @@ private: ContainerConfig& mConfig; libvirt::LibvirtDomain mDom; const std::string mId; + bool mDetachOnExit; int getState(); // get the libvirt's domain state void setSchedulerParams(std::uint64_t cpuShares, std::uint64_t vcpuPeriod, std::int64_t vcpuQuota); diff --git a/server/container-connection-transport.cpp b/server/container-connection-transport.cpp index 7f4fb5e..3c14a67 100644 --- a/server/container-connection-transport.cpp +++ b/server/container-connection-transport.cpp @@ -43,7 +43,7 @@ const unsigned int TRANSPORT_READY_TIMEOUT = 2 * 60 * 1000; ContainerConnectionTransport::ContainerConnectionTransport(const std::string& runMountPoint) - : mRunMountPoint(runMountPoint) + : mRunMountPoint(runMountPoint), mDetachOnExit(false) { if (runMountPoint.empty()) { return; @@ -53,12 +53,20 @@ ContainerConnectionTransport::ContainerConnectionTransport(const std::string& ru throw ContainerConnectionException("Could not create: " + runMountPoint); } - // try to umount if already mounted - utils::umount(runMountPoint); + bool isMount = false; + if (!utils::isMountPoint(runMountPoint, isMount)) { + LOGE("Failed to check if " << runMountPoint << " is a mount point."); + throw ContainerConnectionException("Could not check if " + runMountPoint + + " is a mount point."); + } + + if (!isMount) { + LOGD(runMountPoint << " not mounted - mounting."); - if (!utils::mountTmpfs(runMountPoint)) { - LOGE("Initialization failed: could not mount " << runMountPoint); - throw ContainerConnectionException("Could not mount: " + runMountPoint); + if (!utils::mountTmpfs(runMountPoint)) { + LOGE("Initialization failed: could not mount " << runMountPoint); + throw ContainerConnectionException("Could not mount: " + runMountPoint); + } } // if there is no systemd in the container this dir won't be created automatically @@ -73,9 +81,11 @@ ContainerConnectionTransport::ContainerConnectionTransport(const std::string& ru ContainerConnectionTransport::~ContainerConnectionTransport() { - if (!mRunMountPoint.empty()) { - if (!utils::umount(mRunMountPoint)) { - LOGE("Deinitialization failed: could not umount " << mRunMountPoint); + if (!mDetachOnExit) { + if (!mRunMountPoint.empty()) { + if (!utils::umount(mRunMountPoint)) { + LOGE("Deinitialization failed: could not umount " << mRunMountPoint); + } } } } @@ -96,5 +106,9 @@ std::string ContainerConnectionTransport::acquireAddress() return "unix:path=" + dbusPath; } +void ContainerConnectionTransport::setDetachOnExit() +{ + mDetachOnExit = true; +} } // namespace security_containers diff --git a/server/container-connection-transport.hpp b/server/container-connection-transport.hpp index 989ba96..0be1035 100644 --- a/server/container-connection-transport.hpp +++ b/server/container-connection-transport.hpp @@ -46,8 +46,14 @@ public: */ std::string acquireAddress(); + /** + * Set whether object should detach from transport filesystem on exit + */ + void setDetachOnExit(); + private: std::string mRunMountPoint; + bool mDetachOnExit; }; diff --git a/server/container.cpp b/server/container.cpp index 4717429..a4ad5f1 100644 --- a/server/container.cpp +++ b/server/container.cpp @@ -103,6 +103,12 @@ void Container::goBackground() mAdmin->setSchedulerLevel(SchedulerLevel::BACKGROUND); } +void Container::setDetachOnExit() +{ + mAdmin->setDetachOnExit(); + mConnectionTransport->setDetachOnExit(); +} + bool Container::isRunning() { return mAdmin->isRunning(); diff --git a/server/container.hpp b/server/container.hpp index 5607715..26a94c5 100644 --- a/server/container.hpp +++ b/server/container.hpp @@ -79,6 +79,14 @@ public: void goBackground(); /** + * Set if container should be detached on exit. + * + * This sends detach flag to ContainerAdmin object and disables unmounting tmpfs + * in ContainerConnectionTransport. + */ + void setDetachOnExit(); + + /** * @return Is the container running? */ bool isRunning(); diff --git a/server/containers-manager.cpp b/server/containers-manager.cpp index 288bdef..0aaebc7 100644 --- a/server/containers-manager.cpp +++ b/server/containers-manager.cpp @@ -36,7 +36,7 @@ namespace security_containers { -ContainersManager::ContainersManager(const std::string& managerConfigPath) +ContainersManager::ContainersManager(const std::string& managerConfigPath): mDetachOnExit(false) { LOGD("Instantiating ContainersManager object..."); mConfig.parseFile(managerConfigPath); @@ -64,11 +64,15 @@ ContainersManager::ContainersManager(const std::string& managerConfigPath) ContainersManager::~ContainersManager() { LOGD("Destroying ContainersManager object..."); - try { - stopAll(); - } catch (ServerException&) { - LOGE("Failed to stop all of the containers"); + + if (!mDetachOnExit) { + try { + stopAll(); + } catch (ServerException&) { + LOGE("Failed to stop all of the containers"); + } } + LOGD("ContainersManager object destroyed"); } @@ -139,4 +143,13 @@ std::string ContainersManager::getRunningForegroundContainerId() } +void ContainersManager::setContainersDetachOnExit() +{ + mDetachOnExit = true; + + for (auto& container : mContainers) { + container.second->setDetachOnExit(); + } +} + } // namespace security_containers diff --git a/server/containers-manager.hpp b/server/containers-manager.hpp index e3a428c..920d5fd 100644 --- a/server/containers-manager.hpp +++ b/server/containers-manager.hpp @@ -66,10 +66,16 @@ public: */ std::string getRunningForegroundContainerId(); + /** + * Set whether ContainersManager should detach containers on exit + */ + void setContainersDetachOnExit(); + private: ContainersManagerConfig mConfig; typedef std::unordered_map> ContainerMap; ContainerMap mContainers; // map of containers, id is the key + bool mDetachOnExit; }; diff --git a/server/main.cpp b/server/main.cpp index d911071..395d936 100644 --- a/server/main.cpp +++ b/server/main.cpp @@ -114,6 +114,7 @@ int main(int argc, char* argv[]) try { Server server(configPath); server.run(); + server.reloadIfRequired(argv); } catch (std::exception& e) { LOGE("Unexpected: " << utils::getTypeName(e) << ": " << e.what()); diff --git a/server/server.cpp b/server/server.cpp index 01b0ab3..82d0a6d 100644 --- a/server/server.cpp +++ b/server/server.cpp @@ -30,7 +30,13 @@ #include "utils/glib-loop.hpp" #include +#include #include +#include +#include +#include + +extern char** environ; namespace security_containers { @@ -47,18 +53,29 @@ Server::~Server() namespace { -utils::Latch signalLatch; + +std::atomic_bool gUpdateTriggered(false); +utils::Latch gSignalLatch; + void signalHandler(const int sig) { LOGI("Got signal " << sig); - signalLatch.set(); -} + + if (sig == SIGUSR1) { + LOGD("Received SIGUSR1 - triggering update."); + gUpdateTriggered = true; + } + + gSignalLatch.set(); } +} // namespace + void Server::run() { signal(SIGINT, signalHandler); signal(SIGTERM, signalHandler); + signal(SIGUSR1, signalHandler); LOGI("Starting daemon..."); { @@ -68,7 +85,12 @@ void Server::run() manager.startAll(); LOGI("Daemon started"); - signalLatch.wait(); + gSignalLatch.wait(); + + // Detach containers if we triggered an update + if (gUpdateTriggered) { + manager.setContainersDetachOnExit(); + } LOGI("Stopping daemon..."); // manager.stopAll() will be called in destructor @@ -76,10 +98,19 @@ void Server::run() LOGI("Daemon stopped"); } +void Server::reloadIfRequired(char* argv[]) +{ + if (gUpdateTriggered) { + execve(argv[0], argv, environ); + + LOGE("Failed to reload " << argv[0] << ": " << strerror(errno)); + } +} + void Server::terminate() { LOGI("Terminating server"); - signalLatch.set(); + gSignalLatch.set(); } } // namespace security_containers diff --git a/server/server.hpp b/server/server.hpp index 59a1a35..abb8775 100644 --- a/server/server.hpp +++ b/server/server.hpp @@ -38,11 +38,16 @@ public: virtual ~Server(); /** - * Starts all the containers and blocks until SIGINT or SIGTERM + * Starts all the containers and blocks until SIGINT, SIGTERM or SIGUSR1 */ void run(); /** + * Reload the server by launching execve on itself if SIGUSR1 was sent to server. + */ + void reloadIfRequired(char* argv[]); + + /** * Terminates the server. * Equivalent of sending SIGINT or SIGTERM signal */ -- 2.7.4 From 8a7990d12c58f0680025cbb5eaf62343ad9baedd Mon Sep 17 00:00:00 2001 From: Piotr Bartosiewicz Date: Thu, 15 May 2014 13:24:26 +0200 Subject: [PATCH 15/16] Mount smackfs into containers [Bug/Feature] There were no possibilities to check SMACK labels being inside the container [Cause] Smackfs was not mounted [Solution] N/A [Verification] Build, install, check the result of command: ls -1Z # (inside container) Should output labels, not '?' Change-Id: I8aaf961b05e87725df85b6031efb60c45142b977 --- server/configs/libvirt-config/business.xml | 7 +++++++ server/configs/libvirt-config/private.xml | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/server/configs/libvirt-config/business.xml b/server/configs/libvirt-config/business.xml index f2317a8..8f044c0 100644 --- a/server/configs/libvirt-config/business.xml +++ b/server/configs/libvirt-config/business.xml @@ -109,5 +109,12 @@ + + + + + + diff --git a/server/configs/libvirt-config/private.xml b/server/configs/libvirt-config/private.xml index 7df0db4..26cf1f6 100644 --- a/server/configs/libvirt-config/private.xml +++ b/server/configs/libvirt-config/private.xml @@ -109,5 +109,12 @@ + + + + + + -- 2.7.4 From 76585ab1424d20a3bc9f856bdcb73d71d28b90f6 Mon Sep 17 00:00:00 2001 From: Michal Witanowski Date: Tue, 20 May 2014 10:09:14 +0200 Subject: [PATCH 16/16] Fix building without spec [Cause] Undefined macro. [Solution] SYSTEMD_UNIT_DIR defined in CMakeLists.txt. [Verification] Build with CMake. Change-Id: I42ee402003af07b51d3cf07bc5dadf7a5dc442af Signed-off-by: Michal Witanowski --- CMakeLists.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 964ba66..88a1eaf 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -81,6 +81,10 @@ IF(NOT DEFINED SHARE_INSTALL_PREFIX) SET(SHARE_INSTALL_PREFIX "${CMAKE_INSTALL_FULL_DATAROOTDIR}") ENDIF(NOT DEFINED SHARE_INSTALL_PREFIX) +IF(NOT DEFINED SYSTEMD_UNIT_DIR) + SET(SYSTEMD_UNIT_DIR "${LIB_INSTALL_DIR}/systemd/system") +ENDIF(NOT DEFINED SYSTEMD_UNIT_DIR) + SET(SC_CONFIG_INSTALL_DIR ${SYSCONF_INSTALL_DIR}/security-containers) SET(SC_DATA_INSTALL_DIR ${SHARE_INSTALL_PREFIX}/security-containers) -- 2.7.4