Bug 577308 – sys-fs/lvm2: depend() should pipe the output of lvm to /dev/null

• Product: Gentoo Linux
• Status: RESOLVED FIXED

[Description Martin Väth 2016-03-13 14:25:17 UTC]

The init script of lvm2 calls lvm already for the dependency calculation.
The "visible" error for people not using lvm (but having it installed e.g. as a dependency of cryptsetup) is e.g. this:

 * Caching service dependencies ...
File descriptor 3 (pipe:[3216316]) leaked on lvm invocation. Parent PID 1923: /bin/sh
  /run/lvm/lvmetad.socket: connect failed: No such file or directory
  WARNING: Failed to connect to lvmetad. Falling back to internal scanning.                                                                

See also https://forums.gentoo.org/viewtopic-t-1040538.html

IMHO, it would not be a good solution to just redirect the error to /dev/null, but it would be better to fix the issue itself:

Even if openrc does not enforce it, I think that the depend() function in openrc should be very lightweight and return a static result (similar to the global scope of ebuilds): First, this function is called relatively often (even if lvm is not started/stopped), and second, it is very strange if dependencies on the next start depend possibly on results of the current run.

IMHO, the correct solution would be to avoid the automagic adding of
"need lvmetad"
at all but instead perhaps inform the user (e.g. through readme-gentoo-r1.eclass) that he might need to add
rc_need=lvmetad
to /etc/conf.d/lvm in certain configurations.

[Comment 1 Robin Johnson 2016-03-14 19:53:29 UTC]

What version of lvm2 are you using?
files/lvm.rc-2.02.105-r2 contains only this:

depend() {
    before checkfs fsck
    after modules device-mapper
    need lvmetad sysfs
}

[Comment 2 Robin Johnson 2016-03-14 19:55:25 UTC]

Crap, I realize there has been a regression; at some point a bad bump of the ebuild happened, and it's been installing that older init script again.

lvm2-2.02.116-r5.ebuild:		newinitd "${FILESDIR}"/lvmetad.initd-2.02.116-r3 lvmetad
lvm2-2.02.136.ebuild:		newinitd "${FILESDIR}"/lvmetad.initd-2.02.105-r2 lvmetad
...
lvm2-2.02.145-r1.ebuild:		newinitd "${FILESDIR}"/lvmetad.initd-2.02.105-r2 lvmetad

lvm2-2.02.116-r2.ebuild:		newinitd "${FILESDIR}"/lvm.rc-2.02.105-r2 lvm
lvm2-2.02.116-r4.ebuild:		newinitd "${FILESDIR}"/lvm.rc-2.02.116-r4 lvm
lvm2-2.02.116-r5.ebuild:		newinitd "${FILESDIR}"/lvm.rc-2.02.116-r4 lvm
lvm2-2.02.136.ebuild:		newinitd "${FILESDIR}"/lvm.rc-2.02.105-r2 lvm
...
lvm2-2.02.145-r1.ebuild:		newinitd "${FILESDIR}"/lvm.rc-2.02.105-r2 lvm

[Comment 3 Robin Johnson 2016-03-14 19:57:59 UTC]

cardoe:
You introduce this change, your call on how to change it.

[Comment 4 Doug Goldstein (RETIRED) 2016-03-15 02:42:52 UTC]

(In reply to Robin Johnson from comment #3)
> cardoe:
> You introduce this change, your call on how to change it.

My plan was to drag the new init script (not to mention a few other bugs fixes that got dropped along the way) to all the existing versions. That's why I brought up culling the versions we don't want to ever go stable or support. But that's another story.

I'll bump the script to add 2> /dev/null. The suggestions in comment 0 are unfortunately misguided. I would recommend reading the runscript man page.

[Comment 5 Doug Goldstein (RETIRED) 2016-03-15 04:00:14 UTC]

Thanks for the report. Fixed in http://gitweb.gentoo.org/repo/gentoo.git/commit/?id=d537027f4c66273bd05b9b9105f8fd1b5ab25f31

[Comment 6 Martin Väth 2016-03-16 15:49:30 UTC]

(In reply to Doug Goldstein from comment #4)
> The suggestions in comment 0 are unfortunately misguided. [...]
> I would recommend reading the runscript man page.

I would have expected that you know that there is no such manpage.
Certainly, openrc does nowhere recommend to make the dependency calculation dependent on currently running daemons:

This is a straight route to breakage, because there are situations when the dependencies are calculated before anything else has started (e.g. if some /etc/init.d file was touched but after an unclean shutdown).

I will locally avoid the problematic approach by INSTALL_MASK-ing the lvm2 init scripts.

[Comment 7 Doug Goldstein (RETIRED) 2016-03-16 21:23:48 UTC]

(In reply to Martin Väth from comment #6)
> (In reply to Doug Goldstein from comment #4)
> > The suggestions in comment 0 are unfortunately misguided. [...]
> > I would recommend reading the runscript man page.
> 
> I would have expected that you know that there is no such manpage.
> Certainly, openrc does nowhere recommend to make the dependency calculation
> dependent on currently running daemons:

RUNSCRIPT(8)                System Manager's Manual               RUNSCRIPT(8)

NAME
     runscript — a means of hooking shell commands into a service

If you want the new name is "openrc-run", I recommend reading the EXAMPLE section.

There is absolutely no dependency on running daemons. Its a generated dependency based on a configuration file. The command that is run is "lvm dumpconfig global" to get the parsed contents of /etc/lvm/lvm.conf. Unfortunately users have had multiple entries in /etc/lvm/lvm.conf and by running the "lvm dumpconfig global" command to parse the file and we take the same action that lvm will since we're using the lvm parsing code.

> 
> This is a straight route to breakage, because there are situations when the
> dependencies are calculated before anything else has started (e.g. if some
> /etc/init.d file was touched but after an unclean shutdown).

An unclean shutdown might result in warnings on boot. No news here. People having actual concrete problems when certain things are not started in the proper order is a real issue that this addresses.

[Comment 8 Martin Väth 2016-03-19 13:53:53 UTC]

(In reply to Doug Goldstein from comment #7)
> If you want the new name is "openrc-run"

Thanks. You are right: The manpage was simply renamed in current stable openrc.

> There is absolutely no dependency on running daemons.

So "lvm dumpconfig global" works in case of a correct configuration correctly even if lvmd is not running yet.,,
In this case, it is really strange that lvm prints a warning.
Filtering this warning is indeed a good idea.

Sorry for the noise.

[Comment 9 Lars Wendler (Polynomial-C) (RETIRED) 2016-12-08 14:41:52 UTC]

*** Bug 580408 has been marked as a duplicate of this bug. ***