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=-@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!