redfish: Add Chassis listing associated drive

If chassis has drives a drive url is added to the chassis, of the form:
redfish/v1/Chassis/<chassis>/Drives

When queried, the drive URL will list all d

redfish: Add Chassis listing associated drive

If chassis has drives a drive url is added to the chassis, of the form:
redfish/v1/Chassis/<chassis>/Drives

When queried, the drive URL will list all drives associated with the
chassis. This is in accordance with the redfish schema.

Samples for the following URLs are below

wget -qO- http://localhost:80/redfish/v1/Chassis/DC_SCM/Drives
{
"@odata.id": "/redfish/v1/Chassis/DC_SCM/Drives",
"@odata.type": "#DriveCollection.DriveCollection",
"Members": [
{
"@odata.id": "/redfish/v1/Chassis/DC_SCM/Drives/mmcblk0"
}
],
"Members@odata.count": "1",
"Name": "Drive Collection"
}

Tested:
With the redfish validator: No new errors

Change-Id: Ibdbe7fee5014d6515a77683c8eaca9ca86b6b148
Signed-off-by: John Edward Broadbent <jebr@google.com>

show more ...

Try to fix the lambda formatting issue

clang-tidy has a setting, LambdaBodyIndentation, which it says:
"For callback-heavy code, it may improve readability to have the
signature indented two levels

Try to fix the lambda formatting issue

clang-tidy has a setting, LambdaBodyIndentation, which it says:
"For callback-heavy code, it may improve readability to have the
signature indented two levels and to use OuterScope."

bmcweb is very callback heavy code. Try to enable it and see if that
improves things. There are many cases where the length of a lambda call
will change, and reindent the entire lambda function. This is really
bad for code reviews, as it's difficult to see the lines changed. This
commit should resolve it. This does have the downside of reindenting a
lot of functions, which is unfortunate, but probably worth it in the
long run.

All changes except for the .clang-format file were made by the robot.

Tested: Code compiles, whitespace changes only.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ib4aa2f1391fada981febd25b67dcdb9143827f43

show more ...

health: take json_ptr instead of reference when filling status

The existing codes populates the health status on the |AsyncResponse| or
a given JSON reference. This doesn't work if we want to popula

health: take json_ptr instead of reference when filling status

The existing codes populates the health status on the |AsyncResponse| or
a given JSON reference. This doesn't work if we want to populates status
on an array of objects, since the array can be resized which changes the
address of each object.

This commit changed the contructor to take a JSON pointer instead.
|HealthPopulate| will populates status on
|AsyncResponse->res.jsonValue|[json_ptr]. If the point can't be resolved
in the |jsonValue|, |HealthPopulate| populates nothing.

Fixed all places where the old reference based constructor is used.

This commit is extremely useful when implementing efficient level-1
expand handler on ResourceCollections. It also prevents issues on
reference lifecycles.

Tested:
1. It builds
2. Tested DIMM/System/Storage health on real hardware, works as expected
3. Tested on Redfish Service Validator, no new failures on health
properties.

Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I305515522af50b48be92a3f4689d8166f3bc0cc0

show more ...

Remove brace initialization of json objects

Brace initialization of json objects, while quite interesting from an
academic sense, are very difficult for people to grok, and lead to
inconsistencies.

Remove brace initialization of json objects

Brace initialization of json objects, while quite interesting from an
academic sense, are very difficult for people to grok, and lead to
inconsistencies. This patchset aims to remove a majority of them in
lieu of operator[]. Interestingly, this saves about 1% of the binary
size of bmcweb.

This also has an added benefit that as a design pattern, we're never
constructing a new object, then moving it into place, we're always
adding to the existing object, which in the future _could_ make things
like OEM schemas or properties easier, as there's no case where we're
completely replacing the response object.

Tested:
Ran redfish service validator. No new failures.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Iae409b0a40ddd3ae6112cb2d52c6f6ab388595fe

show more ...

storage: Call function only if interfaces exist

Avoid making the dbus call if the interface does not exist and is
expected to error out to reduce the number of dbus calls.

Tested:
Redfish Validator

storage: Call function only if interfaces exist

Avoid making the dbus call if the interface does not exist and is
expected to error out to reduce the number of dbus calls.

Tested:
Redfish Validator passed for Drives
```
$ wget -qO- \
http://localhost:80/redfish/v1/Systems/system/Storage/1/Drives/drive
{
"@odata.id": "/redfish/v1/Systems/system/Storage/1/Drives/drive",
"@odata.type": "#Drive.v1_7_0.Drive",
"Id": "warthog",
"Links": {
"Chassis": {
"@odata.id": "/redfish/v1/Chassis/chassis"
}
},
"Manufacturer": "XXX",
"MediaType": "SSD",
"Model": "XXX",
"Name": "drive",
"PartNumber": "xxxxxxxxxxxxx",
"Protocol": "SATA",
"SerialNumber": "123456",
"Status": {
"Health": "OK",
"HealthRollup": "OK",
"State": "Enabled"
}
}
```

Change-Id: Ic83c8ee5a49f75b71d443781faf8b65d8fab31b6
Signed-off-by: Willy Tu <wltu@google.com>

show more ...

Add setUpRedfishRoute to all nodes in redfish

For better or worse, the series ahead of this is making use of
setUpRedfishRoute to do the common "redfish specified" things that need
to be done for a

Add setUpRedfishRoute to all nodes in redfish

For better or worse, the series ahead of this is making use of
setUpRedfishRoute to do the common "redfish specified" things that need
to be done for a connection, like header checking, filtering, and other
things. In the current model, where BMCWEB_ROUTE is a common function
for all HTTP routes, this means we need to propagate this injection call
into the whole tree ahead of the requests being handled.

In a perfect world, we would invent something like a REDFISH_ROUTE
macro, but because macros are discouraged, the routes take a variadic
template of parameters, and each call to the route has a .privileges()
call in the middle, there's no good way to effect this change in a less
costly manner. This was messaged both in the prior reviews, and on
discord sourcing improvements on this pattern, to which none arose.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Id29cc799e214edad41e48fc7ce6eed0521f90ecb

show more ...

Consitently use dbus::utility types

This saves about 4k on the binary size

Tested: Redfish service validator passes.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I9546227a19c691b1aecb

Consitently use dbus::utility types

This saves about 4k on the binary size

Tested: Redfish service validator passes.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I9546227a19c691b1aecb80e80307889548c0293f

show more ...

Drive: Add MediaType, Capacity, and Protocol Resource for Drive

Populate the MediaType with DriveType to identify the Drive.

- https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-dbus-interface

Drive: Add MediaType, Capacity, and Protocol Resource for Drive

Populate the MediaType with DriveType to identify the Drive.

- https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-dbus-interfaces/+/44971

Populate the Capacity as the size in bytes of the Drive.
- https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-dbus-interfaces/+/46332

Populate the Protocol with DriveProtocol for the communication protocol
types
- https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-dbus-interfaces/+/46333

Tested:

Passed Redfish Validator with no new error.
```
*** /redfish/v1/Systems/system/Storage/storage0/Drives/drive0
Type (#Drive.v1_7_0.Drive), GET SUCCESS (time: 0.307086)
PASS
```

```
{
"@odata.id": "/redfish/v1/Systems/system/Storage/storage0/Drives/drive0",
"@odata.type": "#Drive.v1_7_0.Drive",
"CapacityBytes": 250059350016,
"Id": "drive0",
"Manufacturer": "",
"MediaType": "SSD",
"Name": "drive0",
"PartNumber": "",
"Protocol": "SATA",
"SerialNumber": "0",
"Status": {
"Health": "OK",
"HealthRollup": "OK",
"State": "Enabled"
}
}

```

Change-Id: I533eade92ec461b957f0c13eb69d06fead8b10f3
Signed-off-by: Willy Tu <wltu@google.com>

show more ...

Enable readability-container-size-empty tests

This one is a little trivial, but it does help in readability.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I5366d4eec8af2f781b3bad804131a

Enable readability-container-size-empty tests

This one is a little trivial, but it does help in readability.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I5366d4eec8af2f781b3bad804131ae2eb806e3aa

show more ...

Using sdbusplus::asio::getProperty

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/o

Using sdbusplus::asio::getProperty

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>

show more ...

Move to common variant

This saves approximately 34kB in the compressed binary size of bmcweb
due to reduced template instantiations. This amounts to a 2.5%
reduction in the overall size.

Note, the

Move to common variant

This saves approximately 34kB in the compressed binary size of bmcweb
due to reduced template instantiations. This amounts to a 2.5%
reduction in the overall size.

Note, there were a few places where we broke const-correctness in the
form of pulling a non-const reference out of a const variant. This
new variant now requires const correctness, so some consts are
added where required.

Tested: Code compiles.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I6a60c8881c1268627eedb4ffddf16689dc5f6ed2

show more ...

Update clang-format

refer: https://github.com/openbmc/docs/blob/master/style/cpp/.clang-format
`Don't break long string literals`

Tested: built bmcweb successfully and RedfishValidator Passed.

Sig

Update clang-format

refer: https://github.com/openbmc/docs/blob/master/style/cpp/.clang-format
`Don't break long string literals`

Tested: built bmcweb successfully and RedfishValidator Passed.

Signed-off-by: George Liu <liuxiwei@inspur.com>
Change-Id: Ib58f7c942fd3838592e043c57e0b6ffcdc3d963b

show more ...

Drive: Refactor Drive property request functions

Refactor the drive resource and reorganized the code.

Tested:

Passed Redfish Validator with no new error.
```
*** /redfish/v1/Systems/system/Storag

Drive: Refactor Drive property request functions

Refactor the drive resource and reorganized the code.

Tested:

Passed Redfish Validator with no new error.
```
*** /redfish/v1/Systems/system/Storage/storage0/Drives/drive0
Type (#Drive.v1_7_0.Drive), GET SUCCESS (time: 0.307086)
PASS
```

```
{
"@odata.id": "/redfish/v1/Systems/system/Storage/storage0/Drives/drive0",
"@odata.type": "#Drive.v1_7_0.Drive",
"Id": "drive0",
"Name": "drive0",
"Status": {
"Health": "OK",
"HealthRollup": "OK",
"State": "Enabled"
}
}
```

Change-Id: Iceba90b39bd2d7a423c7fae03760b81a8e010606
Signed-off-by: Willy Tu <wltu@google.com>

show more ...

Automate PrivilegeRegistry to code

This commit attempts to automate the creation of our privileges
structures from the redfish privilege registry. It accomplishes this by
updating parse_registries.

Automate PrivilegeRegistry to code

This commit attempts to automate the creation of our privileges
structures from the redfish privilege registry. It accomplishes this by
updating parse_registries.py to also pull down the privilege registry
from DMTF.
The script then generates privilege_registry.hpp, which include const
defines for all the privilege registry entries in the same format that
the Privileges struct accepts. This allows new clients to simply
reference the variable to these privilege structures, instead of having
to manually (ie error pronely) put the privileges in themselves.

This commit updates all the routes.

For the moment, override and OEM schemas are not considered. Today we
don't have any OEM-specific Redfish routes, so the existing ones inherit
their parents schema. Overrides have other issues, and are already
incorrect as Redfish defines them.

Binary size remains unchanged after this patchset.

Tested:
Ran redfish service validator

Ran test case from f9a6708c4c6490257e2eb6a8c04458f500902476 to ensure
that the new privileges constructor didn't cause us to regress the brace
construction initializer.

Checked binary size with:
gzip -c
$BBPATH/tmp/work/s7106-openbmc-linux-gnueabi/obmc-phosphor-image/1.0-r0/rootfs/usr/bin/bmcweb
| wc -c
1244048

(tested on previous patchset)

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ideede3d5b39d50bffe7fe78a0848bdbc22ac387f

show more ...

Remove ambiguous privileges constructor

There are a number of endpoints that assume that a given routes
privileges are governed by a single set of privileges, instead of
multiple sets ORed together.

Remove ambiguous privileges constructor

There are a number of endpoints that assume that a given routes
privileges are governed by a single set of privileges, instead of
multiple sets ORed together. To handle this, there were two overloads
of the privileges() method, one that took a vector of Privileges, and
one that took an initializer_list of const char*. Unfortunately, this
leads some code in AccountService to pick the wrong overload when it's
called like this
.privileges( {{"ConfigureUsers"}, {"ConfigureManager"},
{"ConfigureSelf"}})

This is supposed to be "User must have ConfigureUsers, or
ConfigureManager, or ConfigureSelf". Currently, because it selects the
wrong overload, it computes to "User must have ConfigureUsers AND
ConfigureManager AND ConfigureSelf.

The double braces are supposed to cause this to form a vector of
Privileges, but it appears that the initializer list gets consumed, and
the single invocation of initializer list is called. Interestingly,
trying to put in a privileges overload of
intializer_list<initializer_list<const char*>> causes the compilation to
fail with an ambiguous call error, which is what I would've expected to
see previously in this case, but alas, I'm only a novice when it comes
to how the C++ standard works in these edge cases. This is likely due
in part to the fact that they were templates of an unused template param
(seemingly copied from the previous method) and SFINAE rules around
templates.

This commit functionally removes one of the privileges overloads, and
adds a second set of braces to every privileges call that previously had
a single set of braces. Previous code will not compile now, which is
IMO a good thing.

This likely popped up in the Node class removal, because the Node class
explicitly constructs a vector of Privilege objects, ensuing it can hit
the right overload

Tested:
Ran Redfish service validator

Tested the specific use case outlined on discord with:
Creating a new user with operator privilege:
```
redfishtool -S Always -u root -p 0penBmc -vvvvvvvvv -r 192.168.7.2
AccountService adduser foo mysuperPass1 Operator
```

Then attempting to list accounts:
```
curl -vvvv --insecure --user foo:mysuperPass1
https://192.168.7.2/redfish/v1/AccountService/Accounts/foo
```

Which succeeded and returned the account in question.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I83e62b70e97f56dc57d43b9081f333a02fe85495

show more ...

Remove Redfish Node class

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 a

Remove Redfish Node class

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

show more ...

Redfish : Return after InternalError instead of Continue

"After setting response to internal error you should stop adding more content to response. Try to return instead of continuing a loop"

https

Redfish : Return after InternalError instead of Continue

"After setting response to internal error you should stop adding more content to response. Try to return instead of continuing a loop"

https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/39970/9/redfish-core/lib/pcie_slots.hpp#169

Signed-off-by: Chicago Duan <duanzhijia01@inspur.com>
Change-Id: Iadd3062ba7fef31ba61eea1e79eb3a903716b9e9

show more ...

Using AsyncResp everywhere

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 a

Using AsyncResp everywhere

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

show more ...

Turn on ALL perf checks

1st, alphabetize the tidy-list for good housekeeping.

Next, enable all the clang-tidy performance checks, and resolve all the
issues. most of the issues boil down to:
1. Us

Turn on ALL perf checks

1st, alphabetize the tidy-list for good housekeeping.

Next, enable all the clang-tidy performance checks, and resolve all the
issues. most of the issues boil down to:
1. Using std::move on const variables. This does nothing.
2. Passing big variables (like std::string) by value.
3. Using double quotes on a find call, which constructs an intermediate
string, rather than using the character overload.

Tested
Loaded on system, logged in successfully and pulled down webui-vue. No
new errors.

Walked the Redfish tree a bit, and observed no new problems.

Ran redfish service validator. Got no new failures (although there are
a lot of log service deprecation warnings that we should look at).

Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I2238958c4b22c1e554e09a0a1787c744bdbca43e

show more ...

Lots of performance improvements

(In the voice of the kid from sixth sense) I see string copies...

Apparently there are a lot of places we make unnecessary copies. This
fixes all of them.

Not sure

Lots of performance improvements

(In the voice of the kid from sixth sense) I see string copies...

Apparently there are a lot of places we make unnecessary copies. This
fixes all of them.

Not sure how to split this up into smaller patches, or if it even needs
split up. It seems pretty easy to review to me, because basically every
diff is identical.

Change-Id: I22b4ae4f96f7e4082d2bc701098a04f7bed95369
Signed-off-by: Ed Tanous <ed@tanous.net>
Signed-off-by: Wludzik, Jozef <jozef.wludzik@intel.com>

show more ...

Enable unused variable warnings and resolve

This commit enables the "unused variables" warning in clang. Throughout
this, it did point out several issues that would've been functional
bugs, so I th

Enable unused variable warnings and resolve

This commit enables the "unused variables" warning in clang. Throughout
this, it did point out several issues that would've been functional
bugs, so I think it was worthwhile. It also cleaned up several unused
variable from old constructs that no longer exist.

Tested:
Built with clang. Code no longer emits warnings.

Downloaded bmcweb to system and pulled up the webui, observed webui
loads and logs in properly.

Change-Id: I51505f4222cc147d6f2b87b14d7e2ac4a74cafa8
Signed-off-by: Ed Tanous <ed@tanous.net>

show more ...

Enable clang warnings

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 variabl

Enable clang warnings

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

show more ...

Remove middlewares

Middlewares, while kinda cool from an academic standpoint, make our
build times even worse than they already are. Given that we only really
use 1 real middleware today (token aut

Remove middlewares

Middlewares, while kinda cool from an academic standpoint, make our
build times even worse than they already are. Given that we only really
use 1 real middleware today (token auth) and it needs to move into the
parser mode anyway (for security limiting buffer sizes), we might as well
use this as an opportunity to delete some code.

Some other things that happen:
1. Persistent data now moves out of the crow namespace
2. App is no longer a template
3. All request_routes implementations no longer become templates. This
should be a decent (unmeasured) win on compile times.

This commit was part of a commit previously called "various cleanups".
This separates ONLY the middleware deletion part of that.

Note, this also deletes about 400 lines of hard to understand code.

Change-Id: I4c19e25491a153a2aa2e4ef46fc797bcb5b3581a
Signed-off-by: Ed Tanous <ed@tanous.net>

show more ...

clang-format: update to latest from docs repo

This is from openbmc/docs/style/cpp/.clang-format

Other OpenBMC repos are doing the same.

Tested: Built and validator passed.
Change-Id: Ief26c755c9ce

clang-format: update to latest from docs repo

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>

show more ...

Redfish: Allow slash at the end of Resource

This is defined in the Redfish protocol.
Easiest way to allow this is to end the Node URL with "/", which most
Nodes in bmcweb already had.

Before:
curl

Redfish: Allow slash at the end of Resource

This is defined in the Redfish protocol.
Easiest way to allow this is to end the Node URL with "/", which most
Nodes in bmcweb already had.

Before:
curl -k https://${bmc}/redfish/v1/TaskService/
Not Found

After both /redfish/v1/TaskService/ and /redfish/v1/TaskService
return the Task Service.

Tested:
Validator passed.

Change-Id: Ic806dc5c91f631b87642e49b486a6b6da7fdf955
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>

show more ...