From 98be3e394a78f8866e731019b1828419dc66561a Mon Sep 17 00:00:00 2001 From: Ed Tanous Date: Thu, 16 Sep 2021 15:05:36 -0700 Subject: Remove copies from log services and virtual media Both of these entries make complete copies of the Request object. Following the pattern in the prior commit, move these to more modern patterns. For log service, this simply means constructing a payload object earlier. For virtual media, it means doing the readJson call and parameter validation much earlier, which generally is the pattern we should strive for, validating and unpacking the structs in the first scope, then dealing with them as structures. To do this, this commit also creates a secondary struct to store the param data in to make the lambdas cleaner. Tested: From Ashmitha POST https://${bmc}/redfish/v1/Managers/bmc/LogServices/Dump/Actions/LogService.CollectDiagnosticData -d '{"DiagnosticDataType":"Manager"}' { "@odata.id": "/redfish/v1/TaskService/Tasks/0", "@odata.type": "#Task.v1_4_3.Task", "Id": "0", "TaskState": "Running", "TaskStatus": "OK" } ----------------------------------------------------------------- On task completion: GET https://${bmc}/redfish/v1/TaskService/Tasks/0 { "@odata.id": "/redfish/v1/TaskService/Tasks/0", "@odata.type": "#Task.v1_4_3.Task", "EndTime": "2021-12-03T13:40:58+00:00", "Id": "0", "Messages": [ { "@odata.type": "#Message.v1_0_0.Message", "Message": "The task with id 0 has started.", "MessageArgs": [ "0" ], "MessageId": "TaskEvent.1.0.1.TaskStarted", "Resolution": "None.", "Severity": "OK" }, { "@odata.type": "#Message.v1_1_1.Message", "Message": "Successfully Completed Request", "MessageArgs": [], "MessageId": "Base.1.8.1.Success", "MessageSeverity": "OK", "Resolution": "None" } ], "Name": "Task 0", "Payload": { "HttpHeaders": [ "Host: 9.3.29.238", "User-Agent: curl/7.71.1", "Accept: */*", "Content-Length: 32", "Location: /redfish/v1/Managers/bmc/LogServices/Dump/Entries/32" ], "HttpOperation": "POST", "JsonBody": "{\n \"DiagnosticDataType\": \"Manager\"\n}", "TargetUri": "/redfish/v1/Managers/bmc/LogServices/Dump/Actions/LogService.CollectDiagnosticData" }, "PercentComplete": 0, "StartTime": "2021-12-03T13:38:20+00:00", "TaskMonitor": "/redfish/v1/TaskService/Tasks/0/Monitor", "TaskState": "Completed", "TaskStatus": "OK" } Signed-off-by: Ed Tanous Change-Id: I178a3a7a7b27dfd34ec50a06398ac243a9d4ab67 --- redfish-core/lib/log_services.hpp | 19 ++++--- redfish-core/lib/virtual_media.hpp | 105 ++++++++++++++++++------------------- 2 files changed, 62 insertions(+), 62 deletions(-) diff --git a/redfish-core/lib/log_services.hpp b/redfish-core/lib/log_services.hpp index f71f81b66d..cdd746f4d9 100644 --- a/redfish-core/lib/log_services.hpp +++ b/redfish-core/lib/log_services.hpp @@ -723,7 +723,7 @@ inline void deleteDumpEntry(const std::shared_ptr& asyncResp, } inline void - createDumpTaskCallback(const crow::Request& req, + createDumpTaskCallback(task::Payload&& payload, const std::shared_ptr& asyncResp, const uint32_t& dumpId, const std::string& dumpPath, const std::string& dumpType) @@ -772,7 +772,7 @@ inline void task->startTimer(std::chrono::minutes(3)); task->populateResp(asyncResp->res); - task->payload.emplace(req); + task->payload.emplace(std::move(payload)); } inline void createDump(const std::shared_ptr& asyncResp, @@ -847,8 +847,9 @@ inline void createDump(const std::shared_ptr& asyncResp, } crow::connections::systemBus->async_method_call( - [asyncResp, req, dumpPath, dumpType](const boost::system::error_code ec, - const uint32_t& dumpId) { + [asyncResp, payload(task::Payload(req)), dumpPath, + dumpType](const boost::system::error_code ec, + const uint32_t& dumpId) mutable { if (ec) { BMCWEB_LOG_ERROR << "CreateDump resp_handler got error " << ec; @@ -857,7 +858,8 @@ inline void createDump(const std::shared_ptr& asyncResp, } BMCWEB_LOG_DEBUG << "Dump Created. Id: " << dumpId; - createDumpTaskCallback(req, asyncResp, dumpId, dumpPath, dumpType); + createDumpTaskCallback(std::move(payload), asyncResp, dumpId, + dumpPath, dumpType); }, "xyz.openbmc_project.Dump.Manager", "/xyz/openbmc_project/dump/" + @@ -2907,10 +2909,11 @@ inline void requestRoutesCrashdumpCollect(App& app) return; } - auto collectCrashdumpCallback = [asyncResp, req]( + auto collectCrashdumpCallback = [asyncResp, + payload(task::Payload(req))]( const boost::system::error_code ec, - const std::string&) { + const std::string&) mutable { if (ec) { if (ec.value() == @@ -2948,7 +2951,7 @@ inline void requestRoutesCrashdumpCollect(App& app) "member='PropertiesChanged',arg0namespace='com.intel.crashdump'"); task->startTimer(std::chrono::minutes(5)); task->populateResp(asyncResp->res); - task->payload.emplace(req); + task->payload.emplace(std::move(payload)); }; if (oemDiagnosticDataType == "OnDemand") diff --git a/redfish-core/lib/virtual_media.hpp b/redfish-core/lib/virtual_media.hpp index 64f9a4cb42..34c1c4b06d 100644 --- a/redfish-core/lib/virtual_media.hpp +++ b/redfish-core/lib/virtual_media.hpp @@ -775,6 +775,17 @@ inline void doVmAction(const std::shared_ptr& asyncResp, } } +struct InsertMediaActionParams +{ + std::string imageUrl; + std::optional userName; + std::optional password; + std::optional transferMethod; + std::optional transferProtocolType; + std::optional writeProtected = true; + std::optional inserted; +}; + inline void requestNBDVirtualMediaRoutes(App& app) { BMCWEB_ROUTE( @@ -792,11 +803,37 @@ inline void requestNBDVirtualMediaRoutes(App& app) return; } + InsertMediaActionParams actionParams; + + // Read obligatory parameters (url of + // image) + if (!json_util::readJson( + req, asyncResp->res, "Image", actionParams.imageUrl, + "WriteProtected", actionParams.writeProtected, + "UserName", actionParams.userName, "Password", + actionParams.password, "Inserted", + actionParams.inserted, "TransferMethod", + actionParams.transferMethod, "TransferProtocolType", + actionParams.transferProtocolType)) + { + BMCWEB_LOG_DEBUG << "Image is not provided"; + return; + } + + bool paramsValid = validateParams( + asyncResp->res, actionParams.imageUrl, + actionParams.inserted, actionParams.transferMethod, + actionParams.transferProtocolType); + + if (paramsValid == false) + { + return; + } crow::connections::systemBus->async_method_call( - [asyncResp, req, + [asyncResp, actionParams, resName](const boost::system::error_code ec, - const GetObjectType& getObjectType) { + const GetObjectType& getObjectType) mutable { if (ec) { BMCWEB_LOG_ERROR @@ -810,9 +847,9 @@ inline void requestNBDVirtualMediaRoutes(App& app) BMCWEB_LOG_DEBUG << "GetObjectType: " << service; crow::connections::systemBus->async_method_call( - [service, resName, req, + [service, resName, actionParams, asyncResp](const boost::system::error_code ec, - ManagedObjectType& subtree) { + ManagedObjectType& subtree) mutable { if (ec) { BMCWEB_LOG_DEBUG << "DBUS response error"; @@ -857,53 +894,14 @@ inline void requestNBDVirtualMediaRoutes(App& app) continue; } - // Legacy mode - std::string imageUrl; - std::optional userName; - std::optional password; - std::optional - transferMethod; - std::optional - transferProtocolType; - std::optional writeProtected = - true; - std::optional inserted; - - // Read obligatory parameters (url of - // image) - if (!json_util::readJson( - req, asyncResp->res, "Image", - imageUrl, "WriteProtected", - writeProtected, "UserName", - userName, "Password", password, - "Inserted", inserted, - "TransferMethod", - transferMethod, - "TransferProtocolType", - transferProtocolType)) - { - BMCWEB_LOG_DEBUG - << "Image is not provided"; - return; - } - - bool paramsValid = validateParams( - asyncResp->res, imageUrl, inserted, - transferMethod, - transferProtocolType); - - if (paramsValid == false) - { - return; - } - // manager is irrelevant for // VirtualMedia dbus calls - doMountVmLegacy(asyncResp, service, - resName, imageUrl, - !(*writeProtected), - std::move(*userName), - std::move(*password)); + doMountVmLegacy( + asyncResp, service, resName, + actionParams.imageUrl, + !(*actionParams.writeProtected), + std::move(*actionParams.userName), + std::move(*actionParams.password)); return; } @@ -928,7 +926,7 @@ inline void requestNBDVirtualMediaRoutes(App& app) "/redfish/v1/Managers//VirtualMedia//Actions/VirtualMedia.EjectMedia") .privileges(redfish::privileges::postVirtualMedia) .methods(boost::beast::http::verb::post)( - [](const crow::Request& req, + [](const crow::Request&, const std::shared_ptr& asyncResp, const std::string& name, const std::string& resName) { if (name != "bmc") @@ -940,9 +938,8 @@ inline void requestNBDVirtualMediaRoutes(App& app) } crow::connections::systemBus->async_method_call( - [asyncResp, req, - resName](const boost::system::error_code ec, - const GetObjectType& getObjectType) { + [asyncResp, resName](const boost::system::error_code ec, + const GetObjectType& getObjectType) { if (ec) { BMCWEB_LOG_ERROR @@ -956,7 +953,7 @@ inline void requestNBDVirtualMediaRoutes(App& app) BMCWEB_LOG_DEBUG << "GetObjectType: " << service; crow::connections::systemBus->async_method_call( - [resName, service, req, asyncResp{asyncResp}]( + [resName, service, asyncResp{asyncResp}]( const boost::system::error_code ec, ManagedObjectType& subtree) { if (ec) -- cgit v1.2.3