phosphor-hwmon refactoring proposal

[Patrick Venture]

On Thu, Oct 17, 2019 at 6:16 AM Brad Bishop <bradleyb at fuzziesquirrel.com> wrote:
>
> Hi everyone
>
> I have a quick phosphor-hwmon change proposal I’d like to get feedback on.
>
> One issue with phosphor-hwmon that has been brought up a couple times now
> is that it gets its configuration settings from a file with a path in the
> filesystem that mirrors the path to the hwmon device in the device tree.
> This is problematic because the device tree path is not required to be
> stable, so whenever things move around there, the config files all have to
> be moved.
>
> Unfortunately there are > 200 config files like this scattered throughout
> the openbmc source tree.  So my proposal addresses the limitation in a way
> that allows users to move over to the new config file locations on their
> own schedule.
>
> Presently phosphor-hwmon gets its configuration from the environment,
> provided by systemd’s EnvironmentFile option:
>
> EnvironmentFile=/etc/default/obmc/hwmon/%I.conf
> ExecStart=/usr/bin/phosphor-hwmon-readd -o %I
>
> The proposal is simply to quit passing the configuration via the
> environment and instead look for a config file, the location specified via
> logic in the application, with a path like:
>
> /usr/share/phosphor-hwmon/i2c/2-004c.conf
>
> This is the path to the hwmon parent device relative to /sys/bus e.g.
> /sys/bus/i2c/devices/2-004c/.  All the logic to do this would be added to
> the application itself, the sole input being the /sys/devices/... path
> provided by udev.  libsystemd has an sd-device subsystem that could be used
> to do the /sys/devices/… -> /sys/bus/… traversal if that makes things any
> easier.
>
> The new config file could keep the same format as today, or we could move
> it to json and parse it with nlohmann?  If json we could preserve the
> current flat configuration or have dictionaries?
>
> While we are poking around in here, I’d also recommend swapping out the
> current helper script with a SYSTEMD_WANTS as systemd has been updated to
> handle templates in udev rules since this was all originally implemented.
>
> Finally, to enabled a staged migration, the new unit file and udev rules
> would be installed with a new feature flag (e.g. meson option) in place of
> the old.
>
> Please poke holes.  thanks!

I'll let others focus on the hole poking.  I just want to say that
testing this userspace code will be easier if we transition away from
environment-based configuration.  So I'm all for that.

>
> -brad