#
8cb2c024
|
| 27-Mar-2024 |
Ed Tanous <ed@tanous.net> |
Fix moves/forward
Clang has new checks for std::move/std::forward correctness, which catches quite a few "wrong" things where we were making copies of callback handlers.
Unfortunately, the lambda s
Fix moves/forward
Clang has new checks for std::move/std::forward correctness, which catches quite a few "wrong" things where we were making copies of callback handlers.
Unfortunately, the lambda syntax of
callback{std::forward<Callback>(callback)}
in a capture confuses it, so change usages to callback = std::forward<Callback>(callback)
to be consistent.
Tested: Redfish service validator passes.
Change-Id: I7a111ec00cf78ecb7d5f5b102c786c1c14d74384 Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
4da0490b
|
| 19-Mar-2024 |
Ed Tanous <ed@tanous.net> |
Use no-switch-default on clang
clang-18 improves this check so that we can actually use it. Enable it and fix all violations.
Change-Id: Ibe4ce19c423d447a4cbe593d1abba948362426af Signed-off-by: Ed
Use no-switch-default on clang
clang-18 improves this check so that we can actually use it. Enable it and fix all violations.
Change-Id: Ibe4ce19c423d447a4cbe593d1abba948362426af Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
d02aad39
|
| 13-Feb-2024 |
Ed Tanous <ed@tanous.net> |
Create Redfish specific setProperty call
There are currently 78 sdbusplus::asio::setProperty calls in redfish-core. The error handler for nearly all of them looks something like:
``` if (ec) {
Create Redfish specific setProperty call
There are currently 78 sdbusplus::asio::setProperty calls in redfish-core. The error handler for nearly all of them looks something like:
``` if (ec) { const sd_bus_error* dbusError = msg.get_error(); if ((dbusError != nullptr) && (dbusError->name == std::string_view( "xyz.openbmc_project.Common.Error.InvalidArgument"))) { BMCWEB_LOG_WARNING("DBUS response error: {}", ec); messages::propertyValueIncorrect(asyncResp->res, "<PropertyName>", <PropertyValue>); return; } messages::internalError(asyncResp->res); return; } messages::success(asyncResp->res);
```
In some cases there are more errors handled that translate to more error messages, but the vast majority only handle InvalidArgument. Many of these, like the ones in account_service.hpp, do the error handling in a lambda, which causes readability problems. This commit starts to make things more consistent, and easier for trivial property sets.
This commit invents a setDbusProperty method in the redfish namespace that tries to handle all DBus errors in a consistent manner. Looking for input on whether this will work before changing over the other 73 calls. Overall this is less code, fewer inline lambdas, and defaults that should work for MOST use cases of calling an OpenBMC daemon, and fall back to more generic errors when calling a "normal" dbus daemon.
As part of this, I've ported over several examples. Some things that might be up in the air: 1. Do we always return 204 no_content on property sets? Today there's a mix of 200, with a Base::Success message, and 204, with an empty body. 2. Do all DBus response codes map to the same error? A majority are covered by xyz.openbmc_project.Common.Error.InvalidArgument, but there are likely differences. If we allow any daemon to return any return code, does that cause compatibility problems later?
Tested: ``` curl -k --user "root:0penBmc" -H "Content-Type: application/json" -X PATCH -d '{"HostName":"openbmc@#"}' https://192.168.7.2/redfish/v1/Managers/bmc/EthernetInterfaces/eth0 ```
Returns the appropriate error in the response Base.1.16.0.PropertyValueIncorrect
Change-Id: If033a1112ba516792c9386c997d090c8f9094f3a Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
0885057c
|
| 06-Mar-2024 |
Ed Tanous <ed@tanous.net> |
Clean up power/thermal to use readJsonObject
Change-Id: I69ed29472b209e8782be63c3f0f2e8ca63dc14a4 Signed-off-by: Ed Tanous <ed@tanous.net>
|
#
6f4bd290
|
| 08-Mar-2024 |
Alexander Hansen <alexander.hansen@9elements.com> |
fix usage of std::ranges::replace_copy
As hinted at in the usage example [1], the destination range must have sufficient size to contain the elements.
If the destination range has size 0, then it w
fix usage of std::ranges::replace_copy
As hinted at in the usage example [1], the destination range must have sufficient size to contain the elements.
If the destination range has size 0, then it will be empty after the function call. Then the "Name" property in powersupply json will be "" and there will only be one powersupply because of the deduplication code.
Tested: Powersupplies appear as usual
References:
[1] https://en.cppreference.com/w/cpp/algorithm/ranges/replace_copy
Change-Id: I81dd21a2dd6eb9b29a67007d6d6229d3a752690f Signed-off-by: Alexander Hansen <alexander.hansen@9elements.com>
show more ...
|
#
aaf08ac7
|
| 04-Oct-2023 |
Matt Simmering <matthew.simmering@intel.com> |
Add StandbyOffline to Redfish sensor getState
Currently in Redfish a sensor's state will only be "Absent" or "Enabled". In the case where a sensor is marked as not available on D-Bus it would be mo
Add StandbyOffline to Redfish sensor getState
Currently in Redfish a sensor's state will only be "Absent" or "Enabled". In the case where a sensor is marked as not available on D-Bus it would be more accurate and informative to have the state as "StandbyOffline". A sensor's availability will be published on its xyz.openbmc_project.State.Decorator.Availability interface.
Tested on Intel system. Put sensors into an unavailable state by putting CPU into S5 power state, and verified that Redfish state is "StandbyOffline". Then when sensors are back in an available state their Redfish state is back to "Enabled".
Change-Id: I4b846b678a0f90f60d182ac38f1becd21265cdd2 Signed-off-by: Matt Simmering <matthew.simmering@intel.com>
show more ...
|
#
18f8f608
|
| 18-Jul-2023 |
Ed Tanous <edtanous@google.com> |
Remove some boost includes
The less we rely on boost, and more on std algorithms, the less people have to look up, and the more likely that our code will deduplicate.
Replace all uses of boost::alg
Remove some boost includes
The less we rely on boost, and more on std algorithms, the less people have to look up, and the more likely that our code will deduplicate.
Replace all uses of boost::algorithms with std alternatives.
Tested: Redfish Service Validator passes.
Change-Id: I8a26f39b5709adc444b4178e92f5f3c7b988b05b Signed-off-by: Ed Tanous <edtanous@google.com>
show more ...
|
#
8ece0e45
|
| 02-Jan-2024 |
Ed Tanous <ed@tanous.net> |
Fix spelling mistakes
These were found with: codespell -w $(git ls-files | grep "\.[hc]\(pp\)\?$")
At some point in the future, we might want to get this enabled in CI.
Change-Id: Iccb57b2adfd06a2
Fix spelling mistakes
These were found with: codespell -w $(git ls-files | grep "\.[hc]\(pp\)\?$")
At some point in the future, we might want to get this enabled in CI.
Change-Id: Iccb57b2adfd06a2e177e99db2923fe4e8e329118 Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
931edc79
|
| 01-Nov-2023 |
Ed Tanous <edtanous@google.com> |
Make callback a template to avoid memory leak
Tested: Gunnar built this and below for a p10bmc, webui-vue looks reasonable and the Validator had no new errors. Did a few operations: delete a log, se
Make callback a template to avoid memory leak
Tested: Gunnar built this and below for a p10bmc, webui-vue looks reasonable and the Validator had no new errors. Did a few operations: delete a log, set an ntp server, etc.
Change-Id: I587ccd04515164fce1ea0bf5baf9f820347c63e6 Signed-off-by: Ed Tanous <edtanous@google.com> Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
show more ...
|
#
6804b5c8
|
| 31-Oct-2023 |
Ed Tanous <edtanous@google.com> |
Remove use after free in error handling path
Change-Id: I04476b016584f1d19af035ae51e0c04076b4de0b Signed-off-by: Ed Tanous <edtanous@google.com>
|
#
5a39f77a
|
| 20-Oct-2023 |
Patrick Williams <patrick@stwcx.xyz> |
clang-format: copy latest and re-format
clang-format-17 has some backwards incompatible changes that require additional settings for best compatibility and re-running the formatter. Copy the latest
clang-format: copy latest and re-format
clang-format-17 has some backwards incompatible changes that require additional settings for best compatibility and re-running the formatter. Copy the latest .clang-format from the docs repository and reformat the repository.
Change-Id: I2f9540cf0d545a2da4d6289fc87b754f684bc9a7 Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
show more ...
|
#
80f79a40
|
| 24-Aug-2023 |
Michael Shen <gpgpgp@google.com> |
Fix typo `DBusInteracesMap` -> `DBusInterfacesMap`
Change-Id: I9a851076eccee9d79ad7bb036e58b717e06ad5d1 Signed-off-by: Michael Shen <gpgpgp@google.com>
|
#
3544d2a7
|
| 06-Aug-2023 |
Ed Tanous <edtanous@google.com> |
Use ranges
C++20 brought us std::ranges for a lot of algorithms. Most of these conversions were done using comby, similar to:
``` comby -verbose 'std::lower_bound(:[a].begin(),:[b].end(),:[c])' 's
Use ranges
C++20 brought us std::ranges for a lot of algorithms. Most of these conversions were done using comby, similar to:
``` comby -verbose 'std::lower_bound(:[a].begin(),:[b].end(),:[c])' 'std::ranges::lower_bound(:[a], :[c])' $(git ls-files | grep "\.[hc]\(pp\)\?$") -in-place ```
Change-Id: I0c99c04e9368312555c08147d474ca93a5959e8d Signed-off-by: Ed Tanous <edtanous@google.com>
show more ...
|
#
0f3fbe52
|
| 11-Aug-2023 |
Anjaliintel-21 <anjali.ray@intel.com> |
Add negation to logic
As the value of the PowerInputWatts and PowerOutputWatts are getting exchanged,so I added negation in the logic.This will correct the values.
Tested: I have tested it and now
Add negation to logic
As the value of the PowerInputWatts and PowerOutputWatts are getting exchanged,so I added negation in the logic.This will correct the values.
Tested: I have tested it and now the values are coming correct.
Change-Id: I67bf6c5050ceb05c13419b370105d80f913b0c17 Signed-off-by: Anjaliintel-21 <anjali.ray@intel.com>
show more ...
|
#
3f5eb755
|
| 28-Jun-2023 |
Ban Feng <kcfeng0@nuvoton.com> |
Fix unable to find sensor object
This is due to MemberId is the combination of sensorType+sensorName, and we only extract sensorName from objectsWithConnection. Therefore, prepend the sensorType to
Fix unable to find sensor object
This is due to MemberId is the combination of sensorType+sensorName, and we only extract sensorName from objectsWithConnection. Therefore, prepend the sensorType to prevent this from occuring.
Tested: code complies, and confirmed via curl, function works.
Change-Id: Ic76607576475547030b9556a64c902e560aabf5d Signed-off-by: Ban Feng <kcfeng0@nuvoton.com> Signed-off-by: Ed Tanous <edtanous@google.com>
show more ...
|
#
62598e31
|
| 17-Jul-2023 |
Ed Tanous <ed@tanous.net> |
Replace logging with std::format
std::format is a much more modern logging solution, and gives us a lot more flexibility, and better compile times when doing logging.
Unfortunately, given its level
Replace logging with std::format
std::format is a much more modern logging solution, and gives us a lot more flexibility, and better compile times when doing logging.
Unfortunately, given its level of compile time checks, it needs to be a method, instead of the stream style logging we had before. This requires a pretty substantial change. Fortunately, this change can be largely automated, via the script included in this commit under scripts/replace_logs.py. This is to aid people in moving their patchsets over to the new form in the short period where old patches will be based on the old logging. The intention is that this script eventually goes away.
The old style logging (stream based) looked like.
BMCWEB_LOG_DEBUG << "Foo " << foo;
The new equivalent of the above would be: BMCWEB_LOG_DEBUG("Foo {}", foo);
In the course of doing this, this also cleans up several ignored linter errors, including macro usage, and array to pointer deconstruction.
Note, This patchset does remove the timestamp from the log message. In practice, this was duplicated between journald and bmcweb, and there's no need for both to exist.
One design decision of note is the addition of logPtr. Because the compiler can't disambiguate between const char* and const MyThing*, it's necessary to add an explicit cast to void*. This is identical to how fmt handled it.
Tested: compiled with logging meson_option enabled, and launched bmcweb
Saw the usual logging, similar to what was present before: ``` [Error include/webassets.hpp:60] Unable to find or open /usr/share/www/ static file hosting disabled [Debug include/persistent_data.hpp:133] Restored Session Timeout: 1800 [Debug redfish-core/include/event_service_manager.hpp:671] Old eventService config not exist [Info src/webserver_main.cpp:59] Starting webserver on port 18080 [Error redfish-core/include/event_service_manager.hpp:1301] inotify_add_watch failed for redfish log file. [Info src/webserver_main.cpp:137] Start Hostname Monitor Service... ``` Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I86a46aa2454be7fe80df608cb7e5573ca4029ec8
show more ...
|
#
db0d36ef
|
| 30-Jun-2023 |
Ed Tanous <edtanous@google.com> |
Add contains type tidy check
On general container usage, contains() is more descriptive than count() > 0. We have one violation of this, fix it and enable the check.
Tested: Clang-tidy passes.
Ch
Add contains type tidy check
On general container usage, contains() is more descriptive than count() > 0. We have one violation of this, fix it and enable the check.
Tested: Clang-tidy passes.
Change-Id: Ib5702ef97c6da033b6587c9cfebbe30dfbfe80b4 Signed-off-by: Ed Tanous <edtanous@google.com>
show more ...
|
#
8b24275d
|
| 27-Jun-2023 |
Ed Tanous <edtanous@google.com> |
Rename all error_code instances to ec
We're not consistent here, which leads to people copying and pasting code all over, which has lead to a bunch of different names for error codes.
This commit c
Rename all error_code instances to ec
We're not consistent here, which leads to people copying and pasting code all over, which has lead to a bunch of different names for error codes.
This commit changes to coerce them all to "ec", because that's what boost uses for a naming convention.
Tested: Rename only, code compiles.
Change-Id: I7053cc738faa9f7a82f55fc46fc78618bdf702a5 Signed-off-by: Ed Tanous <edtanous@google.com>
show more ...
|
#
9ae226fa
|
| 21-Jun-2023 |
George Liu <liuxiwei@inspur.com> |
Refactor setProperty method
SetProperty is a method we should use more, and use consistently in the codebase, this commit makes it consistently used from the utility namespace.
Tested: Refactor. C
Refactor setProperty method
SetProperty is a method we should use more, and use consistently in the codebase, this commit makes it consistently used from the utility namespace.
Tested: Refactor. Code compiles.
Signed-off-by: George Liu <liuxiwei@inspur.com> Change-Id: I5939317d23483e16bd98a8298f53e75604ef374d
show more ...
|
#
5eb468da
|
| 20-Jun-2023 |
George Liu <liuxiwei@inspur.com> |
Refactor getManagedObjects method
Since the getManagedObjects method has been implemented in dbus_utility and this commit is to integrate all the places where the GetManagedObjects method is obtaine
Refactor getManagedObjects method
Since the getManagedObjects method has been implemented in dbus_utility and this commit is to integrate all the places where the GetManagedObjects method is obtained, and use the method in dbus_utility uniformly.
Signed-off-by: George Liu <liuxiwei@inspur.com> Change-Id: Ic13f2bef7b30f805cd3444a75d7df17b031f2eb0
show more ...
|
#
b90d14f2
|
| 31-May-2023 |
Myung Bae <myungbae@us.ibm.com> |
Fix Error log entries to Warning
Some logging entries are categorized as ERROR, but they would better be as WARNING.
1) ``` $ curl -k -X GET https://${bmc}:18080/redfish/v1/Managers/bmc/LogServices
Fix Error log entries to Warning
Some logging entries are categorized as ERROR, but they would better be as WARNING.
1) ``` $ curl -k -X GET https://${bmc}:18080/redfish/v1/Managers/bmc/LogServices/Dump/Entries/INVALID { .... "code": "Base.1.13.0.InternalError", "message": "The request failed due to an internal service error. The service is still operational." } }%
(2023-06-01 23:29:40) [ERROR "log_services.hpp":665] Can't find Dump Entry (2023-06-01 23:29:40) [CRITICAL "error_messages.cpp":282] Internal Error \ ../../../../../../../../../bmcweb/redfish-core/lib/log_services.hpp(666:36) \ `redfish::getDumpEntryById(const std::shared_ptr<bmcweb::AsyncResp>&, const std::string&, \ const std::string&)::<lambda(const boost::system::error_code&, const dbus::utility::ManagedObjectType&)>`: ```
2) ``` $ curl -k -X GET https://${bmc}:18080/redfish/v1/UpdateService/FirmwareInventory/INVALID
(2023-05-31 15:03:38) [ERROR "update_service.hpp":1010] Input swID X1cd6ce5fZ not found! ```
Tested: - Set bmcweb-logging=error to obtain Error or higher logs - Run the above commands and watch out logs - Redfish validator passed and see whether there are unexpected error or higher level logs
Change-Id: I5f14eedd68fd3454cdf2a5b2f34442a7718e718a Signed-off-by: Myung Bae <myungbae@us.ibm.com>
show more ...
|
#
ac106bf6
|
| 07-Jun-2023 |
Ed Tanous <edtanous@google.com> |
Consistently name AsyncResp variables
In about half of our code, AsyncResp objects take the name asyncResp, and in the other half they take the name aResp. While the difference between them is negl
Consistently name AsyncResp variables
In about half of our code, AsyncResp objects take the name asyncResp, and in the other half they take the name aResp. While the difference between them is negligeble and arbitrary, having two naming conventions makes it more difficult to do automated changes over time via grep.
This commit was generated automtatically with the command: git grep -l 'aResp' | xargs sed -i 's|aResp|asyncResp|g'
Tested: Code compiles.
Change-Id: Id363437b6a78f51e91cbf60aa0a0c2286f36a037 Signed-off-by: Ed Tanous <edtanous@google.com>
show more ...
|
#
aec0ec30
|
| 31-May-2023 |
Myung Bae <myungbae@us.ibm.com> |
Fix NotFound Sensors to report as 404
Sensors that are not found are incorrectly reported as internal Server error and its logging is done as Error. . It will be changed to 404 - Not found and its l
Fix NotFound Sensors to report as 404
Sensors that are not found are incorrectly reported as internal Server error and its logging is done as Error. . It will be changed to 404 - Not found and its logging will be WARNING.
``` redfishtool raw GET -r ${bmc} -u admin -p 0penBmc0 -S Always /redfish/v1/Chassis/chassis/Sensors/temperature_PCIE_1_Temp_invalid redfishtool: Transport: Response Error: status_code: 500 -- Internal Server Error redfishtool: raw: Error getting response
curl -k -X GET https://${bmc}/redfish/v1/Chassis/chassis/Sensors/temperature_PCIE_1_Temp_invalid { "@odata.id": "/redfish/v1/Chassis/chassis/Sensors/temperature_PCIE_1_Temp_invalid", "error": { "@Message.ExtendedInfo": [ { "@odata.type": "#Message.v1_1_1.Message", "Message": "The request failed due to an internal service error. The service is still operational.", "MessageArgs": [], "MessageId": "Base.1.13.0.InternalError", "MessageSeverity": "Critical", "Resolution": "Resubmit the request. If the problem persists, consider resetting the service." } ], "code": "Base.1.13.0.InternalError", "message": "The request failed due to an internal service error. The service is still operational." } }% ```
Its logging is
``` redfishtool: Transport: Response Error: status_code: 500 -- Internal Server Error(2023-05-31 15:16:43) [CRITICAL "error_messages.cpp":282] Internal Error ../../../../../../../../../bmcweb/redfish-core/lib/sensors.hpp(2928:36) `redfish::sensors::handleSensorGet(App&, const crow::Request&, const std::shared_ptr<bmcweb::AsyncResp>&, const std::string&, const std::string&)::<lambda(const boost::system::error_code&, const dbus::utility::MapperGetObject&)>`:
(2023-05-31 15:16:43) [ERROR "sensors.hpp":2929] Sensor getSensorPaths resp_handler: Dbus error generic:5 ```
The expected behavior will be
``` redfishtool raw GET -r ${bmc} -u admin -p 0penBmc0 -S Always /redfish/v1/Chassis/chassis/Sensors/temperature_PCIE_1_Temp_invalid redfishtool: Transport: Response Error: status_code: 404 -- Not Found
curl -k -X GET https://${bmc}/redfish/v1/Chassis/chassis/Sensors/temperature_PCIE_1_Temp_invalid { "@odata.id": "/redfish/v1/Chassis/chassis/Sensors/temperature_PCIE_1_Temp_invalid", "error": { "@Message.ExtendedInfo": [ { "@odata.type": "#Message.v1_1_1.Message", "Message": "The requested resource of type temperature_PCIE_1_Temp_invalid named 'Sensor' was not found.", "MessageArgs": [ "temperature_PCIE_1_Temp_invalid", "Sensor" ], "MessageId": "Base.1.13.0.ResourceNotFound", "MessageSeverity": "Critical", "Resolution": "Provide a valid resource identifier and resubmit the request." } ], "code": "Base.1.13.0.ResourceNotFound", "message": "The requested resource of type temperature_PCIE_1_Temp_invalid named 'Sensor' was not found." } }%
```
Its logging will be:
``` (2023-05-31 20:17:55) [WARNING "sensors.hpp":2928] Sensor not found from getSensorPaths ```
Change-Id: I5a51c1b5c0125b5396068311602964d4e249e297 Signed-off-by: Myung Bae <myungbae@us.ibm.com>
show more ...
|
#
283860f5
|
| 29-Aug-2022 |
Ed Tanous <edtanous@google.com> |
Don't push non finite numbers to Redfish
Redfish Sensor schema is based around Edm.Number, which doesn't have an allowance for things like infinity, -infinity, or NAN. Because these are theoretical
Don't push non finite numbers to Redfish
Redfish Sensor schema is based around Edm.Number, which doesn't have an allowance for things like infinity, -infinity, or NAN. Because these are theoretically possible in the dbus interfaces, we need to omit the properties if they are set to anything that Redfish doesn't support.
Because the DBus sensor Value interface relies on NAN to represent unavailable, this is explicitly set to null in the json response. This behavior was discussed with DMTF in a forum meeting, and is the protocol-correct behavior for handling unavailable numbers. All other number-assigning dbus properties are omitted from the response, to show that they are "not supported" if they produce out-of-range values.
Tested: Unclear if there are any implementations that do this to test against. Code inspection only.
Redfish-service-validator passes (on previous patchset).
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ia3dde24cd604b0bb5dc596e7b8a6461a4b339b71
show more ...
|
#
ef4c65b7
|
| 24-Apr-2023 |
Ed Tanous <edtanous@google.com> |
Boost::urls::format
Boost 1.82 dropped a lovely new toy, boost::urls::format, which is a lot like our urlFromPieces method, but better in that it makes the resulting uris more readable, and allows d
Boost::urls::format
Boost 1.82 dropped a lovely new toy, boost::urls::format, which is a lot like our urlFromPieces method, but better in that it makes the resulting uris more readable, and allows doing things like fragments in a single line instead of multiple. We should prefer it in some cases.
Tested: Redfish service validator passes. Spot checks of URLs work as expected. Unit tests pass.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ia7b38f0a95771c862507e7d5b4aa68aa1c98403c
show more ...
|