| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
This saves about 4k on the binary size
Tested: Redfish service validator passes.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I9546227a19c691b1aecb80e80307889548c0293f
|
|
Part of enforcing cpp core guidelines involves explicitly including all
constructors required on a non-trivial class. We were missing quite a
few. In all cases, the copy/move/and operator= methods are simply
deleted.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ie8d6e8bf2bc311fa21a9ae48b0d61ee5c1940999
|
|
https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/49840 was recently
checked in that made some changes here, and had issues that weren't
caught on my system because of how my sensor setup is setup. This
commit changes to only make a single copy, then filter the copy inplace,
rather than make a copy, filter, then do the move.
Tested: Ran redfish service validator in a similar setup to Romulus, and
saw that it passed with the same failures as previously.
Unit tested:
curl --insecure -u root:0penBmc "https://192.168.7.2:443/redfish/v1/TaskService"
now succeeds
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I5b59b7074e0a7aad4e95c5ddb625ff24170f3981
|
|
For all async calls, we should be consistently capturing non trivial
objects by const reference. This corrects bmcweb to be consistent and
capture errors by const value, and objects by const reference.
Tested: Code compiles. Trivial changes.
This saves about 300 bytes on our compressed binary size.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ib3e0b6edef9803a1c480701556949488406305d4
|
|
Some subsystems seem to have invented their own typedefs for this stuff,
move to using the one typedef in dbus::utility so we're consistent, and
we reduce our templates.
Tested: code compiles
This saves a negligible amount (104 bytes compressed) on our binary
size.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I952ea1f960aa703808d0ac80f35dc24cdd8d5027
|
|
It simplifies a lot of code and after changing sdbusplus implementation
slightly reduces binary size if used together with:
https://gerrit.openbmc-project.xyz/c/openbmc/sdbusplus/+/49467
* Uncompressed size: 3033148 -> 3012164, -20984 B
* gzip compressed size: 1220586 -> 1214625, -5961 B
Tested:
- Redfish validator output is the same before and after the change
Change-Id: Ibe3227d3f4230de2363ba3d9396e51130c8240a5
Signed-off-by: Jonathan Doman <jonathan.doman@intel.com>
Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>
|
|
This commit resolves https://github.com/openbmc/bmcweb/issues/208
Issue was an example of common error https://github.com/openbmc/bmcweb/issues/12
Current mechanism creates incorrect propagation of health statuses to additional nodes. Dbus paths are matched solely by looking at beginning of their path using boost::starts_with. To make an example on memory health statuses system, that created unwanted one-sided connection of dimms placed on path ".../dimm2" and '.../dimm21'. When status object with path '.../dimm21/warning' appeared, it was altering the health setting on both dimms mentioned, just because their beginning path was in fact matching string-wise. That behaviour needed a change to prevent presented imprecise matching.
This commit adds a check for a slash '/' sign which marks closing of singular path part. Now objects that are compared to their destination paths are guaranteed not to interact with partially cut names where single characters could determine its match. A slash '/' is not required to match to an object if their path is exactly identical (due to paths not being finished by slash) - that is ensured by use of second, alternative match condition checked with boost::equals.
Tested: I used bmcweb's memory mechanism (redfish-core/lib/memory.hpp) to assess that statuses are not being incorrectly propagated anymore after introduction of this commit. All dimm health statuses are presented on redfish. I checked health statuses of example dimms that could be vulnerable to this issue, to be exact dimm10 and dimm1. Then, I proceeded to create an warning status (reverse association object: "warning") association object (https://github.com/openbmc/docs/blob/master/architecture/object-mapper.md#associations) with object path /xyz/openbmc_project/inventory/system/chassis/motherboard/dimm10 so that getAllStatusAssociations() function in redfish-core/lib/health.hpp could find it and apply health status change. By getting the data before and after creating association object prepared for dimm10, the difference was seen only in status of dimm10, which is appropriate to created association. I repeated the process again for dimm22 and dimm2. Observation of health statuses of both dimms in mentioned cases led to trustworthy conclusion - string-wise comparition does not create unwanted propagations anymore.
Signed-off-by: Karol Wojciechowski <karol.wojciechowski@intel.com>
Change-Id: Id5e113373f537afa33dc206ed9e2e90598e23f8f
|
|
Reduces the total number of lines and will allow for easier testing of
the redfish responses.
A main purpose of the node class was to set app.routeDynamic(). However
now app.routeDynamic can handle the complexity that was once in critical
to node. The macro app.routeDynamic() provides a shorter cleaner
interface to the unerlying app.routeDyanic call. The old pattern set
permissions for 6 interfaces (get, head, patch, put, delete_, and post)
even if only one interface is created. That pattern creates unneeded
code that can be safely removed with no effect.
Unit test for the responses would have to mock the node the class in
order to fully test responses.
see https://github.com/openbmc/bmcweb/issues/181
The following files still need node to be extracted.
virtual_media.hpp
account_service.hpp
redfish_sessions.hpp
ethernet.hpp
The files above use a pattern that is not trivial to address. Often their
responses call an async lambda capturing the inherited class. ie
(https://github.com/openbmc/bmcweb/blob/ffed87b5ad1797ca966d030e7f979770
28d258fa/redfish-core/lib/account_service.hpp#L1393)
At a later point I plan to remove node from the files above.
Tested:
I ran the docker unit test with the following command.
WORKSPACE=$(pwd) UNIT_TEST_PKG=bmcweb
./openbmc-build-scripts/run-unit-test-docker.sh
I ran the validator and this change did not create any issues.
python3 RedfishServiceValidator.py -c config.ini
Signed-off-by: John Edward Broadbent <jebr@google.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I147a0289c52cb4198345b1ad9bfe6fdddf57f3df
|
|
Get the core using AsyncResp everywhere, and not have each individual handler
creating its own object.We can call app.handle() without fear of the response
getting ended after the first tree is done populating.
Don't use res.end() anymore.
Tested:
1. Validator passed.
Signed-off-by: zhanghaicheng <zhanghch05@inspur.com>
Change-Id: I867367ce4a0caf8c4b3f4e07e06c11feed0782e8
|
|
Now that CI can handle clang-tidy, and a lot of the individual fixes
have landed for the various static analysis checks, lets see how close
we are.
This includes bringing a bunch of the code up to par with the checks
that require. Most of them fall into the category of extraneous else
statements, const correctness problems, or extra copies.
Tested:
CI only. Unit tests pass.
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I9fbd346560a75fdd3901fa40c57932486275e912
|
|
This commit enables clang warnings, and fixes all warnings that were
found. Most of these fall into a couple categories:
Variable shadow issues were fixed by renaming variables
unused parameter warnings were resolved by either checking error codes
that had been ignored, or removing the name of the variable from the
scope.
Other various warnings were fixed in the best way I was able to come up
with.
Note, the redfish Node class is especially insidious, as it causes all
imlementers to have variables for parameters, regardless of whether or
not they are used. Deprecating the Node class is on my list of things
to do, as it adds extra overhead, and in general isn't a useful
abstraction. For now, I have simply fixed all the handlers.
Tested:
Added the current meta-clang meta layer into bblayers.conf, and added
TOOLCHAIN_pn-bmcweb = "clang" to my local.conf
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: Ia75b94010359170159c703e535d1c1af182fe700
|
|
This is from openbmc/docs/style/cpp/.clang-format
Other OpenBMC repos are doing the same.
Tested: Built and validator passed.
Change-Id: Ief26c755c9ce012823e16a506342b0547a53517a
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
This adds health support to dimms. It also updates
the health object to look for an individual inventory
items health.
Tested:
Validator passed
{
"@odata.id": "/redfish/v1/Systems/system/Memory/memory_device11",
"@odata.type": "#Memory.v1_6_0.Memory",
"CapacityMiB": 129728,
"DataWidthBits": 64,
"Id": "memory_device11",
"Manufacturer": "Intel",
"MemoryDeviceType": "xyz.openbmc_project.Inventory.Item.Dimm.DeviceType.Logical",
"Name": "DIMM Slot",
"PartNumber": "",
"SerialNumber": "",
"Status": {
"Health": "Critical",
"HealthRollup": "Critical",
"State": "Enabled"
}
}
Change-Id: If2e1450b4228036f00ff78e6484e8da409a8039b
Signed-off-by: James Feist <james.feist@linux.intel.com>
|
|
Populate does a mapper call and a get managed objects
and should only be called once. Enforce it.
Tested: No actual change, it is currently never called
twice, this is just for future protection with multiple
async calls.
Change-Id: I8fb9d8d19b2aa2a1c957a0ac8b609adf5e6ba6d0
Signed-off-by: James Feist <james.feist@linux.intel.com>
|
|
These fields were missing Health. Add health objects
to get the health associated with these items. Also update
the health object to be able to add more than the 'main'
health for a url, by allowing passing a json reference.
Also, add a 'children' vector of more shared_ptr<HealthPopulate>
so we don't double up on d-bus calls.
Tested:
"MemorySummary": {
"Status": {
"Health": "OK",
"HealthRollup": "OK",
"State": "Disabled"
},
"TotalSystemMemoryGiB": 0
},
"Model": "S2600WFT",
"Name": "system",
"PartNumber": "..........",
"PowerState": "On",
"ProcessorSummary": {
"Count": 2,
"Model": "Intel Xeon processor",
"Status": {
"Health": "OK",
"HealthRollup": "OK",
"State": "Enabled"
}
},
Change-Id: I06f802da93a44cfbac40b63d507e3b9faf0c999a
Signed-off-by: James Feist <james.feist@linux.intel.com>
|
|
using the list of warnings from here:
https://github.com/lefticus/cppbestpractices/blob/e73393f25a85f83fed7399d8b65cb117d00b2231/02-Use_the_Tools_Available.md#L100
Seems like a good place to start, and would improve things a bit
type-wise. This patchset attempts to correct all the issues in one
shot.
Tested:
It builds. Will test various subsystems that have been touched
Signed-off-by: Ed Tanous <ed.tanous@intel.com>
Change-Id: I588c26440e5a97f718a0f0ea74cc84107d53aa1e
|
|
Look for associations for inventory and compare the
inventory warning / critical and global warning / critical
to get HealthRollup and Health respectively.
Tested:
Used sensor override to set BMC temp to Upper critical
and saw:
{
"@odata.context": "/redfish/v1/$metadata#Chassis.Chassis",
"@odata.id": "/redfish/v1/Chassis/WFP_Baseboard",
"@odata.type": "#Chassis.v1_4_0.Chassis",
"ChassisType": "RackMount",
"Id": "WFP_Baseboard",
"Links": {
"ComputerSystems": [
{
"@odata.id": "/redfish/v1/Systems/system"
}
],
"ManagedBy": [
{
"@odata.id": "/redfish/v1/Managers/bmc"
}
]
},
"Manufacturer": "Intel Corporation",
"Model": "S2600WFT",
"Name": "WFP_Baseboard",
"PartNumber": "123456789",
"Power": {
"@odata.id": "/redfish/v1/Chassis/WFP_Baseboard/Power"
},
"PowerState": "Off",
"SerialNumber": "123454321",
"Status": {
"Health": "Warning",
"HealthRollup": "Critical",
"State": "StandbyOffline"
},
"Thermal": {
"@odata.id": "/redfish/v1/Chassis/WFP_Baseboard/Thermal"
}
}
Change-Id: Idd9e832db18bb4769f1452fe243d68339a6f844d
Signed-off-by: James Feist <james.feist@linux.intel.com>
|
