Bug 1450546 - Paths to some tools are hardcoded to /sbin or /usr/sbin
Summary: Paths to some tools are hardcoded to /sbin or /usr/sbin
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: GlusterFS
Classification: Community
Component: core
Version: mainline
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: nh2
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2017-05-13 00:24 UTC by nh2
Modified: 2018-06-20 17:56 UTC (History)
4 users (show)

Fixed In Version: glusterfs-v4.1.0
Clone Of:
Environment:
Last Closed: 2018-06-20 17:56:51 UTC
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Embargoed:


Attachments (Terms of Use)

Description nh2 2017-05-13 00:24:36 UTC
Description of problem (using gluster 3.10.1):

In some places, Gluster hardcodes locations to lookup tools like xfs_info and tune2fs to looking them up in /sbin or /usr/sbin (see e.g. https://github.com/gluster/glusterfs/blob/9dc37dd8aa57bea5b86ac4f2a44e31106aeb58ad/xlators/mgmt/glusterd/src/glusterd-utils.c#L6088).

This is inconsistent with calls to other tools that Gluster does, where it does not hardcode, and instead relies on PATH, which makes sense (especially since /sbin and /usr/sbin are typically already on PATH).

Hardcoding paths makes problems on distributions where e.g. xfs_info are not in /sbin or /usr/sbin (for example NixOS), and also makes it harder to let gluster use instrumented tools e.g. for logging purposes.

I think that gluster should not hardcode such paths, but consistently rely on them being in PATH, as it does in many other instances.

Comment 1 nh2 2017-05-13 00:28:32 UTC
Of course another alternative would be to make these paths compile-time option, but it seems to me that using PATH is the simpler and more consistent solution as many other parts of Gluster already follow this logic.

Comment 2 nh2 2017-05-13 00:58:51 UTC
I've made a commit for 3.10 that relies on PATH everywhere:

https://github.com/nh2/glusterfs/commit/67fbd3aadc2c4caeb14418609f5c7af6de36081b

Please indicate if you agree with my assessment, then I'll happily put it under review.

Comment 3 Raghavendra Talur 2017-05-14 18:07:35 UTC
Needs review from packaging component. Adding needinfo on Kaleb.


nh2, if agreed upon you will have to send the patch to review.gluster.org as github repo is a read-only mirror for gerrit instance.

Comment 4 nh2 2017-05-14 18:27:02 UTC
Yep sure

Comment 5 Kaleb KEITHLEY 2017-05-14 23:14:42 UTC
What needinfo are you asking me for? Are you asking whether I agree with nh2 about using the PATH instead of hard-coded FQ paths? I suppose in general I do agree.

If by compile time options you mean either some sort of autoconf build magic or a maze of feature test macros in the source to attempt to use a FQ path to every tool on every Linux distribution (to say nothing of *BSD) then I think that's a recipe for a maintenance nightmare.

Submit the patch in gerrit on review.gluster.org against the master branch. Once it has been reviewed and merged on the master branch then if you want you can submit a separate backport the 3.11, 3.10, and 3.8 branches.

Comment 6 nh2 2017-05-14 23:28:05 UTC
OK, thanks! I will work on a patch for master then and put it through Gerrit.

Comment 7 Raghavendra Talur 2017-05-15 06:58:09 UTC
(In reply to Kaleb KEITHLEY from comment #5)
> What needinfo are you asking me for? Are you asking whether I agree with nh2
> about using the PATH instead of hard-coded FQ paths? I suppose in general I
> do agree.

Yes, just wanted you to have a look at the proposed patch at github so as to reduce work that nh2 has to do to get it on gerrit.

Thanks Kaleb.

Comment 8 Worker Ant 2018-03-21 18:15:45 UTC
REVIEW: https://review.gluster.org/19759 (Don't use hardcoded /sbin, /usr/bin etc. paths. Fixes #1450546) posted (#1) for review on master by Niklas Hambüchen

Comment 9 Worker Ant 2018-05-01 01:33:05 UTC
REVIEW: https://review.gluster.org/19759 (Don't use hardcoded /sbin, /usr/bin etc. paths. Fixes #1450546) posted (#2) for review on master by Kaleb KEITHLEY

Comment 10 Worker Ant 2018-05-03 11:47:29 UTC
COMMIT: https://review.gluster.org/19759 committed in master by "Jeff Darcy" <jeff.us> with a commit message- Don't use hardcoded /sbin, /usr/bin etc. paths. Fixes #1450546

Instead, rely on programs to be in PATH, as gluster already
does in many places across its code base.

Change-Id: Id21152fe42f5b67205d8f1571b0656c4d5f74246
BUG: 1450546
Signed-off-by: Niklas Hambuechen <mail>

Comment 11 Shyamsundar 2018-06-20 17:56:51 UTC
This bug is getting closed because a release has been made available that should address the reported issue. In case the problem is still not fixed with glusterfs-v4.1.0, please open a new bug report.

glusterfs-v4.1.0 has been announced on the Gluster mailinglists [1], packages for several distributions should become available in the near future. Keep an eye on the Gluster Users mailinglist [2] and the update infrastructure for your distribution.

[1] http://lists.gluster.org/pipermail/announce/2018-June/000102.html
[2] https://www.gluster.org/pipermail/gluster-users/


Note You need to log in before you can comment on or make changes to this bug.