| Message ID | 20210831090459.2306727-8-vishal.l.verma@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Policy based reconfiguration for daxctl | expand |
> Subject: [ndctl PATCH 7/7] daxctl: add systemd service and udev rule for > auto-onlining > > 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: > > daxdev-reconfigure@-dev-dax0.0.service > > 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 daxdev-reconfigure@-dev-dax0.0.service > > Update the RPM spec file to include the newly added files to the RPM > build. > > Cc: QI Fuli <qi.fuli@fujitsu.com> > Cc: Dan Williams <dan.j.williams@intel.com> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > --- > configure.ac | 9 ++++++++- > daxctl/90-daxctl-device.rules | 1 + > daxctl/Makefile.am | 10 ++++++++++ > daxctl/daxdev-auto-reconfigure.sh | 3 +++ > daxctl/daxdev-reconfigure@.service | 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/daxdev-reconfigure@.service > > 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) Hi Vishal, Thank you for the patch. I got some warnings in my test. $ ./autogen.sh daxctl/Makefile.am:33: warning: CLEANFILES multiply defined in condition TRUE ... Makefile.am.in:2: ... 'CLEANFILES' previously defined here daxctl/Makefile.am:1: 'Makefile.am.in' included from here ---------------------------------------------------------------- Initialized build system. For a common configuration please run: ---------------------------------------------------------------- ./configure CFLAGS='-g -O2' --prefix=/usr --sysconfdir=/etc --libdir=/usr/lib64 QI > + > +udevrulesdir = $(UDEVRULESDIR) > +udevrules_DATA = 90-daxctl-device.rules > + > +if ENABLE_SYSTEMD_UNITS > +systemd_unit_DATA = daxdev-reconfigure@.service > +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/daxdev-reconfigure@.service > b/daxctl/daxdev-reconfigure@.service > new file mode 100644 > index 0000000..451fef1 > --- /dev/null > +++ b/daxctl/daxdev-reconfigure@.service > @@ -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" > diff --git a/ndctl.spec.in b/ndctl.spec.in > index 07c36ec..fd1a5ff 100644 > --- a/ndctl.spec.in > +++ b/ndctl.spec.in > @@ -124,8 +124,11 @@ make check > %defattr(-,root,root) > %license LICENSES/preferred/GPL-2.0 LICENSES/other/MIT > LICENSES/other/CC0-1.0 > %{_bindir}/daxctl > +%{_bindir}/daxdev-auto-reconfigure.sh > %{_mandir}/man1/daxctl* > %{_datadir}/daxctl/daxctl.conf > +%{_unitdir}/daxdev-reconfigure@.service > +%config %{_udevrulesdir}/90-daxctl-device.rules > > %files -n LNAME > %defattr(-,root,root) > -- > 2.31.1
On Tue, Aug 31, 2021 at 2:05 AM Vishal Verma <vishal.l.verma@intel.com> 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: > > daxdev-reconfigure@-dev-dax0.0.service 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 daxdev-reconfigure@-dev-dax0.0.service 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 <qi.fuli@fujitsu.com> > Cc: Dan Williams <dan.j.williams@intel.com> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > --- > configure.ac | 9 ++++++++- > daxctl/90-daxctl-device.rules | 1 + > daxctl/Makefile.am | 10 ++++++++++ > daxctl/daxdev-auto-reconfigure.sh | 3 +++ > daxctl/daxdev-reconfigure@.service | 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/daxdev-reconfigure@.service > > 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 = daxdev-reconfigure@.service > +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/daxdev-reconfigure@.service b/daxctl/daxdev-reconfigure@.service > new file mode 100644 > index 0000000..451fef1 > --- /dev/null > +++ b/daxctl/daxdev-reconfigure@.service > @@ -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.
On Fri, 2021-09-17 at 11:10 -0700, Dan Williams wrote: > On Tue, Aug 31, 2021 at 2:05 AM Vishal Verma <vishal.l.verma@intel.com> 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: > > > > daxdev-reconfigure@-dev-dax0.0.service > > 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 daxdev-reconfigure@-dev-dax0.0.service > > 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 <qi.fuli@fujitsu.com> > > Cc: Dan Williams <dan.j.williams@intel.com> > > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > > --- > > configure.ac | 9 ++++++++- > > daxctl/90-daxctl-device.rules | 1 + > > daxctl/Makefile.am | 10 ++++++++++ > > daxctl/daxdev-auto-reconfigure.sh | 3 +++ > > daxctl/daxdev-reconfigure@.service | 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/daxdev-reconfigure@.service > > > > 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 = daxdev-reconfigure@.service > > +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/daxdev-reconfigure@.service b/daxctl/daxdev-reconfigure@.service > > new file mode 100644 > > index 0000000..451fef1 > > --- /dev/null > > +++ b/daxctl/daxdev-reconfigure@.service > > @@ -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?
On Wed, Nov 17, 2021 at 3:29 PM Verma, Vishal L <vishal.l.verma@intel.com> wrote: > > On Fri, 2021-09-17 at 11:10 -0700, Dan Williams wrote: > > On Tue, Aug 31, 2021 at 2:05 AM Vishal Verma <vishal.l.verma@intel.com> 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: > > > > > > daxdev-reconfigure@-dev-dax0.0.service > > > > 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 daxdev-reconfigure@-dev-dax0.0.service > > > > 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 <qi.fuli@fujitsu.com> > > > Cc: Dan Williams <dan.j.williams@intel.com> > > > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > > > --- > > > configure.ac | 9 ++++++++- > > > daxctl/90-daxctl-device.rules | 1 + > > > daxctl/Makefile.am | 10 ++++++++++ > > > daxctl/daxdev-auto-reconfigure.sh | 3 +++ > > > daxctl/daxdev-reconfigure@.service | 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/daxdev-reconfigure@.service > > > > > > 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 = daxdev-reconfigure@.service > > > +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/daxdev-reconfigure@.service b/daxctl/daxdev-reconfigure@.service > > > new file mode 100644 > > > index 0000000..451fef1 > > > --- /dev/null > > > +++ b/daxctl/daxdev-reconfigure@.service > > > @@ -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.
On Fri, 2021-09-17 at 11:10 -0700, Dan Williams wrote: > On Tue, Aug 31, 2021 at 2:05 AM Vishal Verma <vishal.l.verma@intel.com> 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: > > > > daxdev-reconfigure@-dev-dax0.0.service > > 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? I don't know of a convention - but 'what it does' seemed more natural for a service than 'when it's called'. It also correlates better with usual system service names (i.e. they are named after what they do). > > 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? Systemd black magic? the dax0.0 doesn't come from anything I configure - that's just how systemd's 'instantiated services' work. Each newly added device gets it's own service tied to a unique identifier for the device. For these, it happens to be /dev/dax0.0, which gets escaped to -dev-dax0.0. More reading here: http://0pointer.de/blog/projects/instances.html > > > > > 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 daxdev-reconfigure@-dev-dax0.0.service > > 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. There will be both a log and a service per device - the unit file we supply/install is essentially a template for these instantiated services, but the actual service kicked off is <foo>@<device- id>.service > > > > > Update the RPM spec file to include the newly added files to the RPM > > build. > > > > Cc: QI Fuli <qi.fuli@fujitsu.com> > > Cc: Dan Williams <dan.j.williams@intel.com> > > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > > --- > > configure.ac | 9 ++++++++- > > daxctl/90-daxctl-device.rules | 1 + > > daxctl/Makefile.am | 10 ++++++++++ > > daxctl/daxdev-auto-reconfigure.sh | 3 +++ > > daxctl/daxdev-reconfigure@.service | 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/daxdev-reconfigure@.service > > > > 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 = daxdev-reconfigure@.service > > +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/daxdev-reconfigure@.service b/daxctl/daxdev-reconfigure@.service > > new file mode 100644 > > index 0000000..451fef1 > > --- /dev/null > > +++ b/daxctl/daxdev-reconfigure@.service > > @@ -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.
On Wed, Nov 17, 2021 at 6:40 PM Verma, Vishal L <vishal.l.verma@intel.com> wrote: > > On Fri, 2021-09-17 at 11:10 -0700, Dan Williams wrote: > > On Tue, Aug 31, 2021 at 2:05 AM Vishal Verma <vishal.l.verma@intel.com> 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: > > > > > > daxdev-reconfigure@-dev-dax0.0.service > > > > 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? > > I don't know of a convention - but 'what it does' seemed more natural > for a service than 'when it's called'. It also correlates better with > usual system service names (i.e. they are named after what they do). > > > > > 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? > > Systemd black magic? the dax0.0 doesn't come from anything I configure > - that's just how systemd's 'instantiated services' work. Each newly > added device gets it's own service tied to a unique identifier for the > device. For these, it happens to be /dev/dax0.0, which gets escaped to > -dev-dax0.0. > > More reading here: > http://0pointer.de/blog/projects/instances.html > > > > > > > > > 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 daxdev-reconfigure@-dev-dax0.0.service > > > > 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. > > There will be both a log and a service per device - the unit file we > supply/install is essentially a template for these instantiated > services, but the actual service kicked off is <foo>@<device- > id>.service Ah, ok, that makes sense.
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 = daxdev-reconfigure@.service +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/daxdev-reconfigure@.service b/daxctl/daxdev-reconfigure@.service new file mode 100644 index 0000000..451fef1 --- /dev/null +++ b/daxctl/daxdev-reconfigure@.service @@ -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" diff --git a/ndctl.spec.in b/ndctl.spec.in index 07c36ec..fd1a5ff 100644 --- a/ndctl.spec.in +++ b/ndctl.spec.in @@ -124,8 +124,11 @@ make check %defattr(-,root,root) %license LICENSES/preferred/GPL-2.0 LICENSES/other/MIT LICENSES/other/CC0-1.0 %{_bindir}/daxctl +%{_bindir}/daxdev-auto-reconfigure.sh %{_mandir}/man1/daxctl* %{_datadir}/daxctl/daxctl.conf +%{_unitdir}/daxdev-reconfigure@.service +%config %{_udevrulesdir}/90-daxctl-device.rules %files -n LNAME %defattr(-,root,root)
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: daxdev-reconfigure@-dev-dax0.0.service 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 daxdev-reconfigure@-dev-dax0.0.service Update the RPM spec file to include the newly added files to the RPM build. Cc: QI Fuli <qi.fuli@fujitsu.com> Cc: Dan Williams <dan.j.williams@intel.com> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> --- configure.ac | 9 ++++++++- daxctl/90-daxctl-device.rules | 1 + daxctl/Makefile.am | 10 ++++++++++ daxctl/daxdev-auto-reconfigure.sh | 3 +++ daxctl/daxdev-reconfigure@.service | 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/daxdev-reconfigure@.service