From 3bfa3b29c0515a9e77c7c69fe072b7ff2e0fc302 Mon Sep 17 00:00:00 2001 From: Ed Tanous Date: Wed, 31 Jan 2024 12:18:03 -0800 Subject: Move to process v2 Boost process v2 brings some significant benefits to our launching of processes[1]. In bmcweb terms: 1. The code is radically simpler, which decreaeses compile times, and reduces the scope for code scanning tools. 2. The code now uses standard asio pipes instead of inventing its own. 3. Separate compilation. Tested: We don't have a lot of unit tests for the virtual media stuff that I can run, but we do have unit tests for credentials pipe, which in this change have been ported over, so the feature works. Unit tests are passing. [1] https://www.boost.org/doc/libs/1_80_0/doc/html/boost_process/v2.html#boost_process.v2.introduction Change-Id: Ia20226819d75ff6e492f8852185f0b73e8f5cf83 Signed-off-by: Ed Tanous --- include/credential_pipe.hpp | 23 ++++++++++++++++++----- include/vm_websocket.hpp | 24 ++++++++++++------------ redfish-core/lib/virtual_media.hpp | 3 +-- test/include/credential_pipe_test.cpp | 18 +++++++++--------- 4 files changed, 40 insertions(+), 28 deletions(-) diff --git a/include/credential_pipe.hpp b/include/credential_pipe.hpp index 169d47c6cb..2cc3dc8b28 100644 --- a/include/credential_pipe.hpp +++ b/include/credential_pipe.hpp @@ -1,9 +1,13 @@ #pragma once +#include "logging.hpp" + #include +#include #include +#include +#include #include -#include #include #include @@ -12,7 +16,15 @@ class CredentialsPipe { public: - explicit CredentialsPipe(boost::asio::io_context& io) : impl(io) {} + explicit CredentialsPipe(boost::asio::io_context& io) : impl(io), read(io) + { + boost::system::error_code ec; + boost::asio::connect_pipe(read, impl, ec); + if (ec) + { + BMCWEB_LOG_CRITICAL("Failed to connect pipe {}", ec.what()); + } + } CredentialsPipe(const CredentialsPipe&) = delete; CredentialsPipe(CredentialsPipe&&) = delete; @@ -25,9 +37,9 @@ class CredentialsPipe explicit_bzero(pass.data(), pass.capacity()); } - int fd() const + int releaseFd() { - return impl.native_source(); + return read.release(); } template @@ -44,7 +56,8 @@ class CredentialsPipe std::forward(handler)); } - boost::process::async_pipe impl; + boost::asio::writable_pipe impl; + boost::asio::readable_pipe read; private: std::string user; diff --git a/include/vm_websocket.hpp b/include/vm_websocket.hpp index 19054a6d97..3a72b3adb5 100644 --- a/include/vm_websocket.hpp +++ b/include/vm_websocket.hpp @@ -3,10 +3,11 @@ #include "app.hpp" #include "websocket.hpp" +#include +#include #include -#include -#include -#include +#include +#include #include @@ -26,8 +27,11 @@ static constexpr auto nbdBufferSize = (128 * 1024 + 16) * 4; class Handler : public std::enable_shared_from_this { public: - Handler(const std::string& mediaIn, boost::asio::io_context& ios) : - pipeOut(ios), pipeIn(ios), media(mediaIn), + Handler(const std::string& media, boost::asio::io_context& ios) : + pipeOut(ios), pipeIn(ios), + proxy(ios, "/usr/bin/nbd-proxy", {media}, + boost::process::v2::process_stdio{ + .in = pipeIn, .out = pipeOut, .err = nullptr}), outputBuffer(new boost::beast::flat_static_buffer), inputBuffer(new boost::beast::flat_static_buffer) {} @@ -56,9 +60,6 @@ class Handler : public std::enable_shared_from_this void connect() { std::error_code ec; - proxy = boost::process::child("/usr/bin/nbd-proxy", media, - boost::process::std_out > pipeOut, - boost::process::std_in < pipeIn, ec); if (ec) { BMCWEB_LOG_ERROR("Couldn't connect to nbd-proxy: {}", ec.message()); @@ -148,10 +149,9 @@ class Handler : public std::enable_shared_from_this }); } - boost::process::async_pipe pipeOut; - boost::process::async_pipe pipeIn; - boost::process::child proxy; - std::string media; + boost::asio::readable_pipe pipeOut; + boost::asio::writable_pipe pipeIn; + boost::process::v2::process proxy; bool doingWrite{false}; std::unique_ptr> diff --git a/redfish-core/lib/virtual_media.hpp b/redfish-core/lib/virtual_media.hpp index 529b9dc5d9..37fb2045c9 100644 --- a/redfish-core/lib/virtual_media.hpp +++ b/redfish-core/lib/virtual_media.hpp @@ -25,7 +25,6 @@ #include "registries/privilege_registry.hpp" #include "utils/json_utils.hpp" -#include #include #include #include @@ -470,7 +469,7 @@ inline void doMountVmLegacy(const std::shared_ptr& asyncResp, // Open pipe secretPipe = std::make_shared( crow::connections::systemBus->get_io_context()); - fd = secretPipe->fd(); + fd = secretPipe->releaseFd(); // Pass secret over pipe secretPipe->asyncWrite( diff --git a/test/include/credential_pipe_test.cpp b/test/include/credential_pipe_test.cpp index 0c722222da..3a750b4669 100644 --- a/test/include/credential_pipe_test.cpp +++ b/test/include/credential_pipe_test.cpp @@ -3,8 +3,9 @@ #include #include -#include -#include +#include +#include +#include #include #include @@ -26,21 +27,20 @@ static void handler(boost::asio::io_context& io, TEST(CredentialsPipe, BasicSend) { - boost::beast::file_posix file; + boost::asio::io_context io; + boost::asio::readable_pipe testPipe(io); { - boost::asio::io_context io; CredentialsPipe pipe(io); - file.native_handle(dup(pipe.fd())); - ASSERT_GT(file.native_handle(), 0); + testPipe = boost::asio::readable_pipe(io, pipe.releaseFd()); + ASSERT_GT(testPipe.native_handle(), 0); pipe.asyncWrite("username", "password", std::bind_front(handler, std::ref(io))); - io.run(); } + io.run(); std::array buff{}; boost::system::error_code ec; - size_t r = file.read(buff.data(), buff.size(), ec); + boost::asio::read(testPipe, boost::asio::buffer(buff), ec); ASSERT_FALSE(ec); - ASSERT_EQ(r, 18); EXPECT_THAT(buff, ElementsAre('u', 's', 'e', 'r', 'n', 'a', 'm', 'e', '\0', 'p', -- cgit v1.2.3