From ed2aceab6ee059a40d939ea21364bc18ec80d94b Mon Sep 17 00:00:00 2001 From: Przemyslaw Czarnowski Date: Mon, 14 Mar 2022 22:20:54 +0100 Subject: Make mount/unmount dbus calls asynchronous Change the default behavior of mount/umount dbus calls from blocking to unblocking ones. Once mount/unmount is triggered, appropriate action is running in the background moving handling of operation result to async event. At the end of processing dbus completion signal is sent to client with uint value of operation status (identical with errno code). Tested: Manual scheduling of mount and unmount operations with monitoring dbus communication of virtual-media service - matching api calls with completion signal. Signed-off-by: Przemyslaw Czarnowski --- src/interfaces/mount_point_state_machine.hpp | 9 ++ src/state/activating_state.cpp | 3 +- src/state/active_state.hpp | 15 +-- src/state/deactivating_state.hpp | 5 +- src/state/initial_state.hpp | 146 ++++++--------------------- src/state/ready_state.hpp | 7 +- src/state_machine.hpp | 48 ++++++++- src/utils.hpp | 81 +++++++++++++++ 8 files changed, 186 insertions(+), 128 deletions(-) diff --git a/src/interfaces/mount_point_state_machine.hpp b/src/interfaces/mount_point_state_machine.hpp index db521fb..e6b175c 100644 --- a/src/interfaces/mount_point_state_machine.hpp +++ b/src/interfaces/mount_point_state_machine.hpp @@ -3,6 +3,8 @@ #include "configuration.hpp" #include "resources.hpp" +#include + struct BasicState; namespace interfaces @@ -20,6 +22,13 @@ struct MountPointStateMachine virtual ~MountPointStateMachine() = default; + virtual void notify(const std::error_code& ec = {}) = 0; + virtual void notificationStart() = 0; + virtual void + notificationInitialize(std::shared_ptr con, + const std::string& svc, const std::string& iface, + const std::string& name) = 0; + virtual std::string_view getName() const = 0; virtual Configuration::MountPoint& getConfig() = 0; virtual std::optional& getTarget() = 0; diff --git a/src/state/activating_state.cpp b/src/state/activating_state.cpp index c22174f..7143545 100644 --- a/src/state/activating_state.cpp +++ b/src/state/activating_state.cpp @@ -54,7 +54,8 @@ std::unique_ptr ActivatingState::handleEvent([ [maybe_unused]] SubprocessStoppedEvent event) { LogMsg(Logger::Error, "Process ended prematurely"); - return std::make_unique(machine); + return std::make_unique(machine, std::errc::connection_refused, + "Process ended prematurely"); } std::unique_ptr ActivatingState::activateProxyMode() diff --git a/src/state/active_state.hpp b/src/state/active_state.hpp index a204dc3..beb1da1 100644 --- a/src/state/active_state.hpp +++ b/src/state/active_state.hpp @@ -14,7 +14,10 @@ struct ActiveState : public BasicStateT std::unique_ptr process, std::unique_ptr gadget) : BasicStateT(machine), - process(std::move(process)), gadget(std::move(gadget)){}; + process(std::move(process)), gadget(std::move(gadget)) + { + machine.notify(); + }; virtual std::unique_ptr onEnter() { @@ -43,12 +46,9 @@ struct ActiveState : public BasicStateT Configuration::inactivityTimeout.count(), "s) - Unmounting"); // unmount media & stop retriggering timer - boost::asio::spawn( - machine.getIoc(), - [&machine = machine]( - [[maybe_unused]] boost::asio::yield_context yield) { - machine.emitUnmountEvent(); - }); + boost::asio::post(machine.getIoc(), [&machine = machine]() { + machine.emitUnmountEvent(); + }); return; } else @@ -80,6 +80,7 @@ struct ActiveState : public BasicStateT std::unique_ptr handleEvent([[maybe_unused]] UnmountEvent event) { + machine.notificationStart(); return std::make_unique(machine, std::move(process), std::move(gadget)); } diff --git a/src/state/deactivating_state.hpp b/src/state/deactivating_state.hpp index 57f1072..472b6f1 100644 --- a/src/state/deactivating_state.hpp +++ b/src/state/deactivating_state.hpp @@ -64,14 +64,17 @@ struct DeactivatingState : public BasicStateT { LogMsg(Logger::Info, machine.getName(), " udev StateChange::removed"); + return std::make_unique(machine); } else { LogMsg(Logger::Error, machine.getName(), " udev StateChange::", static_cast>( udevStateChangeEvent->devState)); + return std::make_unique( + machine, std::errc::connection_refused, + "Not expected udev state"); } - return std::make_unique(machine); } return nullptr; } diff --git a/src/state/initial_state.hpp b/src/state/initial_state.hpp index 324f456..f9255b0 100644 --- a/src/state/initial_state.hpp +++ b/src/state/initial_state.hpp @@ -1,7 +1,13 @@ #include "active_state.hpp" #include "basic_state.hpp" +#include "logger.hpp" #include "ready_state.hpp" +#include + +#include +#include +#include #include struct InitialState : public BasicStateT @@ -200,85 +206,18 @@ struct InitialState : public BasicStateT auto iface = event.objServer->add_interface(path, name); - const auto timerPeriod = std::chrono::milliseconds(100); - const auto duration = std::chrono::seconds( - machine.getConfig().timeout.value_or( - Configuration::MountPoint::defaultTimeout) + - 5); - const auto waitCnt = - std::chrono::duration_cast(duration) / - timerPeriod; - LogMsg(Logger::Debug, "[App] waitCnt == ", waitCnt); + iface->register_signal("Completion"); + machine.notificationInitialize(event.bus, path, name, "Completion"); // Common unmount - iface->register_method( - "Unmount", [&machine = machine, waitCnt, - timerPeriod](boost::asio::yield_context yield) { - LogMsg(Logger::Info, "[App]: Unmount called on ", - machine.getName()); - machine.emitUnmountEvent(); - - auto repeats = waitCnt; - boost::asio::steady_timer timer(machine.getIoc()); - while (repeats > 0) - { - if (machine.getState().get_if()) - { - LogMsg(Logger::Debug, "[App] Unmount ok"); - return true; - } - boost::system::error_code ignored_ec; - timer.expires_from_now(timerPeriod); - timer.async_wait(yield[ignored_ec]); - repeats--; - } - LogMsg(Logger::Error, - "[App] timedout when waiting for ReadyState"); - throw sdbusplus::exception::SdBusError(EBUSY, - "Resource is busy"); - return false; - }); + iface->register_method("Unmount", [&machine = machine]() { + LogMsg(Logger::Info, "[App]: Unmount called on ", + machine.getName()); - // Common mount - const auto handleMount = - [waitCnt, timerPeriod]( - boost::asio::yield_context yield, - interfaces::MountPointStateMachine& machine, - std::optional - target) { - machine.emitMountEvent(std::move(target)); - - auto repeats = waitCnt; - boost::asio::steady_timer timer(machine.getIoc()); - while (repeats > 0) - { - if (auto s = machine.getState().get_if()) - { - if (s->error) - { - throw sdbusplus::exception::SdBusError( - static_cast(s->error->code), - s->error->message.c_str()); - } - LogMsg(Logger::Error, "[App] Mount failed"); - return false; - } - if (machine.getState().get_if()) - { - LogMsg(Logger::Info, "[App] Mount ok"); - return true; - } - boost::system::error_code ignored_ec; - timer.expires_from_now(timerPeriod); - timer.async_wait(yield[ignored_ec]); - repeats--; - } - LogMsg(Logger::Error, - "[App] timedout when waiting for ActiveState"); - throw sdbusplus::exception::SdBusError(EBUSY, - "Resource is busy"); - return false; - }; + machine.emitUnmountEvent(); + + return true; + }); // Mount specialization if (isLegacy) @@ -287,9 +226,9 @@ struct InitialState : public BasicStateT using optional_fd = std::variant; iface->register_method( - "Mount", [&machine = machine, handleMount]( - boost::asio::yield_context yield, - std::string imgUrl, bool rw, optional_fd fd) { + "Mount", [&machine = machine](boost::asio::yield_context yield, + std::string imgUrl, bool rw, + optional_fd fd) { LogMsg(Logger::Info, "[App]: Mount called on ", getObjectPath(machine), machine.getName()); @@ -333,48 +272,21 @@ struct InitialState : public BasicStateT utils::secureCleanup(buf); } - try - { - auto ret = - handleMount(yield, machine, std::move(target)); - if (machine.getTarget()) - { - machine.getTarget()->credentials.reset(); - } - LogMsg(Logger::Info, "[App]: mount completed ", ret); - return ret; - } - catch (const std::exception& e) - { - LogMsg(Logger::Error, e.what()); - if (machine.getTarget()) - { - machine.getTarget()->credentials.reset(); - } - throw; - return false; - } - catch (...) - { - if (machine.getTarget()) - { - machine.getTarget()->credentials.reset(); - } - throw; - return false; - } + machine.emitMountEvent(std::move(target)); + + return true; }); } - else + else // proxy { - iface->register_method( - "Mount", [&machine = machine, - handleMount](boost::asio::yield_context yield) { - LogMsg(Logger::Info, "[App]: Mount called on ", - getObjectPath(machine), machine.getName()); + iface->register_method("Mount", [&machine = machine]() mutable { + LogMsg(Logger::Info, "[App]: Mount called on ", + getObjectPath(machine), machine.getName()); - return handleMount(yield, machine, std::nullopt); - }); + machine.emitMountEvent(std::nullopt); + + return true; + }); } iface->initialize(); diff --git a/src/state/ready_state.hpp b/src/state/ready_state.hpp index bf1e160..e20e700 100644 --- a/src/state/ready_state.hpp +++ b/src/state/ready_state.hpp @@ -24,7 +24,10 @@ struct ReadyState : public BasicStateT }; ReadyState(interfaces::MountPointStateMachine& machine) : - BasicStateT(machine){}; + BasicStateT(machine) + { + machine.notify(); + }; ReadyState(interfaces::MountPointStateMachine& machine, const std::errc& ec, const std::string& message) : @@ -33,6 +36,7 @@ struct ReadyState : public BasicStateT { LogMsg(Logger::Error, machine.getName(), " Errno = ", static_cast(ec), " : ", message); + machine.notify(std::make_error_code(ec)); } std::unique_ptr onEnter() override @@ -47,6 +51,7 @@ struct ReadyState : public BasicStateT std::unique_ptr handleEvent(MountEvent event) { + machine.notificationStart(); if (event.target) { machine.getTarget() = std::move(event.target); diff --git a/src/state_machine.hpp b/src/state_machine.hpp index 259802c..d5f3265 100644 --- a/src/state_machine.hpp +++ b/src/state_machine.hpp @@ -1,10 +1,13 @@ #pragma once - #include "interfaces/mount_point_state_machine.hpp" #include "state/initial_state.hpp" +#include "utils.hpp" +#include +#include #include #include +#include struct MountPointStateMachine : public interfaces::MountPointStateMachine { @@ -106,9 +109,52 @@ struct MountPointStateMachine : public interfaces::MountPointStateMachine } } + virtual void + notificationInitialize(std::shared_ptr con, + const std::string& svc, const std::string& iface, + const std::string& name) override + { + auto signal = std::make_unique(std::move(con), svc, + iface, name); + + auto timer = std::make_unique(ioc); + + completionNotification = std::make_unique( + std::move(signal), std::move(timer)); + } + void notificationStart() + { + auto notificationHandler = [this](const boost::system::error_code& ec) { + if (ec == boost::system::errc::operation_canceled) + { + return; + } + + LogMsg(Logger::Error, + "[App] timedout when waiting for target state"); + + completionNotification->notify( + std::make_error_code(std::errc::device_or_resource_busy)); + }; + + LogMsg(Logger::Debug, "Started notification"); + completionNotification->start( + std::move(notificationHandler), + std::chrono::seconds( + config.timeout.value_or( + Configuration::MountPoint::defaultTimeout) + + 5)); + } + + virtual void notify(const std::error_code& ec = {}) override + { + completionNotification->notify(ec); + } + boost::asio::io_context& ioc; std::string name; Configuration::MountPoint config; + std::unique_ptr completionNotification; std::optional target; std::unique_ptr state = std::make_unique(*this); diff --git a/src/utils.hpp b/src/utils.hpp index 587cc41..db9f325 100644 --- a/src/utils.hpp +++ b/src/utils.hpp @@ -1,14 +1,19 @@ #pragma once +#include "logger.hpp" + #include #include +#include #include #include #include #include +#include #include #include +#include #include namespace fs = std::filesystem; @@ -281,4 +286,80 @@ class VolatileFile std::string filePath; const std::size_t size; }; + +class SignalSender +{ + public: + SignalSender(std::shared_ptr con, + const std::string& obj, const std::string& iface, + const std::string& name) : + con(con), + interface(iface), object(obj), name(name){}; + + SignalSender() = delete; + SignalSender(const SignalSender&) = delete; + + void send(std::optional status) + { + auto msgSignal = + con->new_signal(object.c_str(), interface.c_str(), name.c_str()); + + msgSignal.append(status.value_or(std::error_code{}).value()); + LogMsg(Logger::Debug, "Sending signal: Object: ", object, + ", Interface: ", interface, ", Name: ", name, + "Status: ", status.value_or(std::error_code{}).value()); + msgSignal.signal_send(); + } + + private: + std::shared_ptr con; + std::string interface; + std::string object; + std::string name; +}; + +class NotificationWrapper +{ + public: + NotificationWrapper(std::unique_ptr signal, + std::unique_ptr timer) : + signal(std::move(signal)), + timer(std::move(timer)) + { + } + + void start(std::function&& handler, + const std::chrono::seconds& duration) + { + LogMsg(Logger::Debug, "Notification initiated"); + started = true; + timer->expires_from_now(duration); + timer->async_wait([this, handler{std::move(handler)}]( + const boost::system::error_code& ec) { + started = false; + handler(ec); + }); + } + + void notify(const std::error_code& ec) + { + if (started) + { + timer->cancel(); + if (ec) + signal->send((ec)); + else + signal->send(std::nullopt); + started = false; + return; + } + LogMsg(Logger::Debug, "Notification(ec) supressed (not started)"); + } + + private: + std::unique_ptr signal; + std::unique_ptr timer; + bool started{false}; +}; + } // namespace utils -- cgit v1.2.3