Bug 710055 - RFE: expand globs in ExecStart and friends arguments
RFE: expand globs in ExecStart and friends arguments
Status: CLOSED WONTFIX
Product: Fedora
Classification: Fedora
Component: systemd (Show other bugs)
rawhide
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: systemd-maint
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-02 07:17 EDT by Ville Skyttä
Modified: 2011-12-15 19:18 EST (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2011-12-15 19:18:14 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Ville Skyttä 2011-06-02 07:17:14 EDT
I'm considering migrating hddtemp in Fedora to use native systemd service instead of the old init script.

http://pkgs.fedoraproject.org/gitweb/?p=hddtemp.git;a=tree
One feature of the current init script is that if no devices are given in $HDDTEMP_OPTIONS, simple default set of device names is passed to hddtemp.  I'd like to be able to pass something like /dev/[hs]d[a-z] as an argument to hddtemp and have it glob expanded, but seems there's no trivial way to do this in systemd; globs are not expanded in EnvironmentFile nor ExecStart, and I suppose I cannot use actual shell code in EnvironmentFile either.

There are multiple ways to get this done in hddtemp itself, and I suppose I could run it through a wrapper script which adds the default set of devices to the command line if none are present, as well as try some ExecStartPre tricks to modify the EnvironmentFile in place if it contains no device arguments.  But I think a good addition to systemd would be glob expansion at least in ExecStart, probably also in other systemd.service Exec* values (after environment variable expansion), that'd make this and other similar cases easier.
Comment 1 Bill Nottingham 2011-06-02 11:26:35 EDT
sh -c ...?

More seriously, this should get its device list from sysfs, etc.
Comment 2 Ville Skyttä 2011-06-02 15:00:05 EDT
(In reply to comment #1)
> sh -c ...?

Ah yes, that'd probably work for the most trivial cases, but for this particular case preserving backwards compatibility would get hairy enough to warrant a separate script I think.

> More seriously, this should get its device list from sysfs, etc.

Getting off topic for systemd, but I believe hddtemp needs to be passed the /dev devices, I don't know of a way to get list of these conveniently from /sys/*.  But then again I think autodetecting/enumerating sysfs is something that hddtemp should be doing internally.  What did you mean by "etc"?


I wonder why was this bug marked as private to "Red Hat Technical Support (internal)"?
Comment 3 Bill Nottingham 2011-06-02 15:15:01 EDT
(In reply to comment #2)
> But then again I think autodetecting/enumerating sysfs is something
> that hddtemp should be doing internally.  What did you mean by "etc"?

Just that it should be extended to get its device list properly and dynamically, rather than having it passed on the command line.

For example, using one of the udev bindings to search all block devices for 'ID_ATA_FEATURE_SET_SMART_ENABLED'.

> I wonder why was this bug marked as private to "Red Hat Technical Support
> (internal)"?

Because I misclicked something, fixed.
Comment 4 Lennart Poettering 2011-06-13 09:28:34 EDT
Hmpf, iterating through /dev is not acceptable. No program should do that.

Services with multiple individual process instances should usually use the instantiation feature of systemd. So my suggestion would be to package hddtemp similar to hdapsd, i.e. with one daemon instance per device pulled in via udev rules. Have a look at that package for details.

(Also I do wonder what the usefulness of hddtemp is in times where udisks provides the same information)
Comment 5 Ville Skyttä 2011-06-13 12:02:50 EDT
(In reply to comment #4)
> Services with multiple individual process instances should usually use the
> instantiation feature of systemd. So my suggestion would be to package hddtemp
> similar to hdapsd, i.e. with one daemon instance per device pulled in via udev
> rules.

I don't think that would work too well for hddtemp.  There's only one instance of it running for a number of disks, and there are tools that work with its simple protocol at one TCP port where the daemon reports temperatures for all disks it monitors.

> (Also I do wonder what the usefulness of hddtemp is in times where udisks
> provides the same information)

I'm not aware of exactly what udisks provides, but gkrellm integration and size of dependency chain are the first ones that spring to my mind as things that speak for hddtemp over udisks.
Comment 6 Lennart Poettering 2011-06-14 15:41:20 EDT
(In reply to comment #5)
> (In reply to comment #4)
> > Services with multiple individual process instances should usually use the
> > instantiation feature of systemd. So my suggestion would be to package hddtemp
> > similar to hdapsd, i.e. with one daemon instance per device pulled in via udev
> > rules.
> 
> I don't think that would work too well for hddtemp.  There's only one instance
> of it running for a number of disks, and there are tools that work with its
> simple protocol at one TCP port where the daemon reports temperatures for all
> disks it monitors.
 
If it is a singleton service then it should be fixed to enumerate devices on its own with libudev. (And you probably want to use libudev anyway, to support hotplugged hard disks, in particular because at boot time when your service is spawned there is no guarantee that all connected disks are known to the kernel yet).
Comment 7 Ville Skyttä 2011-06-14 17:52:17 EDT
(In reply to comment #6)
> If it is a singleton service then it should be fixed to [...]

That's something to ask upstream, not a package maintainer task, and what you suggest would not be a "fix" but a new feature for it.  This discussion is starting to go towards hddtemp and away from what was the original subject of this bug.  hddtemp was just an example I used, please ignore it - the requested feature is not in any way specific to it.
Comment 8 Michal Schmidt 2011-06-14 18:22:01 EDT
(In reply to comment #7)
> hddtemp was just an example I used, please ignore it - the requested
> feature is not in any way specific to it.

OK, let's ignore hddtemp, but then we have no use-case for the requested feature.
Unless you can come up with a better example, the feature should not be implemented.
Comment 9 Ville Skyttä 2011-06-15 16:38:24 EDT
A generic use case is "invoke foo passing /path/to/*.conf as arguments".  Given the mindset in comment 8 and that the "fix the service instead" argument from comment 6 can be used to shoot down any example case one might find or think of, spending more time on this seems futile.
Comment 10 Fedora Admin XMLRPC Client 2011-10-20 12:25:20 EDT
This package has changed ownership in the Fedora Package Database.  Reassigning to the new owner of this component.
Comment 11 Michal Schmidt 2011-12-15 19:18:14 EST
(In reply to comment #9)
> A generic use case is "invoke foo passing /path/to/*.conf as arguments".

Looks like no systemd developer supports adding glob expansion to ExecStart. Closing as WONTFIX.

If there's a service that really needs this, it's no shame to use:
ExecStart=/bin/sh -c '/bin/foo /path/to/*.conf'
or a wrapper script.

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