[Pkg-zfsonlinux-devel] Bug#842237: zfs-share.service tries to call /usr/sbin/rm

[Fabian Grünbichler]

On Thu, Oct 27, 2016 at 11:28:47AM +0200, Petter Reinholdtsen wrote:
> [Fabian Grünbichler]
> > The correct approach seems to be to patch the arc* script automake files
> > to use the sbindir macro (although that currently gets set to /sbin ,
> > not /usr/sbin , so maybe the scripts need some special casing here),
> > and not to change the bindir macro to point to an sbin directory.
> >
> > The quick and dirty fix is to hardcode /bin/rm in
> > zfs-share.service.in.
> 
> If I understand you correctly, you propose this change, which make sense
> to me if the tools should only be executable by root, independent of the
> rm problem at hand:
> 
> diff --git a/cmd/arc_summary/Makefile.am b/cmd/arc_summary/Makefile.am
> index 815af3b..defa4b2 100644
> --- a/cmd/arc_summary/Makefile.am
> +++ b/cmd/arc_summary/Makefile.am
> @@ -1 +1 @@
> -dist_bin_SCRIPTS = arc_summary.py
> +dist_sbin_SCRIPTS = arc_summary.py
> diff --git a/cmd/arcstat/Makefile.am b/cmd/arcstat/Makefile.am
> index 8987b24..71e4125 100644
> --- a/cmd/arcstat/Makefile.am
> +++ b/cmd/arcstat/Makefile.am
> @@ -1 +1 @@
> -dist_bin_SCRIPTS = arcstat.py
> +dist_sbin_SCRIPTS = arcstat.py
> diff --git a/cmd/dbufstat/Makefile.am b/cmd/dbufstat/Makefile.am
> index 19bffb0..1a4b4ef 100644
> --- a/cmd/dbufstat/Makefile.am
> +++ b/cmd/dbufstat/Makefile.am
> @@ -1 +1 @@
> -dist_bin_SCRIPTS = dbufstat.py
> +dist_sbin_SCRIPTS = dbufstat.py
> diff --git a/debian/rules b/debian/rules
> index 15e0592..9c7d462 100755
> --- a/debian/rules
> +++ b/debian/rules
> @@ -48,7 +48,7 @@ endif
>  
>         @# Build the userland, but don't build the kernel modules.
>         dh_auto_configure -- \
> -         --bindir=/usr/sbin \
> +         --bindir=/usr/bin \
>           --sbindir=/sbin \
>           --libdir=/lib \
>           --with-udevdir=/lib/udev \
> 

IMHO, that's a step in the right direction, especially for preventing
future misplaced files (bindir should not be set to [/usr]/sbin). But
note that this moves the scripts from their current location in
/usr/sbin to just /sbin (not sure whether this is worth spending extra
effort on, especially since usr merge seems to be happening soon?).

> But given that rm by definition always is available in /bin/,
> independent of how zfs was compiled, I believe this change make more
> sense to fix this exact problem:
> 
> diff --git a/etc/systemd/system/zfs-share.service.in b/etc/systemd/system/zfs-share.service.in
> index 688731e..494f5cb 100644
> --- a/etc/systemd/system/zfs-share.service.in
> +++ b/etc/systemd/system/zfs-share.service.in
> @@ -9,7 +9,7 @@ PartOf=smb.service
>  [Service]
>  Type=oneshot
>  RemainAfterExit=yes
> -ExecStartPre=- at bindir@/rm -f /etc/dfs/sharetab
> +ExecStartPre=-/bin/rm -f /etc/dfs/sharetab
>  ExecStart=@sbindir@/zfs share -a
>  
>  [Install]
> 

This is what we've done in our downstream package, I did not send a
patch because the change is so trivial ;)

> Perhaps we should commit both?

Sounds good to me. Thanks for the quick response btw!