[RFC] lvmetad_is_disabled: check config before connecting to socket

[RFC] lvmetad_is_disabled: check config before connecting to socket

[Thomas Lamprecht]

Else, lvmetad always gets started due to systemds socket auto
activation when executing, for example:
 # pvscan --cache
as pvscan uses lvmetad_is_disabled to check if metad is disabled
before connecting. This is OK for systems where systemd is not used
but for others, the systemd 'auto activation on socket connect'
feature causes the start of lvm2-lvmetad.service which is not desired
when disabled in the config.

ref: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=826570
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---

An alternative approach would be to just guard the lvmetad_is_disabled
call in tools/pvscan.c : 412 with the config check - and thus address
the FIXME mentioned there.
Would be maybe the less intrusive version?

RFC.

 lib/cache/lvmetad.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/cache/lvmetad.c b/lib/cache/lvmetad.c
index 1ba844fc8..94fba8979 100644
--- a/lib/cache/lvmetad.c
+++ b/lib/cache/lvmetad.c
@@ -2846,6 +2846,12 @@ int lvmetad_is_disabled(struct cmd_context *cmd, const char **reason)
 	const char *reply_reason;
 	int ret = 0;
 
+	if (!find_config_tree_bool(cmd, global_use_lvmetad_CFG, NULL)) {
+		*reason = "the disable flag was set directly";
+		ret = 1;
+		goto out;
+	}
+
 	reply = daemon_send_simple(_lvmetad, "get_global_info",
 				   "token = %s", "skip",
 				   "pid = " FMTd64, (int64_t)getpid(),
-- 
2.11.0

[RFC] lvmetad_is_disabled: check config before connecting to socket

[Zdenek Kabelac]

Dne 13.10.2017 v 11:08 Thomas Lamprecht napsal(a):
> Else, lvmetad always gets started due to systemds socket auto
> activation when executing, for example:
>   # pvscan --cache
> as pvscan uses lvmetad_is_disabled to check if metad is disabled
> before connecting. This is OK for systems where systemd is not used
> but for others, the systemd 'auto activation on socket connect'
> feature causes the start of lvm2-lvmetad.service which is not desired
> when disabled in the config.
> 
> ref: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=826570
> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
> ---
> 
> An alternative approach would be to just guard the lvmetad_is_disabled
> call in tools/pvscan.c : 412 with the config check - and thus address
> the FIXME mentioned there.
> Would be maybe the less intrusive version?
> 


Hi

The reason why is checked the presence for active service when lvmetad is 
disable is - we have to print warning that  lvmetad is there, but command is 
not going to use it/update it....

This may considerably harm lvmetad consistency.

If you want to avoid starting lvmetad systemd service - just mask this service 
- that's the correct way.


Regards

Zdenek

[RFC] lvmetad_is_disabled: check config before connecting to socket

[Thomas Lamprecht]

On 10/13/2017 12:06 PM, Zdenek Kabelac wrote:
> Dne 13.10.2017 v 11:08 Thomas Lamprecht napsal(a):
>> Else, lvmetad always gets started due to systemds socket auto
>> activation when executing, for example:
>> ? # pvscan --cache
>> as pvscan uses lvmetad_is_disabled to check if metad is disabled
>> before connecting. This is OK for systems where systemd is not used
>> but for others, the systemd 'auto activation on socket connect'
>> feature causes the start of lvm2-lvmetad.service which is not desired
>> when disabled in the config.
>>
>> ref: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=826570
>> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
>> ---
>>
>> An alternative approach would be to just guard the lvmetad_is_disabled
>> call in tools/pvscan.c : 412 with the config check - and thus address
>> the FIXME mentioned there.
>> Would be maybe the less intrusive version?
>>
> 
> 
> Hi
> 
> The reason why is checked the presence for active service when lvmetad is disable is - we have to print warning that? lvmetad is there, but command is not going to use it/update it....

Why not just do the initial fix over a lvmetad.pid file?
Simpler and avoids auto activation.

> 
> This may considerably harm lvmetad consistency.
> 
> If you want to avoid starting lvmetad systemd service - just mask this service - that's the correct way.
> 

I must then mask also the socket as it has a require on the server.
As lvm2-monitor.service requires both, lvm2-lvmetad.service lvm2-lvmetad.socket

This then leads to a (after some time) periodic log spamming:

> Oct 10 08:17:23 dev5 systemd[1]: lvm2-monitor.service: Cannot add dependency job, ignoring: Unit lvm2-lvmetad.socket is masked.
> Oct 10 08:17:23 dev5 systemd[1]: lvm2-lvmetad.socket: Cannot add dependency job, ignoring: Unit lvm2-lvmetad.socket is masked.

So not ideal for us, our and Debian's user.

cheers,
Thomas

[RFC] lvmetad_is_disabled: check config before connecting to socket

[Zdenek Kabelac]

Dne 13.10.2017 v 13:23 Thomas Lamprecht napsal(a):
> On 10/13/2017 12:06 PM, Zdenek Kabelac wrote:
>> Dne 13.10.2017 v 11:08 Thomas Lamprecht napsal(a):
>>> Else, lvmetad always gets started due to systemds socket auto
>>> activation when executing, for example:
>>>  ? # pvscan --cache
>>> as pvscan uses lvmetad_is_disabled to check if metad is disabled
>>> before connecting. This is OK for systems where systemd is not used
>>> but for others, the systemd 'auto activation on socket connect'
>>> feature causes the start of lvm2-lvmetad.service which is not desired
>>> when disabled in the config.
>>>
>>> ref: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=826570
>>> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
>>> ---
>>>
>>> An alternative approach would be to just guard the lvmetad_is_disabled
>>> call in tools/pvscan.c : 412 with the config check - and thus address
>>> the FIXME mentioned there.
>>> Would be maybe the less intrusive version?
>>>
>>
>>
>> Hi
>>
>> The reason why is checked the presence for active service when lvmetad is disable is - we have to print warning that? lvmetad is there, but command is not going to use it/update it....
> 
> Why not just do the initial fix over a lvmetad.pid file?
> Simpler and avoids auto activation.
> 

There is clear race - your command would be 'checking' for pid for lvmetad 
which is just being re/started.  The only valid way for checking of presence 
of running lvmetad in the system is a presence of socket. All other solution 
have various troubles.

> I must then mask also the socket as it has a require on the server.
> As lvm2-monitor.service requires both, lvm2-lvmetad.service lvm2-lvmetad.socket

Well yes - if you do not want to use 'lvmetad' in your system - you need to 
use different set of service dependencies.

The main point is - user might be switching of usage of lvmetad with any 
command - i.e. just pass different setting with --config command option.

> 
> This then leads to a (after some time) periodic log spamming:
> 
>> Oct 10 08:17:23 dev5 systemd[1]: lvm2-monitor.service: Cannot add dependency job, ignoring: Unit lvm2-lvmetad.socket is masked.
>> Oct 10 08:17:23 dev5 systemd[1]: lvm2-lvmetad.socket: Cannot add dependency job, ignoring: Unit lvm2-lvmetad.socket is masked.
> 
> So not ideal for us, our and Debian's user.

IMHO this rather means the specification of the service should likely use 
different sort of dependence on a socket so it would not generate this log 
message.

Regards

Zdenek

[RFC] lvmetad_is_disabled: check config before connecting to socket

[Peter Rajnoha]

On 10/13/2017 02:04 PM, Zdenek Kabelac wrote:
> Dne 13.10.2017 v 13:23 Thomas Lamprecht napsal(a):
>> I must then mask also the socket as it has a require on the server.
>> As lvm2-monitor.service requires both, lvm2-lvmetad.service
>> lvm2-lvmetad.socket

Well, I think we can improve the ordering of the services a bit so we
can remove the "Requires: lvm2-lvmetad.socket" in lvm2-monitor.service
and just rely on the soft dependency with "After: lvm2-lvmetad.socket"
in lvm2-monitor.service. This would allow us to mask the
lvm2-lvmetad.socket without problems then.

Looking at this chart, I think we need to move the lvm2-lvmetad.socket
under local-fs-pre.target so we're sure it's prepared soon enough.

  https://www.freedesktop.org/software/systemd/man/bootup.html

I'll see if that helps and if yes, I'll post the patch.

-- 
Peter

[RFC] lvmetad_is_disabled: check config before connecting to socket

[David Teigland]

On Fri, Oct 13, 2017 at 12:06:49PM +0200, Zdenek Kabelac wrote:

> The reason why is checked the presence for active service when lvmetad is
> disable is - we have to print warning that  lvmetad is there, but command is
> not going to use it/update it....

That happens here:

https://sourceware.org/git/?p=lvm2.git;a=blob;f=lib/commands/toolcontext.c;h=39b300a78bbceda9f5092604be54e04f6f457584;hb=HEAD#l1706

and does not involve connecting to lvmetad.

Dave

[RFC] lvmetad_is_disabled: check config before connecting to socket

[David Teigland]

On Fri, Oct 13, 2017 at 11:08:42AM +0200, Thomas Lamprecht wrote:
> Else, lvmetad always gets started due to systemds socket auto
> activation when executing, for example:
>  # pvscan --cache
> as pvscan uses lvmetad_is_disabled to check if metad is disabled
> before connecting. 

> An alternative approach would be to just guard the lvmetad_is_disabled
> call in tools/pvscan.c : 412 with the config check - and thus address
> the FIXME mentioned there.
> Would be maybe the less intrusive version?

When lvm.conf has use_lvmetad=0, 'pvscan --cache' should not try to
connect to lvmetad, it should return here:

https://sourceware.org/git/?p=lvm2.git;a=blob;f=tools/pvscan.c;h=6581990f8afeb17f4c7f82c6f6b59fe2ed0b990b;hb=HEAD#l323

so I'm not sure it would get to line 412.

This was a little complicated to get working just right in every case, so
I wouldn't be surprised if it's still off somewhere.  There may be some
place where we should call lvmetad_used() prior to calling
lvmetad_is_disabled.  I'm not seeing in your message where exactly
lvmetad_is_disabled is being called for you.

Dave