| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
In the past, we've tried to erradicate the use of
nlohmann::json(initiatlizer_list<...>) because it bloats binary sizes,
as every type is given a new nlohmann constructor.
This commit hunts down the last few places where we call this. There is
still 2 remaining in openbmc_dbus_rest after this, but those are variant
accesses that are difficult to triage, and considering it's a less used
api, they're left as is.
Tested: WIP
Change-Id: Iaac24584bb78bb238da69010b511c1d598bd38bc
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Add the start update D-Bus interface based flow for multi-form content
path. This involves mapping the TargetURI to the corresponding
serviceName and objectPath which hosts the specific D-Bus interface.
As per discussion with Redfish community both ResourceURI and
FirmwareInventory Redfish URI can be used as TargetURI. Current
implementation already allows /redfish/v1/Managers/<bmc>, hence support
for this specific ResourceURI has been preserved. New implementation
adds FirmwareInventory Redfish URI for TargetURI as default option.
https://redfishforum.com/thread/1054.
For more details on design refer to -
https://gerrit.openbmc.org/c/openbmc/phosphor-dbus-interfaces/+/65738
https://gerrit.openbmc.org/c/openbmc/docs/+/65739
Tested: Redfish Validator and Build passes.
multipart form data update request with Resource URI as target
```
curl -k -H "X-Auth-Token: $token" -H "Content-Type:multipart/form-data" \
-X POST -F UpdateParameters="{\"Targets\":[\"/redfish/v1/Managers/bmc\"],\"@Redfish.OperationApplyTime\":\"Immediate\"};type=application/json" \
-F "UpdateFile=@obmc-phosphor-image-romulus-20240425222313.static.mtd.all.tar;type=application/octet-stream" \
https://${bmc}/redfish/v1/UpdateService/update
{
"@odata.id": "/redfish/v1/TaskService/Tasks/0",
"@odata.type": "#Task.v1_4_3.Task",
"Id": "0",
"TaskState": "Running",
"TaskStatus": "OK"
}
```
multipart form data update request with Firmware Inventory URI as target
```
curl -k -H "X-Auth-Token: $token" -H "Content-Type:multipart/form-data" \
-X POST -F UpdateParameters="{\"Targets\":[\"/redfish/v1/Managers/bmc\"],\"@Redfish.OperationApplyTime\":\"Immediate\"};type=application/json" \
-F "UpdateFile=@obmc-phosphor-image-romulus-20240509003505.static.mtd.all.tar;type=application/octet-stream" \
https://${bmc}/redfish/v1/UpdateService/update
{
"@odata.id": "/redfish/v1/TaskService/Tasks/1",
"@odata.type": "#Task.v1_4_3.Task",
"Id": "1",
"TaskState": "Running",
"TaskStatus": "OK"
}
```
Change-Id: Id46de79d3af8834630a793678a6fc0e859295afe
Signed-off-by: Jagpal Singh Gill <paligill@gmail.com>
|
|
In the initial implementation of metadata indexing the bmc knew at
compile time what schemas it could potentially publish. bmcweb took the
approach of adding all schemas of all versions to the $metadata
resource. Since that was made, two major changes have happened.
First, Redfish has added significantly more versions of each schema, as
well as significantly more schemas to the point where the metadata index
is now 213KB. While this file compresses fairly well, the size is
obvious from the large amount of time that redfish service validator
takes to parse the schemas, compared to actually acquiring BMC redfish
resources.
Second, aggregation was added, where an aggregated Redfish service might
implement any number of schemas, including OEM ones.
In an effort to fix this, this patch takes the compile-time algorithm in
update_schemas.py, and moves it into bmcweb itself, parsing the files on
disk as needed on demand. This has some immediate benefits; First, is
that now schemas can be potentially installed from anywhere, not only
from within the bmcweb build, and they will be resolved at runtime.
Second, patches that want to add support for a given schema need to only
symlink the schema into the correct folder, without needing to rerun
update_schemas.py. This saves time in review.
Finally, this opens to door to reducing the schema versions present in
the metadata to the unique set of only what this bmcweb instance, and
its aggregated BMCs expose.
Tested: Redfish service validator passes. Need A/B checking to verify
the file is byte for byte the same.
GET /redfish/v1/$metadata returns what looks like sane results, with a
correct content-type.
Unit tests require the use of TemporaryFileHandle, so that class is
moved into a more general folder, outside of test/http.
Change-Id: I326159099c6b6c4056023b2e173c5f074ed88ce1
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Removing the IPv4 default gateway doesn't work correctly when only a
single static address has been assigned. This is expected to be the
common mode of operation, and needs to work correctly.
When more than one static address is managed it's necessary to
preserve the existing gateway. If any address is left unmodified,
added, or is modified the gateway must be preserved.
Tested:
Turned off DHCPv4, and assigned a single static address
Sent a PATCH null to delete the address.
Confirmed the default gateway got cleared.
Assigned two static addresses.
Sent a PATCH {}, null
Sent PATCH null
Confirmed expected default gateway handling
Assigned two static addresses.
Sent a PATCH null, {}
Sent PATCH null
Confirmed expected default gateway handling
Change-Id: I85c4a0533f9468b424602aeb636b8f4f218a9a13
Signed-off-by: Johnathan Mantey <johnathanx.mantey@intel.com>
|
|
These includes seem to have snuck in. In theory nothing in redfish
should be taking a #include in anything in openbmc-rest.
Tested: Code compiles
Change-Id: Ifec2a9b18f296870f67b15f98fc44c67050e9e28
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
In the early days of bmcweb, we made two pretty critical assumptions;
First, is that a given platform would only have a single BMC instance
(represented as "bmc") and a single host instance (represented as
"system").
Second we assumed that, given that Redfish suggests against hardcoding
URIs in client implementation and leaves them freeform, clients would
code to the standard.
Our own webui-vue hardcodes Redfish URIs [1], and the documentation is
littered with examples of hardcoded curl examples of hardcoding these
URIs. That bug was filed in 2020, and the issue has only gotten worse
over time.
This patchset is an attempt to give a target that we can start solving
these issues, without trying to boil the ocean and fix all clients in
parallel.
This commit adds the meson options
redfish-manager-uri-name
and
redfish-system-uri-name
These are used to control the "name" that bmcweb places in the fixed
locations in the ManagerCollection and ComputerSystemCollection schemas.
Note, managers is added, but is not currently testable. It will be
iterated on over time.
Tested:
Changed the URL options to "edsbmc" and "edssystem" in meson options.
Redfish service validator passes.
URLs appear changed when walking the tree.
[1] https://github.com/openbmc/webui-vue/issues/43
Change-Id: I4b44685067051512bd065da8c2e3db68ae5ce23a
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
When a user's password is changed, existing Redfish sessions for that
user, created with the old password, continue to work.
As per OWASP session management, "The session ID must be renewed or
regenerated by the web application after any privilege level change
within the associated user session... Common scenarios to consider
include; password changes, permission changes, or switching from a
regular user role to an administrator role within the web application."
[1] https://cheatsheetseries.owasp.org/cheatsheets/Session_Management_Cheat_Sheet.html
This commit removes existing user sessions when the user's password is
changed. This commit leaves the current session in place though a new
removeSessionsByUsernameExceptSession().
This commit doesn't completely get us fully to what owasp says but is a
start.
Tested:
Create some users:
```
curl -k -v -X POST -H "Content-Type: application/json" \
https://$bmc/redfish/v1/AccountService/Accounts/ -d \
'{"UserName":"testadminuser","Password":"<password>","RoleId":"Administrator","Enabled":true}'
```
Using basic auth was able to update own password and another user's
password.
Using token auth, verified the current session did not get deleted but
other sessions from that user did.
```
curl -k -H "Content-Type: application/json" -X POST -D headers.txt \
https://${bmc}/redfish/v1/SessionService/Sessions -d \
'{"UserName":"testadminuser", "Password":"<password>"}'
```
```
curl -k -v -X PATCH -H "X-Auth-Token: $token" \
-H "Content-Type:application/json" \
https://$bmc/redfish/v1/AccountService/Accounts/testadminuser \
-d '{"Password":"<password>"}'
```
Verified when changing another user's password all sessions were
dropped.
Change-Id: I4de60b84964a6b29c021dc3a2bece9ed4bc09eac
Signed-off-by: Ravi Teja <raviteja28031990@gmail.com>
|
|
Refactor task creation into a separate function so it can be used from
different places in code. The new usage of this function will be from
start update interface based flow. More details refer to -
https://gerrit.openbmc.org/c/openbmc/phosphor-dbus-interfaces/+/65738
https://gerrit.openbmc.org/c/openbmc/docs/+/65739
Tested: Firmware update using curl.
Change-Id: I5e8a0ab98f49657178ee733fa4d34fbf40a7b1f3
Signed-off-by: Jagpal Singh Gill <paligill@gmail.com>
|
|
Refactor parsing multi-part forms into a different function for using it
in legacy vs start update flows. More more details refer to -
https://gerrit.openbmc.org/c/openbmc/phosphor-dbus-interfaces/+/65738
https://gerrit.openbmc.org/c/openbmc/docs/+/65739
Tested: Used curl to test firmware update.
Change-Id: I2ec1ec9f4ac04349a1fbd588664f2d51bae827ea
Signed-off-by: Jagpal Singh Gill <paligill@gmail.com>
|
|
Static analysis now shows this code as "dead", even though it's not.
This is a merge conflict that was handled wrong.
Tested: inspection only. Suspect TFTP will now work.
Change-Id: I51e52d62c51b251baf4c6ae74b100c1eda95603d
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
This is yet another step in parsing HTTP requests.
Tested:
'''
curl -vvvv -k --user "root:0penBmc" -H "Content-Type: application/json" \
-X POST https://192.168.7.2/redfish/v1/UpdateService/Actions/UpdateService.SimpleUpdate \
-d '{ \
"TransferProtocol":"TFTP", \
"ImageURI":"https://192.168.7.1/myfile.bin" \
}'
'''
Returns ActionParameterNotSupported
TransferProtocol: Omitted
ImageURI: https://192.168.7.1/myfile.bin
Returns ActionParameterNotSupported
TransferProtocol: Omitted
ImageURI: 192.168.7.1/myfile.bin
Returns ActionParameterValueTypeError
TransferProtocol: Bad
ImageURI: https:/192.168.7.1/myfile.bin
Returns: ActionParameterNotSupported
No changes to GET requests, so Redfish Service Validator not necessary.
Change-Id: Ibf4b69877031f3b8617412c06d40f2d0d0827ac3
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
The Redfish schema for creating static IPv4 addresses requires the IP
address, the netmask, and a gateway IP address. There's an issue
inherent with this method. A network interface is only permitted a
single IPv4 default gateway. If more than one IPv4 static address is
assigned to the NIC each entry is processed, and potentially
conflicting default gateways may be assigned. The last entry processed
assigns the IPv4 default gateway. This behavior will cause unexpected
results. It is necessary to prevent assigning mismatched default
gateway values.
The IPv4 address removal process requires additional work also. The
default gateway value is left in place even after the final static
IPv4 address is removed. It is necessary to perform an additional
action to clear the gateway address. Without explicit removal the
network is left in a condition that may prevent IP traffic from being
able to be sent from the BMC. This even in the event that the NIC is
actively being managed via DHCPv4.
Tested:
Disabled DHCPv4 on a secondary NIC (eth1)
Assigned a static IPv4 address.
Inspected the systemd-networkd config file in order to confirm the
Gateway entry is added. This is done to be explicitly sure the
network.config file has the Gateway entry.
Sent a Redfish PATCH command to delete the static IPv4 address.
Confirmed that the systemd-networkd config file no longer contained a
Gateway entry. This is done to be explicitly sure the network.config
file no longer contains the Gateway entry.
Created a PATCH containing multiple IPv4 static addresses all with
different Gateway values. Confirmed an error is returned when a
mismatch occurs in the Gateway values.
Assigned a new static address, and then restored DHCPv4.
Confirmed that the default gateway entry in the config file is removed.
Submitted a delete request for the remaining static IPv4 address that
is now orphaned by re-enabling DHCPv4. This removed the static IPv4
address.
Change-Id: Ia12cf2a38ba86266ce71dc28475b0d07b7e09ebc
Signed-off-by: Johnathan Mantey <johnathanx.mantey@intel.com>
|
|
The Redfish standard seems to have caught up with some of the OEM
schemas and features we already have, namely MutualTLS and Basic Auth
disablement.
This commit implements most of the GET parameters for which we already
have backends. ClientCertificate is pointed to the same resources as
TrustStore.
Tested: generate_auth_certificates.py succeeds, and shows a certificate
in ClientCertificate collection
Get AccountService, and ClientAuthentication/Certificates returns
expected values.
Redfish service validator passes.
Change-Id: If18e34e9dfa8f38293fceff288596811afd16d4a
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
25b54db introduced a bug where CrashDump was not looking at the correct
option. Was using BMCWEB_REDFISH_DUMP_LOG instead of the correct
BMCWEB_REDFISH_CPU_LOG.
This was caught in CI by a system that doesn't have CrashDump enabled
but was hitting:
1 failGet errors in /redfish/v1/Systems/system/LogServices/Crashdump
Tested: None. Visually inspected and this matches
redfish-core/src/redfish.cpp.
Change-Id: Ia6e72e5bbeaaa246fbbc5bcb2a525062e63d7d29
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
The configuration options that exist in bmcweb are an amalgimation of
CROW options, CMAKE options using #define, pre-bmcweb ifdef mechanisms
and meson options using a config file. This history has led to a lot of
different ways to configure code in the codebase itself, which has led
to problems, and issues in consistency.
ifdef options do no compile time checking of code not within the branch.
This is good when you have optional dependencies, but not great when
you're trying to ensure both options compile.
This commit moves all internal configuration options to:
1. A namespace called bmcweb
2. A naming scheme matching the meson option. hyphens are replaced with
underscores, and the option is uppercased. This consistent transform
allows matching up option keys with their code counterparts, without
naming changes.
3. All options are bool true = enabled, and any options with _ENABLED or
_DISABLED postfixes have those postfixes removed. (note, there are
still some options with disable in the name, those are left as-is)
4. All options are now constexpr booleans, without an explicit compare.
To accomplish this, unfortunately an option list in config/meson.build
is required, given that meson doesn't provide a way to dump all options,
as is a manual entry in bmcweb_config.h.in, in addition to the
meson_options. This obsoletes the map in the main meson.build, which
helps some of the complexity.
Now that we've done this, we have some rules that will be documented.
1. Runtime behavior changes should be added as a constexpr bool to
bmcweb_config.h
2. Options that require optionally pulling in a dependency shall use an
ifdef, defined in the primary meson.build. (note, there are no
options that currently meet this class, but it's included for
completeness.)
Note, that this consolidation means that at configure time, all options
are printed. This is a good thing and allows direct comparison of
configs in log files.
Tested: Code compiles
Server boots, and shows options configured in the default build. (HTTPS,
log level, etc)
Change-Id: I94e79a56bcdc01755036e4e7278c7e69e25809ce
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
The One chassis option has been gone for a long time, but this ifdef
looks like it got missed. Remove it.
Tested: code compiles.
Change-Id: I013e824806e72bc608ae4383ce4ba707641aeec6
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
As much as the two vm implementations SEEM different, the differences
largely lie in how we're getting the nbd proxy socket. One is relying
on launching a process (nbd-proxy), the other is getting the fd from
dbus. Given [1] exists and is in process, we need to have a plan for
getting these two VM implementations into one, once that patchset is
complete.
This commit: Splits the vm-websocket option into vm-websocket-provider,
providing two options, nbd-proxy, and virtual-media (the names of the
respective apps). To accomplish this, it moves the contents of
nbd-proxy into include/vm-websocket, so we can compare the similarities
and start consolidating.
The longer term intent is that the nbd-proxy option will be completely
removed, and the code deleted. This has the additional advantage that
we will no longer require the boost::process dependency, as all info
will be available on dbus.
As part of this, the nbd proxy websocket is also registered at /vm/0/0,
to be backward compatible with the old interfaces.
Tested: Code compiles. Need some help here.
[1] https://gerrit.openbmc.org/c/openbmc/jsnbd/+/49944
Change-Id: Iedbca169ea40d45a8775f843792b874a248bb594
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Use language from header for registries list instead of hardcoded 'en'.
Language from header is already being used in some parts of code[1],
but is hardcoded sometimes. This commit fixes inconsistency.
TESTED: current language in header with Redfish definition is now
consistently taken into account.
[1] https://gerrit.openbmc.org/c/openbmc/bmcweb/+/70741/1/redfish-core/lib/message_registries.hpp#b214
Change-Id: Ic5e8e5e76d171b1cb18953e5602f09132b222f3b
Signed-off-by: Alexander Paramonov <Sasha110397@gmail.com>
|
|
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 <ed@tanous.net>
|
|
This commit adds HTTPBasicAuth Get/Patch support
Tested By:
Redfish service validator passes.
```
curl -k --user "root:0penBmc" -H "Content-Type: application/json" -X PATCH -d '{"HTTPBasicAuth":"Enabled"}' https://192.168.7.2/redfish/v1/AccountService
```
Succeeds with various values.
Enabled: Basic auth succeeds.
Disabled: Basic auth no longer works. AccountService reports "Disabled"
For HTTPBasicAuth status.
Change-Id: Ic417bf3cd4135f05ab34c8613c7fbce953157b03
Signed-off-by: Ravi Teja <raviteja28031990@gmail.com>
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
As part of the previous patch tests, UpdateService shows this bug, where
on a multipart parser failure, the dbus match object gets instantiated,
and eventually fails. This leads to mediocre logging, and possibly
could leave update service in an undesirable state.
Fix the error by moving the conditional up.
Tested:
Filling a 16MB file with all zeros and sending it now no longer logs
that a monitor has been set up, and returns immediately instead of
waiting for timeout.
```
dd if=/dev/zero of=zeros-file bs=1048576 count=16 of=16mb.txt
curl -k --location POST https://192.168.7.2/redfish/v1/UpdateService/update -F 'UpdateParameters={"Targets":[]} ;type=application/json' -H "Expect:" --user "root:0penBmc" -F UpdateFile=@16mb.txt -v
```
Change-Id: I0962d15c624936b4fa199a675123702003dd697b
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Most of these were found by breaking every redfish class handler into
its own compile unit:
When that's done, these missing headers become compile errors. We
should just fix them.
In addition, this allows us to enable automatic header checking in
clang-tidy using misc-header-cleaner. Because the compiler can now
"see" all the defines, it no longer tries to remove headers that it
thinks are unused.
[1] https://github.com/openbmc/bmcweb/commit/4fdee9e39e9f03122ee16a6fb251a380681f56ac
Tested: Code compiles.
Change-Id: Ifa27ac4a512362b7ded7cc3068648dc4aea6ad7b
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
This function in the next patch will be used for more than just TFTP, so
rename it to match intent, and refactor to use non-TFTP specific types.
Tested: Rename only. Need help on TFTP setups if we need it.
Change-Id: Ifc7485aa60ec53407c38b3d1bec530bdacf50075
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
This refactor of code is in preparation for adding new SimpleUpdate
types. Separating out TFTP helps to keep code organized.
Tested: Need help here. TFTP isn't enabled a lot.
Change-Id: Ifbdd4b73bb0f9c31092d729d1ec3d3f395f680b8
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Similar to other refactorings we've been doing, make UpdateService call
into methods instead of inline lambdas.
Tested: Redfish service validator passes. Structural changes only.
Change-Id: I96b6db5e14fa0f7d357fb0faf63d0457b7963581
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
There's currently a problem with phosphor-timesyncd, where enabling NTP
doesn't immediately reflect in the system status on return[1]. To say
it another way, NTP is not enabled/disabled atomically, which leads to
the following problem.
// Disable NTP
PATCH /redfish/v1/Managers/bmc/NetworkProtocol
{"NTP":{"ProtocolEnabled": false}}
// Set the time manually
PATCH /redfish/v1/Managers/bmc {"DateTime": "<timestring"}
Doing this in rapid succession leads to a 500 error, which is obviously
a bug. In the prior commit, this error was changed to a
PropertyValueConflict error, which is still incorrect, but at least
informative of what's going on. REST APIs are intended to have CRUD
compliance. The response should not be returned until the value has
been accepted, and not doing that can lead to problems.
This commit changes the backend to use systemd directly, rather than
routing through phosphor-settings, to avoid this race.
Quite possibly resolves #264 but haven't tested that.
Tested: The above procedure succeeds.
[1] https://github.com/systemd/systemd/pull/11424
Change-Id: I19241e7677d9b6415aff79ac65c474ae71984417
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
ReadJsonObject isn't required for cases where we don't have a list of
structures, and ideally we should consolidate all fixed readJson calls
in one place (and not have multi-depth readJson calls).
This commit moves all the calls up, and consolidates all the LDAP patch
params into a single struct that can be moved between the layers, rather
than having the parameters individually.
Tested: Does LDAP work anymore? Could use some help if anyone has test
scripts, otherwise code compiles and this is inspection only, but
similar to other mechanical changes we've made recently
Change-Id: I77c0a8b97d4783fdca875c86d7dace122a0a55d7
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
NTPServers is our last usage of nlohmann::json in a readJson unpack.
The capability and unit tests are left in place for that type in case we
need them in the future, but for now, document them as deprecated.
Tested: Redfish service validator passes. Redfish protocol validator
passes most tests (1 known failure in SSE is unrelated to this change).
Change-Id: If4b2ea061a941cc23d47189af7ff453094dc7dca
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
This commit changes sdbusplus setProperty calls (in various files) to
setDbusProperty method in Redfish namespace that handles all DBus
errors in a consistent manner.
It also handles and translates additional DBus errors to Redfish
specific errors in dbus_utils file.
Tested By:
Not tested yet
Change-Id: If440774879413754f4c24f9b6572c3c9fa1fd033
Signed-off-by: Asmitha Karunanithi <asmitk01@in.ibm.com>
|
|
Several places that call *req.ioService were missing nullptr checks.
Add them, and fix the one case where it might not be filled in.
Tested: With HTTP2 enabled, the following command succeeds.
```
curl -k https://192.168.7.2/redfish/v1/UpdateService/update -F 'UpdateParameters={"Targets":["/redfish/v1/Managers/bmc"]} ;type=application/json' --user "root:0penBmc" -F UpdateFile=@/home/ed/bmcweb/16mb.txt -v -H "Expect:"
```
Change-Id: I81e7944c22f5922d461bf5d231086c7468a16e62
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
If we use string_view with std::from_chars, we can use begin() and end()
directly (because they return pointers) and not have to do silly things
like dereference end(), which, while works in practice, is technically
undefined behavior, and some static analyzers complain about it.
Tested: Unit tests pass against both old parsePostCode and new.
Change-Id: Icfdec3b81f4a9c9bed3599571a8bc8779f9bfb98
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Change-Id: Ie1a03bac54183b206bf27e37f1fed804601c8643
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Capturing by auto here causes a copy. Found using static analysis.
Change-Id: Ifbb08f9af0cd6eeec1e611c610e7adf53e17665c
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Change-Id: If511f1210cca7bd1da3a8c5152688487d3036e2f
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
This code path is subtle, but given that slotPresent is only set to true
if totalCores is incremented, there's no way to actually hit this
section of code.
Looking for input on if this is the right behavior.
Change-Id: Ie6dadd2c7a0ca6b8402148ddd9b8a369a4a38b2e
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Despite these objects being called "view" they are still relatively
large, as clang-tidy correctly flags, and we ignore.
Change all function uses to capture by:
const boost::urls::url_view_base&
Which is the base class of all boost URL types, and any class (url,
url_view, etc) is convertible to that base.
Change-Id: I8ee2ea3f4cfba38331303a7e4eb520a2b6f8ba92
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Session might not be initialized, and might be nullptr.
This line was accessing the session BEFORE the nullptr check. Move it
to after. Found using static analysis.
Change-Id: I966c642aee7c76a29c7d0d57d3b78f5f7bef7d62
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
This commit will migrate all the setProperty calls initiated by a
redfish"Action" to "setDbusProperty" method in Redfish namespace that
handles all DBuserrors in a consistent manner.
This method will determine if a setProperty is called during redfish
"Action" or just setting of a dbus property and internally call
appropriate methods that handles different set of errors.
All the Redfish action specific errors are defined in error_messages.hpp
file.
This specific change moves setProperty call in hypervisor_system.hpp and
covers errors in the mentioned file only.
Tested-By:
<Yet to test this usecase>
Change-Id: I3da48fbeabcdcf088c4481021232f08a44797c86
Signed-off-by: Asmitha Karunanithi <asmitk01@in.ibm.com>
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
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>
|
|
"inline static void func()"
Doesn't make sense when put in a header file. Find all instances, and
make them inline like we do everywhere else.
Tested: Code compiles.
Change-Id: I7da5821b1e372941680f82939627af39fdc2a4eb
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Changing
static bool getUniqueEntryID
to
inline bool getUniqueEntryID
Exposes the fact that there's some undefined behavior here, and unit
tests start failing, likely due to stack being reused where it
previously wasn't.
This commit cleans up the code to simplify it, and remove the problem.
Tested: Unit tests pass. Good coverage.
Change-Id: I5b9b8e8bb83c656560193e680d246c8513ed6c02
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
The redfish-health-populate option was scheduled to be removed in 1Q
2024. It is now 2Q, so remove the option. No upstream layers enabled it
and did not find a downstream layer that did either.
This was always limited to a few resources. Overall this design was only
half done. A future "HealthRollup" can be proposed.
Some discord discussion:
[1]: https://discord.com/channels/775381525260664832/855566794994221117/1110728560819327069
Commit disabling this (merged 10 months ago):
[2]: https://github.com/openbmc/bmcweb/commit/6f8273e49cffdd347c223b9538558edfb05e818a
Tested: Code compiles
Change-Id: I4d33c1e674ecdb0fd256df62f3795073454ae7a1
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
Today, patching ethernet ip address arrays can use several
styles.
IpAddresses: [{}, {value: value}, null]
All 3 of those elements are legal. Today, we unpack values like that
with nlohmann::json, then iterate and unpack further. This leads to
problems where:
IpAddresses: [{}, {value: value}, 1.0]
would have the same behavior as the prior, given that we check for
"is_object()" to determine the null state. This is messy at best, and
not typesafe at worst.
Changing this code to use the new class NullOr<> allows the readJson
parser to fail the second example.
Change-Id: Id91f48bb64271dd568041a7c0b1ad285b59d5674
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Use multiple level direct read.
Tested: Visual only. Need help if anyone wants to test.
Change-Id: I67c77bdd42a05a42f9cd1b40dc74517dceebdaad
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Use multiple level direct read.
Tested: Visual only. Need help if anyone wants to test.
Change-Id: Ifcf716a2ba93fd565bbf134d4132532e60e3b4f0
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
nlohmann::json::dump() is not an easy function to get the call
parameters correct on. We should limit the places we use it.
Luckily, both logging and redfish::messages support printing
json values directly. Use them where appropriate.
Tested: Error logging and out of range calls only of heavily used
messages and logging calls. Inspection only.
Change-Id: I57521d8791dd95250c93e8e3b2d4a959740ac713
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
This commit changes sdbusplus setProperty calls in
log_services.hpp file to "setDbusProperty" method in Redfish
namespace that handles all DBus errors in a consistent manner.
Change-Id: Icd9b0f0326c75a1421756d515408b303bdd738e3
Signed-off-by: Asmitha Karunanithi <asmitk01@in.ibm.com>
|
|
The redfish-enable-proccessor-memory-status option was scheduled to be
removed in 1Q 2024. It is now 2Q, so remove the option. No upstream
layers enabled it and I could not find a downstream layer that did
either.
Redfish deprecated the Processor/Memory Summary Status (state, health,
healthrollup) attributes.
Discussion on discord, when disabling:
[1]: https://discord.com/channels/775381525260664832/855566794994221117/1093939076710793296
Commit disabling this (merged 10 months ago):
[2]: https://github.com/openbmc/bmcweb/commit/5fd0aafb0f14fb3011970e8575647bb608688c7c
Tested: Code builds.
Change-Id: I539cd5f384633afa7badf1cecfc6c7a87062f672
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
readability-avoid-nested-conditional-operator
With one exception, we already pass this check. Update the log services
code to make it pass, and update it to use the generated enums.
Tested: Code inspection only.
Change-Id: Ic80a7382beb0f541de4916d7b51e42ed5d5dc542
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
This commit changes sdbusplus setProperty calls in
hypervisor_system.hpp file to "setDbusProperty" method in Redfish
namespace that handles all DBus errors in a consistent manner.
Change-Id: Iebca5eb4e28159d61cd4b13c0343b78efd0f1f39
Signed-off-by: Asmitha Karunanithi <asmitk01@in.ibm.com>
|
