[PATCH] Suppress locking initialisation failure messages when --ignorelockingfailure is used

[PATCH] Suppress locking initialisation failure messages when --ignorelockingfailure is used

[Peter Rajnoha]

We should not show error messages about locking initialisation failures when it is expected
and --ignorelockingfailure option is used (rhbz #561938).

Peter


diff --git a/lib/locking/locking.c b/lib/locking/locking.c
index 54e5e25..db70433 100644
--- a/lib/locking/locking.c
+++ b/lib/locking/locking.c
@@ -237,7 +237,8 @@ int init_locking(int type, struct cmd_context *cmd)
 				 _blocking_supported ? "" : "Non-blocking ");
 
 		if (!init_file_locking(&_locking, cmd)) {
-			log_error("File-based locking initialisation failed.");
+			if (!ignorelockingfailure())
+				log_error("File-based locking initialisation failed.");
 			break;
 		}
 		return 1;
@@ -264,7 +265,8 @@ int init_locking(int type, struct cmd_context *cmd)
 	case 3:
 		log_very_verbose("Cluster locking selected.");
 		if (!init_cluster_locking(&_locking, cmd)) {
-			log_error("Internal cluster locking initialisation failed.");
+			if (!ignorelocking_failure())
+				log_error("Internal cluster locking initialisation failed.");
 			break;
 		}
 		return 1;
@@ -291,7 +293,7 @@ int init_locking(int type, struct cmd_context *cmd)
 			  "be inaccessible.");
 		if (init_file_locking(&_locking, cmd))
 			return 1;
-		else
+		else if (!ignorelockingfailure())
 			log_error("File-based locking initialisation failed.");
 	}
 

[PATCH] Suppress locking initialisation failure messages when --ignorelockingfailure is used

[Milan Broz]

On 02/08/2010 02:17 PM, Peter Rajnoha wrote:
> We should not show error messages about locking initialisation failures when it is expected
> and --ignorelockingfailure option is used (rhbz #561938).

> 
> diff --git a/lib/locking/locking.c b/lib/locking/locking.c
> index 54e5e25..db70433 100644
> --- a/lib/locking/locking.c
> +++ b/lib/locking/locking.c
> @@ -237,7 +237,8 @@ int init_locking(int type, struct cmd_context *cmd)
>  				 _blocking_supported ? "" : "Non-blocking ");
>  
>  		if (!init_file_locking(&_locking, cmd)) {
> -			log_error("File-based locking initialisation failed.");
> +			if (!ignorelockingfailure())
> +				log_error("File-based locking initialisation failed.");

I do not like this patch at all.

"Ignore locking failure" does not mean "Do not print error when locking failed" IMHO.

I thought that dracut use rw root. So something changed again and remounts it to read-only again?

Milan

Suppress locking initialisation failure messages when --ignorelockingfailure is used

[Mike Snitzer]

On Mon, Feb 08 2010 at  8:32am -0500,
Milan Broz <mbroz@redhat.com> wrote:

> 
> On 02/08/2010 02:17 PM, Peter Rajnoha wrote:
> > We should not show error messages about locking initialisation failures when it is expected
> > and --ignorelockingfailure option is used (rhbz #561938).
> 
> > 
> > diff --git a/lib/locking/locking.c b/lib/locking/locking.c
> > index 54e5e25..db70433 100644
> > --- a/lib/locking/locking.c
> > +++ b/lib/locking/locking.c
> > @@ -237,7 +237,8 @@ int init_locking(int type, struct cmd_context *cmd)
> >  				 _blocking_supported ? "" : "Non-blocking ");
> >  
> >  		if (!init_file_locking(&_locking, cmd)) {
> > -			log_error("File-based locking initialisation failed.");
> > +			if (!ignorelockingfailure())
> > +				log_error("File-based locking initialisation failed.");
> 
> I do not like this patch at all.
> 
> "Ignore locking failure" does not mean "Do not print error when locking failed" IMHO.
> 
> I thought that dracut use rw root. So something changed again and
> remounts it to read-only again?

Dracut is rw but we (dracut's lvm hooks) impose read-only locking
through dracut's lvm.conf.  This was done so no metadata updates could
occur within the initramfs environment.

Mike

Suppress locking initialisation failure messages when --ignorelockingfailure is used

[Peter Rajnoha]

On 02/08/2010 02:46 PM, Mike Snitzer wrote:
> 
> Dracut is rw but we (dracut's lvm hooks) impose read-only locking
> through dracut's lvm.conf.  This was done so no metadata updates could
> occur within the initramfs environment.

Yes, dracut environment is rw but the root fs itself is mounted ro first.
The "vgchange -a y --ignorelockingfailure" (that is called after the pivotroot)
is run just before the rw remount happens (..looking at Fedora/RHEL6 rc.sysinit).

We didn't notice that directly before, because init_locking silently "failed"
(but returned with OK status from init_locking fn, since --ignorelockingfailure is set).

Peter

Suppress locking initialisation failure messages when --ignorelockingfailure is used

[Mike Snitzer]

On Mon, Feb 08 2010 at  9:36am -0500,
Peter Rajnoha <prajnoha@redhat.com> wrote:

> On 02/08/2010 02:46 PM, Mike Snitzer wrote:
> > 
> > Dracut is rw but we (dracut's lvm hooks) impose read-only locking
> > through dracut's lvm.conf.  This was done so no metadata updates could
> > occur within the initramfs environment.
> 
> Yes, dracut environment is rw but the root fs itself is mounted ro first.
> The "vgchange -a y --ignorelockingfailure" (that is called after the pivotroot)
> is run just before the rw remount happens (..looking at Fedora/RHEL6 rc.sysinit).
> 
> We didn't notice that directly before, because init_locking silently "failed"
> (but returned with OK status from init_locking fn, since --ignorelockingfailure is set).

Shouldn't rc.sysinit's "vgchange -a y --ignorelockingfailure" come
_after_ root's rw remount?

Dracut has already taken care of activating any LVs needed for the root
FS.  What advantage is there in trying to activate other (non-root) LVs
before root is rw?

Mike

Suppress locking initialisation failure messages when --ignorelockingfailure is used

[Peter Rajnoha]

On 02/08/2010 03:55 PM, Mike Snitzer wrote:
> Shouldn't rc.sysinit's "vgchange -a y --ignorelockingfailure" come
> _after_ root's rw remount?
> 
> Dracut has already taken care of activating any LVs needed for the root
> FS.  What advantage is there in trying to activate other (non-root) LVs
> before root is rw?

Yes, I'm just asking the same questions, too :) It seems to me there's no
reason to call it before the remount. So maybe editing rc.sysinit a little
would provide us a cleaner solution...

Peter

Suppress locking initialisation failure messages when --ignorelockingfailure is used

[Milan Broz]

On 02/08/2010 03:55 PM, Mike Snitzer wrote:
> On Mon, Feb 08 2010 at  9:36am -0500,
> Peter Rajnoha <prajnoha@redhat.com> wrote:
> 
>> On 02/08/2010 02:46 PM, Mike Snitzer wrote:
>>>
>>> Dracut is rw but we (dracut's lvm hooks) impose read-only locking
>>> through dracut's lvm.conf.  This was done so no metadata updates could
>>> occur within the initramfs environment.
>>
>> Yes, dracut environment is rw but the root fs itself is mounted ro first.
>> The "vgchange -a y --ignorelockingfailure" (that is called after the pivotroot)
>> is run just before the rw remount happens (..looking at Fedora/RHEL6 rc.sysinit).
>>
>> We didn't notice that directly before, because init_locking silently "failed"
>> (but returned with OK status from init_locking fn, since --ignorelockingfailure is set).
> 
> Shouldn't rc.sysinit's "vgchange -a y --ignorelockingfailure" come
> _after_ root's rw remount?

Exactly. It should also start volume monitoring etc.

Note that monitoring need rw access (at least for status log to syslog,
but also if it triggers some change, it will require real lock - to perform
repair, snapshot resize or other action; metadata backups etc).

I think that the sequence is broken and we should not paper over it by disabling
the error message.

Milan

Suppress locking initialisation failure messages when --ignorelockingfailure is used

[Mike Snitzer]

On Mon, Feb 08 2010 at 10:04am -0500,
Milan Broz <mbroz@redhat.com> wrote:

> On 02/08/2010 03:55 PM, Mike Snitzer wrote:
> > On Mon, Feb 08 2010 at  9:36am -0500,
> > Peter Rajnoha <prajnoha@redhat.com> wrote:
> > 
> >> On 02/08/2010 02:46 PM, Mike Snitzer wrote:
> >>>
> >>> Dracut is rw but we (dracut's lvm hooks) impose read-only locking
> >>> through dracut's lvm.conf.  This was done so no metadata updates could
> >>> occur within the initramfs environment.
> >>
> >> Yes, dracut environment is rw but the root fs itself is mounted ro first.
> >> The "vgchange -a y --ignorelockingfailure" (that is called after the pivotroot)
> >> is run just before the rw remount happens (..looking at Fedora/RHEL6 rc.sysinit).
> >>
> >> We didn't notice that directly before, because init_locking silently "failed"
> >> (but returned with OK status from init_locking fn, since --ignorelockingfailure is set).
> > 
> > Shouldn't rc.sysinit's "vgchange -a y --ignorelockingfailure" come
> > _after_ root's rw remount?
> 
> Exactly. It should also start volume monitoring etc.
> 
> Note that monitoring need rw access (at least for status log to syslog,
> but also if it triggers some change, it will require real lock - to perform
> repair, snapshot resize or other action; metadata backups etc).

The monitoring gets started, by the "lvm2-monitor" service, after
rc.sysinit.  So we should be fine there.

> I think that the sequence is broken and we should not paper over it by
> disabling the error message.

Agreed, the sequence is broken in rc.sysinit.

Mike

Suppress locking initialisation failure messages when --ignorelockingfailure is used

[Milan Broz]

On 02/08/2010 04:09 PM, Mike Snitzer wrote:
>>> Shouldn't rc.sysinit's "vgchange -a y --ignorelockingfailure" come
>>> _after_ root's rw remount?
>>
>> Exactly. It should also start volume monitoring etc.
>>
>> Note that monitoring need rw access (at least for status log to syslog,
>> but also if it triggers some change, it will require real lock - to perform
>> repair, snapshot resize or other action; metadata backups etc).
> 
> The monitoring gets started, by the "lvm2-monitor" service, after
> rc.sysinit.  So we should be fine there.

I was just not sure if "vgchange -a y" here tries to start monitoring or not.
If so, what happens? (lvm2-monitor init rc skript then will just restart it?)
Is it what we want?

Milan

Suppress locking initialisation failure messages when --ignorelockingfailure is used

[Mike Snitzer]

On Mon, Feb 08 2010 at 10:31am -0500,
Milan Broz <mbroz@redhat.com> wrote:

> On 02/08/2010 04:09 PM, Mike Snitzer wrote:
> >>> Shouldn't rc.sysinit's "vgchange -a y --ignorelockingfailure" come
> >>> _after_ root's rw remount?
> >>
> >> Exactly. It should also start volume monitoring etc.
> >>
> >> Note that monitoring need rw access (at least for status log to syslog,
> >> but also if it triggers some change, it will require real lock - to perform
> >> repair, snapshot resize or other action; metadata backups etc).
> > 
> > The monitoring gets started, by the "lvm2-monitor" service, after
> > rc.sysinit.  So we should be fine there.
> 
> I was just not sure if "vgchange -a y" here tries to start monitoring or not.
> If so, what happens? (lvm2-monitor init rc skript then will just restart it?)
> Is it what we want?

Gah, good point!  Though with current rc.sysinit, because of read-only 
root, I believe monitoring will fail to start because of locking
failure.  But I can't clearly recall; will need to trace it.

Changing rc.sysinit's "vgchange -a y" to be after the root's remount rw
would effectively eliminate the need for the "lvm2-monitor" initscript
(for startup).  This could be a good thing.

But any removal of lvm2-monitor is secondary; as there is potentially
some value in having a script to start/stop monitoring.  Until it is
removed/disabled, at a minimum, we expose ourselves to redundant polling
at boot (e.g. snapshot-merge completion polling).  That is, IFF we move
rc.sysinit's "vgchange -a y" to be after remount rw.

I'm still not seeing a reason why LVM volumes are getting activated
before root's remount rw.  I think we should change that.  Then we're
left having to decide on LVM monitoring (disable that service?).

Mike