Age | Commit message (Collapse) | Author | Files | Lines |
|
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>
|
|
25b54dba775b31021a3a4677eb79e9771bcb97f7 missed several cases where we
had ifndef instead of ifdef. because these weren't the defaults, these
don't show up as failures when testing.
Tested: Redfish service validator passes. Inspection primarily.
Mechanical change.
Change-Id: I3f6915a97eb44d071795aed76476c6bee7e8ed27
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
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>
|
|
Args captured by logging functions should be captured by rvalue, and use
std::forward to get perfect forwarding. In addition, separate out the
various std::out lines.
While we're here, also try to optimize a little. We should ideally be
writing each log line to the output once, and ideally not use iostreams,
which induce a lot of overhead.
Similar to spdlog[1] (which at one point this codebase used), construct
the string, then call fwrite and fflush once, rather than calling
std::cout repeatedly.
Now that we don't have a dependency on iostreams anymore, we can remove
it from the places where it has snuck in.
Tested:
Logging still functions as before. Logs present.
[1] https://github.com/gabime/spdlog/blob/27cb4c76708608465c413f6d0e6b8d99a4d84302/include/spdlog/sinks/stdout_sinks-inl.h#L70C7-L70C13
Change-Id: I1dd4739e06eb506d68989a066d122109b71b92cd
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>
|
|
This is an attempt to solve a class of use-after-move bugs on the
Request objects which have popped up several times. This more clearly
identifies code which owns the Request objects and has a need to keep it
alive. Currently it's just the `Connection` (or `HTTP2Connection`)
(which needs to access Request headers while sending the response), and
the `validatePrivilege()` function (which needs to temporarily own the
Request while doing an asynchronous D-Bus call). Route handlers are
provided a non-owning `Request&` for immediate use and required to not
hold the `Request&` for future use.
Tested: Redfish validator passes (with a few unrelated fails).
Redfish URLs are sent to a browser as HTML instead of raw JSON.
Change-Id: Id581fda90b6bceddd08a5dc7ff0a04b91e7394bf
Signed-off-by: Jonathan Doman <jonathan.doman@intel.com>
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
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>
|
|
25b54db introduced an inadvertent inversion in options. Admittedly,
this option (redfish-dbus-entries) chose to override URLs instead of
creating new ones, which makes it incompatible. Normally we'd require
unique URIs for each entry to make this error not possible, and I
believe that this is the only instance of us registering two handlers
for one url/verb.
Tested: Romulus now boots and functions in qemu. GET /redfish/v1
returns results.
Change-Id: I6125a5a0242b6cfc54ff19866665227c97f45d0a
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
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>
|
|
In the change made to move to std::format, we defined some custom type
formatters in logging.hpp. This had the unintended effect of making
all compile units pull in the majority of boost::url, and nlohmann::json
as includes.
This commit breaks out boost and json formatters into their own separate
includes.
Tested: Code compiles. Logging changes only.
Change-Id: I6a788533169f10e19130a1910cd3be0cc729b020
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>
|
|
Currently `update_schemas.py` generates a schema list definition like
redfish-core/include/schemas.hpp:
```
constexpr std::array schemas {
"AccountService",
"ActionInfo",
...
"OpenBMCAccountService",
};
```
However, if the number of schemas is more than the clang's default
max size, CI may fail.
The default is `-fbracket-depth=256`.
```
In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/functional:65:
[1m/usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/array:288:52: [0m [0;1;31mfatal error:
instantiating fold expression with 276 arguments exceeded expression nesting limit of 256
288 | -> array<enable_if_t<(is_same_v<_Tp, _Up> && ...), _Tp>, [0m
| [0;1;32m ~~~~~~~~~~~~~~~~~~~~~~~~^~~~
[0m [1m../redfish-core/include/schemas.hpp:17:26: [0m [0;1;30mnote:
while substituting deduced template arguments into function template '<deduction guide for array>' [with _Tp =
const char *, _Up = <const char *, const char *, const char *, const char *, const char *,
...
const char *>] [0m
17 | constexpr std::array schemas { [0m
| [0;1;32m ^
[0m1 error generated.
```
To avoid the failure, we can set the size explicitly like
```
constexpr std::array<std::string_view,277> schemas {
"AccountService",
...
```
Tested:
1) Remove `include_list` so that all possible schemas are to be used
2) Run with the fixed `scripts/update_schemas.py`
3) Compiles successfully
Change-Id: Ib68db9fd3be1b6dbe0c4b5cc0e9a4324966d759e
Signed-off-by: Myung Bae <myungbae@us.ibm.com>
|
|
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>
|
|
Change 1 line in scripts/parse_registries.py and rerun the script.
Long term OpenBMC/bmcweb need more direction here on the Privilege
Registry, but for now continue with the current direction of using the
Privilege Registry and taking it from the DMTF.
There is new entries in 1.4/1.5 that are needed for future development.
Tested: It builds.
Change-Id: I4337dc44e794c58f00f7307ea0508b84f14e8c8f
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
After splitting up the compile units, warnings start showing up on the
console about privileges. This is due to them being a static global,
and not shared between the compile units.
Move the array to a constexpr structure so that all compile units see
the same init.
Change-Id: I6e035342a5b229a0601c02092ca3ce665b14bbdb
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 used std::remove, which is a mechanism for removing characters
from strings. Clearly it meant std::filesystem::remove(), which removes
files from the filesystem.
Correct it.
Change-Id: I030966203c1682a11c723c596accdf34637dd1ba
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>
|
|
Stage::T is never set, so it can never happen. Remove it entirely.
Found using static analysis.
Tested: Unit tests pass, good coverage here
Change-Id: I0dfb1aad5bef3ab4451df5e81794e56074f6e939
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Static analysis tools complain that for certain template types, these
comparisons do nothing. Doing nothing is what they're intended to do,
as we have other checks.
Add some constexpr if hints so static analysis tools don't complain.
Change-Id: I60297d094626936d021382ccdc11e4c8b698e3d8
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>
|
|
This code accidentally makes a copy, given that getJsonObject returns a
std::optional<nlohmann::json::object_t> which is then loaded into a
std::optional<nlohmann::json>. Because nlohmann::json is implicitly
constructible from an object_t, this code works and compiles, but we
shouldn't need the intermediate object at all.
Change the code to simply load the value as object_t.
Tested: Unit tests pass. Good coverage.
Change-Id: Ic57953e66958e69a1233e18a5bbd980405cac58e
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Before Redfish put all messages in Base, but as the Messages became more
specific Redfish started creating new registries. Redfish might have
went a little registry happy. HeartbeatEvent just has 1 message and all
these new ones registries each just have a handful of messages.
Add the remaining 15 registries:
composition, environmental, ethernetfabric, fabric, heartbeat_event,
job_event, license, logservice, networkdevice, platform, power,
sensor_event, storage_device, telemetry, update.
Some of these are wanted for both current development and future
development but it is hard to decide which ones so just added them all.
power, fabric, telemetry, update are all things we support today. Having
a UpdateInProgress or UpdateSuccessful makes a lot of sense and this
enables that.
Put these alphabetically. Use a new for loop to do this.
Make changes to scripts/parse_registries.py and run the tool.
No difference in size.
Before: 66928640 Apr 10 13:32
obmc-phosphor-image-p10bmc-20240410183051.ext4.mmc.tar
After: 66928640 Apr 10 13:18
obmc-phosphor-image-p10bmc-20240410181439.ext4.mmc.tar
Tested: bmcweb builds.
"./scripts/parse_registries.py --registries license,update" works.
Change-Id: I43b4d041531cf338e9e7e621714ca7d95f6b01a5
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
Static analysis notes that the values in these functions are never
initialized, and that a small section of the branch is no longer
possible to hit, now that a default case has been added in
4da0490bc07a458ad3fc7d586c7eabf6053c572f
Remove the dead code and initialize variables where appropriate.
Tested: Unit tests pass. Decent coverage here.
Change-Id: I42ec4678672fea5b21f98aaae05dfca0629652e7
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>
|
|
Meson supports unity builds[1] natively. There's no reason to continue
with the pseudo unity build we've been using by putting implementations
in header files.
This commit is the first in a long series of starting to break this up
into smaller compile units, in the hopes of dropping incremental compile
times for developers, and reduce the total per-core memory usage that
gcc requires.
This commit breaks out the run() function from main() and the
constructor of RedfishService from redfish.hpp into their own compile
units. According to tracing, even after broken out, these are still by
far the two longest to compile units in the build.
Tested: Code compiles. Debug build on a 24 core build server results in
a decrease in compile time for compiling just bmcweb from 1m38s to
1m22s.
[1] https://mesonbuild.com/Unity-builds.html
Change-Id: Ibf352e8aba61d64c9a41a7a76e94ab3b5a0dde4b
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>
|