[ndctl PATCH] daxctl: fix systemd escaping for 90-daxctl-device.rules

[ndctl PATCH] daxctl: fix systemd escaping for 90-daxctl-device.rules

[Vishal Verma]

Older systemd was more tolerant of how unit names are passed in for
instantiated services via a udev rule, but of late, systemd flags
unescaped unit names, with an error such as:

  fedora systemd[1]: Invalid unit name "daxdev-reconfigure@/dev/dax0.0.service"
  escaped as "daxdev-reconfigure@-dev-dax0.0.service" (maybe you should use
  systemd-escape?).

Update the udev rule to pass the 'DEVNAME' from env through an
appropriate systemd-escape template so that it generates the correctly
escaped string.

Cc: Dan Williams <dan.j.williams@intel.com>
Reported-by: Chunhong Mao <chunhong.mao@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 daxctl/90-daxctl-device.rules | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/daxctl/90-daxctl-device.rules b/daxctl/90-daxctl-device.rules
index ee0670f..e02e7ec 100644
--- a/daxctl/90-daxctl-device.rules
+++ b/daxctl/90-daxctl-device.rules
@@ -1 +1,3 @@
-ACTION=="add", SUBSYSTEM=="dax", TAG+="systemd", ENV{SYSTEMD_WANTS}="daxdev-reconfigure@$env{DEVNAME}.service"
+ACTION=="add", SUBSYSTEM=="dax", TAG+="systemd",\
+  PROGRAM="/usr/bin/systemd-escape -p --template=daxdev-reconfigure@.service $env{DEVNAME}",\
+  ENV{SYSTEMD_WANTS}="%c"

base-commit: 97031db9300654260bc2afb45b3600ac01beaeba
-- 
2.35.1

Re: [ndctl PATCH] daxctl: fix systemd escaping for 90-daxctl-device.rules

[Dan Williams]

On Mon, Apr 18, 2022 at 11:54 AM Vishal Verma <vishal.l.verma@intel.com> wrote:
>
> Older systemd was more tolerant of how unit names are passed in for
> instantiated services via a udev rule, but of late, systemd flags
> unescaped unit names, with an error such as:
>
>   fedora systemd[1]: Invalid unit name "daxdev-reconfigure@/dev/dax0.0.service"
>   escaped as "daxdev-reconfigure@-dev-dax0.0.service" (maybe you should use
>   systemd-escape?).
>

Does systemd-escape exist on older systemd deployments? Is some new
systemd version detection or 'systemd-escape' detection needed in the
build configuration to select the format of 90-daxctl-device.rules?


> Update the udev rule to pass the 'DEVNAME' from env through an
> appropriate systemd-escape template so that it generates the correctly
> escaped string.
>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Reported-by: Chunhong Mao <chunhong.mao@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> ---
>  daxctl/90-daxctl-device.rules | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/daxctl/90-daxctl-device.rules b/daxctl/90-daxctl-device.rules
> index ee0670f..e02e7ec 100644
> --- a/daxctl/90-daxctl-device.rules
> +++ b/daxctl/90-daxctl-device.rules
> @@ -1 +1,3 @@
> -ACTION=="add", SUBSYSTEM=="dax", TAG+="systemd", ENV{SYSTEMD_WANTS}="daxdev-reconfigure@$env{DEVNAME}.service"
> +ACTION=="add", SUBSYSTEM=="dax", TAG+="systemd",\
> +  PROGRAM="/usr/bin/systemd-escape -p --template=daxdev-reconfigure@.service $env{DEVNAME}",\
> +  ENV{SYSTEMD_WANTS}="%c"
>
> base-commit: 97031db9300654260bc2afb45b3600ac01beaeba
> --
> 2.35.1
>

Re: [ndctl PATCH] daxctl: fix systemd escaping for 90-daxctl-device.rules

[Verma, Vishal L]

On Mon, 2022-04-18 at 12:15 -0700, Dan Williams wrote:
> On Mon, Apr 18, 2022 at 11:54 AM Vishal Verma
> <vishal.l.verma@intel.com> wrote:
> > 
> > Older systemd was more tolerant of how unit names are passed in for
> > instantiated services via a udev rule, but of late, systemd flags
> > unescaped unit names, with an error such as:
> > 
> >   fedora systemd[1]: Invalid unit name "daxdev-
> > reconfigure@/dev/dax0.0.service"
> >   escaped as "daxdev-reconfigure@-dev-dax0.0.service" (maybe you
> > should use
> >   systemd-escape?).
> > 
> 
> Does systemd-escape exist on older systemd deployments? Is some new
> systemd version detection or 'systemd-escape' detection needed in the
> build configuration to select the format of 90-daxctl-device.rules?

Good point - I think we're okay. systemd-escape was introduced in v216
back in 2014 [1], and from a quick glance at repology, even the oldest
distros are at least on v219 [2].

[1]: https://github.com/systemd/systemd/blob/main/NEWS#L10370
[2]: https://repology.org/project/systemd/versions

> 
> 
> > Update the udev rule to pass the 'DEVNAME' from env through an
> > appropriate systemd-escape template so that it generates the
> > correctly
> > escaped string.
> > 
> > Cc: Dan Williams <dan.j.williams@intel.com>
> > Reported-by: Chunhong Mao <chunhong.mao@intel.com>
> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> > ---
> >  daxctl/90-daxctl-device.rules | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/daxctl/90-daxctl-device.rules b/daxctl/90-daxctl-
> > device.rules
> > index ee0670f..e02e7ec 100644
> > --- a/daxctl/90-daxctl-device.rules
> > +++ b/daxctl/90-daxctl-device.rules
> > @@ -1 +1,3 @@
> > -ACTION=="add", SUBSYSTEM=="dax", TAG+="systemd",
> > ENV{SYSTEMD_WANTS}="daxdev-reconfigure@$env{DEVNAME}.service"
> > +ACTION=="add", SUBSYSTEM=="dax", TAG+="systemd",\
> > +  PROGRAM="/usr/bin/systemd-escape -p --template=daxdev-
> > reconfigure@.service $env{DEVNAME}",\
> > +  ENV{SYSTEMD_WANTS}="%c"
> > 
> > base-commit: 97031db9300654260bc2afb45b3600ac01beaeba
> > --
> > 2.35.1
> > 

Re: [ndctl PATCH] daxctl: fix systemd escaping for 90-daxctl-device.rules

[Dan Williams]

On Mon, Apr 18, 2022 at 12:48 PM Verma, Vishal L
<vishal.l.verma@intel.com> wrote:
>
> On Mon, 2022-04-18 at 12:15 -0700, Dan Williams wrote:
> > On Mon, Apr 18, 2022 at 11:54 AM Vishal Verma
> > <vishal.l.verma@intel.com> wrote:
> > >
> > > Older systemd was more tolerant of how unit names are passed in for
> > > instantiated services via a udev rule, but of late, systemd flags
> > > unescaped unit names, with an error such as:
> > >
> > >   fedora systemd[1]: Invalid unit name "daxdev-
> > > reconfigure@/dev/dax0.0.service"
> > >   escaped as "daxdev-reconfigure@-dev-dax0.0.service" (maybe you
> > > should use
> > >   systemd-escape?).
> > >
> >
> > Does systemd-escape exist on older systemd deployments? Is some new
> > systemd version detection or 'systemd-escape' detection needed in the
> > build configuration to select the format of 90-daxctl-device.rules?
>
> Good point - I think we're okay. systemd-escape was introduced in v216
> back in 2014 [1], and from a quick glance at repology, even the oldest
> distros are at least on v219 [2].
>
> [1]: https://github.com/systemd/systemd/blob/main/NEWS#L10370
> [2]: https://repology.org/project/systemd/versions

Ok, cool, looks good to me then:

Reviewed-by: Dan Williams <dan.j.williams@intel.com>

RE: [ndctl PATCH] daxctl: fix systemd escaping for 90-daxctl-device.rules

[Mao, Chunhong]

Hi Vishal,

Just to confirm that the patch has been Tested-by: Chunhong Mao <chunhong.mao@intel.com> on my system where the problem was reported initially and it does fix the problem, really appreciate your help!

Thanks,
Chunhong

-----Original Message-----
From: Verma, Vishal L <vishal.l.verma@intel.com> 
Sent: Monday, April 18, 2022 12:48 PM
To: Williams, Dan J <dan.j.williams@intel.com>
Cc: nvdimm@lists.linux.dev; Mao, Chunhong <chunhong.mao@intel.com>
Subject: Re: [ndctl PATCH] daxctl: fix systemd escaping for 90-daxctl-device.rules

On Mon, 2022-04-18 at 12:15 -0700, Dan Williams wrote:
> On Mon, Apr 18, 2022 at 11:54 AM Vishal Verma 
> <vishal.l.verma@intel.com> wrote:
> > 
> > Older systemd was more tolerant of how unit names are passed in for 
> > instantiated services via a udev rule, but of late, systemd flags 
> > unescaped unit names, with an error such as:
> > 
> >   fedora systemd[1]: Invalid unit name "daxdev- 
> > reconfigure@/dev/dax0.0.service"
> >   escaped as "daxdev-reconfigure@-dev-dax0.0.service" (maybe you 
> > should use
> >   systemd-escape?).
> > 
> 
> Does systemd-escape exist on older systemd deployments? Is some new 
> systemd version detection or 'systemd-escape' detection needed in the 
> build configuration to select the format of 90-daxctl-device.rules?

Good point - I think we're okay. systemd-escape was introduced in v216 back in 2014 [1], and from a quick glance at repology, even the oldest distros are at least on v219 [2].

[1]: https://github.com/systemd/systemd/blob/main/NEWS#L10370
[2]: https://repology.org/project/systemd/versions

> 
> 
> > Update the udev rule to pass the 'DEVNAME' from env through an 
> > appropriate systemd-escape template so that it generates the 
> > correctly escaped string.
> > 
> > Cc: Dan Williams <dan.j.williams@intel.com>
> > Reported-by: Chunhong Mao <chunhong.mao@intel.com>
> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> > ---
> >  daxctl/90-daxctl-device.rules | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/daxctl/90-daxctl-device.rules b/daxctl/90-daxctl- 
> > device.rules index ee0670f..e02e7ec 100644
> > --- a/daxctl/90-daxctl-device.rules
> > +++ b/daxctl/90-daxctl-device.rules
> > @@ -1 +1,3 @@
> > -ACTION=="add", SUBSYSTEM=="dax", TAG+="systemd", 
> > ENV{SYSTEMD_WANTS}="daxdev-reconfigure@$env{DEVNAME}.service"
> > +ACTION=="add", SUBSYSTEM=="dax", TAG+="systemd",\
> > +  PROGRAM="/usr/bin/systemd-escape -p --template=daxdev-
> > reconfigure@.service $env{DEVNAME}",\
> > +  ENV{SYSTEMD_WANTS}="%c"
> > 
> > base-commit: 97031db9300654260bc2afb45b3600ac01beaeba
> > --
> > 2.35.1
> >