From a8b6b77d79bac80543033cc3bd140ff6cf4ba2f5 Mon Sep 17 00:00:00 2001 From: Agata Olender Date: Mon, 13 Jan 2020 17:48:24 +0100 Subject: Detailed error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously implemented error handling for Mount checks only if mounting was successful and returns boolean with the result. Following change introduces optional error to existing ReadyState (a.k.a. "idle state"). If state machine enters ReadyState with error it is stored into ReadyState field with std::errc and std::string message. In the case of mount failure with such error information stored, Mount returns graceful D-Bus error reply with specific error code. Tested: Manual and automated tests on WilsonCity platform: - negative tests for invalid network share, unauthorized share access, error injection (renaming expected unix socket names etc) Change-Id: I22cf9b17e9e6342aad0ae68766853734fac79b8e Signed-off-by: Adrian Ambrożewicz Signed-off-by: Agata Olender --- virtual-media/src/state_machine.hpp | 68 +++++++++++++++++++++++++++---------- 1 file changed, 50 insertions(+), 18 deletions(-) diff --git a/virtual-media/src/state_machine.hpp b/virtual-media/src/state_machine.hpp index 9e011b9..9cba565 100644 --- a/virtual-media/src/state_machine.hpp +++ b/virtual-media/src/state_machine.hpp @@ -13,6 +13,7 @@ #include #include #include +#include #include struct MountPointStateMachine @@ -24,6 +25,12 @@ struct MountPointStateMachine } }; + struct Error + { + std::errc code; + std::string message; + }; + struct BasicState { BasicState(MountPointStateMachine& machine, @@ -75,6 +82,15 @@ struct MountPointStateMachine { ReadyState(const BasicState& state) : BasicState(state, __FUNCTION__){}; + ReadyState(const BasicState& state, const std::errc& ec, + const std::string& message) : + BasicState(state, __FUNCTION__), + error{{ec, message}} + { + LogMsg(Logger::Error, state.machine.name, + " Errno = ", static_cast(ec), " : ", message); + }; + virtual void onEnter() { if (machine.target) @@ -90,6 +106,8 @@ struct MountPointStateMachine machine.config.remainingInactivityTimeout = std::chrono::seconds(0); } + + std::optional error; }; struct ActivatingState : public BasicState @@ -412,8 +430,14 @@ struct MountPointStateMachine int waitCnt = 120; while (waitCnt > 0) { - if (std::get_if(&machine.state)) + if (auto s = std::get_if(&machine.state)) { + if (s->error) + { + throw sdbusplus::exception::SdBusError( + static_cast(s->error->code), + s->error->message.c_str()); + } return false; } if (std::get_if(&machine.state)) @@ -497,7 +521,8 @@ struct MountPointStateMachine { if (!state.machine.removeUsbGadget(state)) { - return ReadyState(state); + return ReadyState(state, std::errc::device_or_resource_busy, + "Unable to unmount gadget"); } state.machine.stopProcess(state.process); return WaitingForProcessEndState(state); @@ -524,13 +549,15 @@ struct MountPointStateMachine State operator()(const WaitingForGadgetState& state) { state.machine.stopProcess(state.process); - return ReadyState(state); + return ReadyState(state, std::errc::io_error, + "Process ended prematurely"); } State operator()(const ActiveState& state) { if (!state.machine.removeUsbGadget(state)) { - return ReadyState(state); + return ReadyState(state, std::errc::device_or_resource_busy, + "Unable to unmount gadget"); } return ReadyState(state); } @@ -561,9 +588,8 @@ struct MountPointStateMachine "/usr/sbin/nbd-client", state.machine.config.nbdDevice); if (!process) { - LogMsg(Logger::Error, state.machine.name, - " Failed to create Process for: ", state.machine.name); - return ReadyState(state); + return ReadyState(state, std::errc::operation_canceled, + "Failed to allocate process"); } if (!process->spawn( Configuration::MountPoint::toArgs(state.machine.config), @@ -572,9 +598,8 @@ struct MountPointStateMachine machine.emitSubprocessStoppedEvent(); })) { - LogMsg(Logger::Error, state.machine.name, - " Failed to spawn Process for: ", state.machine.name); - return ReadyState(state); + return ReadyState(state, std::errc::operation_canceled, + "Failed to spawn process"); } auto newState = WaitingForGadgetState(state); newState.process = process; @@ -597,8 +622,8 @@ struct MountPointStateMachine return mountHttpsShare(state); } - LogMsg(Logger::Error, state.machine.name, " URL not recognized"); - return ReadyState(state); + return ReadyState(state, std::errc::invalid_argument, + "URL not recognized"); } State mountSmbShare(const ActivatingState& state) @@ -606,7 +631,8 @@ struct MountPointStateMachine auto mountDir = SmbShare::createMountDir(state.machine.name); if (!mountDir) { - return ReadyState(state); + return ReadyState(state, std::errc::io_error, + "Failed to create mount directory"); } SmbShare smb(*mountDir); @@ -621,14 +647,16 @@ struct MountPointStateMachine if (!smb.mount(remoteParent, state.machine.target->rw)) { fs::remove_all(*mountDir); - return ReadyState(state); + return ReadyState(state, std::errc::invalid_argument, + "Failed to mount CIFS share"); } auto process = spawnNbdKit(state.machine, localFile); if (!process) { SmbShare::unmount(*mountDir); - return ReadyState(state); + return ReadyState(state, std::errc::operation_canceled, + "Unable to setup NbdKit"); } auto newState = WaitingForGadgetState(state); @@ -645,7 +673,8 @@ struct MountPointStateMachine auto process = spawnNbdKit(machine, machine.target->imgUrl); if (!process) { - return ReadyState(state); + return ReadyState(state, std::errc::invalid_argument, + "Failed to mount HTTPS share"); } auto newState = WaitingForGadgetState(state); @@ -837,9 +866,12 @@ struct MountPointStateMachine { return ActiveState(state); } - return ReadyState(state); + return ReadyState(state, std::errc::device_or_resource_busy, + "Unable to configure gadget"); } - return ReadyState(state); + return ReadyState(state, std::errc::operation_not_supported, + "Unexpected udev event: " + + static_cast(devState)); } State operator()(const ReadyState& state) -- cgit v1.2.3