sdbusplus reading InterfacesAdded issue: not all variants are created equal
patrick at stwcx.xyz
Fri Dec 24 03:24:30 AEDT 2021
On Thu, Dec 23, 2021 at 06:45:26PM +0300, Paul Fertser wrote:
> While digging into current state of SNMP notifications (traps) support
> in OpenBMC I found some code I have no idea how to properly improve.
> phosphor-dbus-monitor has a handler that's meant to be called whenever
> a new log entry is created by monitoring InterfacesAdded signals on
> D-Bus logger paths. The processing is at
> It seems OK until you look into PathInterfacesAdded definition which
> includes a hard-coded std::variant<>:
> This already raises suspicions and rightfully so: the interface we're
> specifically interested in, xyz.openbmc_project.Logging.Entry,
> includes AdditionalData property which should be of type
> std::vector<std::string> , but that's not in the list of the allowed
> hardcoded variants.
> If I'm trying to use the std::variant<> type suitable for
> Logging.Entry then msg.read() fails with InvalidEnum error, probably
> trying to parse data about other interfaces, and this is a bad idea
> So what is the correct method of using statically-typed sdbusplus APIs
> to parse such a "dynamic" reply?
The short answer is that there isn't currently. The code doesn't support either
something like 'std::any' or something like 'std::placeholders' to skip past
parsing of an entry.
Generally, you know the type of the signal you're trying to listen to, so this
wasn't very important. There are a few cases where the code interacting with
dbus is "generic" and it becomes important. I think in this case you're
expecting to be able to match against configuration in JSON? So we probably
need to do something to make the situation better.
In phosphor-inventory-manager they've had a similar problem and what we've ended
up having to do is extend the variant list for each new element type they want
to accept. I think they wrote a python script that generates it from the
phosphor-dbus-interfaces YAML information at build time.
There is something a little better than that available though. If you look at
the generated headers for phosphor-dbus-interface, you'll see that each class
has a PropertiesVariant. Here is the one for Logging.Entry:
using PropertiesVariant = sdbusplus::utility::dedup_variant_t<
`dedup_variant_t` is a class that evaluates to a std::variant, but makes sure
that each type is only listed once. You could pretty easily add a
`merge_variant` on top of this that would be the union of all the variant types.
Then, the type you'd pass into `msg.read` would be:
using Value =
Where we'd list all the interfaces we expect phosphor-dbus-monitor to be able to
You mentioned the InvalidEnum error. I'd have to see what you were doing there
but if you both an Enum-type and a std::string in the variant it should have
parsed as a std::string if the Enum-matching was unsuccessful and not thrown an
error. I thought I had a test case for that exact scenario as well. If you are
seeing that, it sounds like a bug.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 833 bytes
Desc: not available
More information about the openbmc