On Wed, Nov 17, 2021 at 3:29 PM Verma, Vishal L <[email protected]> wrote: > > On Fri, 2021-09-17 at 11:10 -0700, Dan Williams wrote: > > On Tue, Aug 31, 2021 at 2:05 AM Vishal Verma <[email protected]> > > wrote: > > > > > > Install a helper script that calls daxctl-reconfigure-device with the > > > new 'check-config' option for a given device. This is meant to be called > > > via a systemd service. > > > > > > Install a systemd service that calls the above wrapper script with a > > > daxctl device passed in to it via the environment. > > > > > > Install a udev rule that is triggered for every daxctl device, and > > > triggers the above oneshot systemd service. > > > > > > Together, these three things work such that upon boot, whenever a daxctl > > > device is found, udev triggers a device-specific systemd service called, > > > for example: > > > > > > [email protected] > > > > I'm thinking the service would be called daxdev-add, because it is > > servicing KOBJ_ADD events, or is the convention to name the service > > after what it does vs what it services? > > > > Also, I'm curious why would "dax0.0" be in the service name, shouldn't > > this be scanning all dax devices and internally matching based on the > > config file? > > > > > > > > This initiates a daxctl-reconfigure-device with a config lookup for the > > > 'dax0.0' device. If the config has an '[auto-online <unique_id>]' > > > section, it uses the information in that to set the operating mode of > > > the device. > > > > > > If any device is in an unexpected status, 'journalctl' can be used to > > > view the reconfiguration log for that device, for example: > > > > > > journalctl --unit [email protected] > > > > There will be a log per-device, or only if there is a service per > > device? My assumption was that this service is firing off for all > > devices so you would need to filter the log by the device-name if you > > know it... or maybe I'm misunderstanding how this udev service works. > > > > > > > > Update the RPM spec file to include the newly added files to the RPM > > > build. > > > > > > Cc: QI Fuli <[email protected]> > > > Cc: Dan Williams <[email protected]> > > > Signed-off-by: Vishal Verma <[email protected]> > > > --- > > > configure.ac | 9 ++++++++- > > > daxctl/90-daxctl-device.rules | 1 + > > > daxctl/Makefile.am | 10 ++++++++++ > > > daxctl/daxdev-auto-reconfigure.sh | 3 +++ > > > daxctl/[email protected] | 8 ++++++++ > > > ndctl.spec.in | 3 +++ > > > 6 files changed, 33 insertions(+), 1 deletion(-) > > > create mode 100644 daxctl/90-daxctl-device.rules > > > create mode 100755 daxctl/daxdev-auto-reconfigure.sh > > > create mode 100644 daxctl/[email protected] > > > > > > diff --git a/configure.ac b/configure.ac > > > index 9e1c6db..df6ab10 100644 > > > --- a/configure.ac > > > +++ b/configure.ac > > > @@ -160,7 +160,7 @@ AC_CHECK_FUNCS([ \ > > > > > > AC_ARG_WITH([systemd], > > > AS_HELP_STRING([--with-systemd], > > > - [Enable systemd functionality (monitor). > > > @<:@default=yes@:>@]), > > > + [Enable systemd functionality. @<:@default=yes@:>@]), > > > [], [with_systemd=yes]) > > > > > > if test "x$with_systemd" = "xyes"; then > > > @@ -183,6 +183,13 @@ daxctl_modprobe_data=daxctl.conf > > > AC_SUBST([daxctl_modprobe_datadir]) > > > AC_SUBST([daxctl_modprobe_data]) > > > > > > +AC_ARG_WITH(udevrulesdir, > > > + [AS_HELP_STRING([--with-udevrulesdir=DIR], [udev rules.d > > > directory])], > > > + [UDEVRULESDIR="$withval"], > > > + [UDEVRULESDIR='${prefix}/lib/udev/rules.d'] > > > +) > > > +AC_SUBST(UDEVRULESDIR) > > > + > > > AC_ARG_WITH([keyutils], > > > AS_HELP_STRING([--with-keyutils], > > > [Enable keyutils functionality (security). > > > @<:@default=yes@:>@]), [], [with_keyutils=yes]) > > > diff --git a/daxctl/90-daxctl-device.rules b/daxctl/90-daxctl-device.rules > > > new file mode 100644 > > > index 0000000..ee0670f > > > --- /dev/null > > > +++ b/daxctl/90-daxctl-device.rules > > > @@ -0,0 +1 @@ > > > +ACTION=="add", SUBSYSTEM=="dax", TAG+="systemd", > > > ENV{SYSTEMD_WANTS}="daxdev-reconfigure@$env{DEVNAME}.service" > > > diff --git a/daxctl/Makefile.am b/daxctl/Makefile.am > > > index f30c485..d53bdcf 100644 > > > --- a/daxctl/Makefile.am > > > +++ b/daxctl/Makefile.am > > > @@ -28,3 +28,13 @@ daxctl_LDADD =\ > > > $(UUID_LIBS) \ > > > $(KMOD_LIBS) \ > > > $(JSON_LIBS) > > > + > > > +bin_SCRIPTS = daxdev-auto-reconfigure.sh > > > +CLEANFILES = $(bin_SCRIPTS) > > > + > > > +udevrulesdir = $(UDEVRULESDIR) > > > +udevrules_DATA = 90-daxctl-device.rules > > > + > > > +if ENABLE_SYSTEMD_UNITS > > > +systemd_unit_DATA = [email protected] > > > +endif > > > diff --git a/daxctl/daxdev-auto-reconfigure.sh > > > b/daxctl/daxdev-auto-reconfigure.sh > > > new file mode 100755 > > > index 0000000..f6da43f > > > --- /dev/null > > > +++ b/daxctl/daxdev-auto-reconfigure.sh > > > @@ -0,0 +1,3 @@ > > > +#!/bin/bash > > > + > > > +daxctl reconfigure-device --check-config "${1##*/}" > > > diff --git a/daxctl/[email protected] > > > b/daxctl/[email protected] > > > new file mode 100644 > > > index 0000000..451fef1 > > > --- /dev/null > > > +++ b/daxctl/[email protected] > > > @@ -0,0 +1,8 @@ > > > +[Unit] > > > +Description=Automatic daxctl device reconfiguration > > > +Documentation=man:daxctl-reconfigure-device(1) > > > + > > > +[Service] > > > +Type=forking > > > +GuessMainPID=false > > > +ExecStart=/bin/sh -c "exec daxdev-auto-reconfigure.sh %I" > > > > Why is the daxdev-auto-reconfigure.sh indirection needed? Can this not be: > > > > ExecStart=daxctl reconfigure-device -C %I > > > > ...if the format of %l is the issue I think it would be good for > > reconfigure-device to be tolerant of this format. > > Yeah it was the format of %I. I forget exactly what it was, I think it > contains maybe a full /dev/daxX.Y path? Since in the wrapper script I'm > clipping away everything upto the first '/'. Should we make > reconfigure-device understand /dev/daxX.Y?
Yeah, I think it follows the principle of least surprise if: daxctl reconfigure-device /dev/daxX.Y ...and: daxctl reconfigure-device daxX.Y ...behave the same.
