All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] improve activation monitoring option processing
@ 2010-03-23 19:23 Mike Snitzer
  2010-03-24  8:49 ` Milan Broz
  2010-03-24  9:19 ` Milan Broz
  0 siblings, 2 replies; 7+ messages in thread
From: Mike Snitzer @ 2010-03-23 19:23 UTC (permalink / raw)
  To: lvm-devel

Add "monitoring" option to "activation" section of lvm.conf

Have clvmd consult the lvm.conf "activation/monitoring" too.

Introduce toollib.c:get_activation_monitoring_mode().

Error out when both --monitor and --ignoremonitoring are provided.

Add --monitor and --ignoremonitoring support to lvcreate.  Update
lvcreate man page accordingly.

Clarify that '--monitor' controls the start and stop of monitoring in
the {vg,lv}change man pages.

Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
 daemons/clvmd/lvm-functions.c    |    8 ++++++--
 doc/example.conf                 |    4 ++++
 lib/metadata/lv_manip.c          |    2 ++
 lib/metadata/metadata-exported.h |    1 +
 liblvm/lvm_lv.c                  |    1 +
 man/lvchange.8.in                |    3 +--
 man/lvcreate.8.in                |   14 ++++++++++++++
 man/vgchange.8.in                |    3 +--
 tools/commands.h                 |   14 +++++++++-----
 tools/lvchange.c                 |    9 +++++----
 tools/lvcreate.c                 |    3 +++
 tools/toollib.c                  |   21 +++++++++++++++++++++
 tools/toollib.h                  |    3 +++
 tools/vgchange.c                 |    9 +++++----
 14 files changed, 76 insertions(+), 19 deletions(-)

diff --git a/daemons/clvmd/lvm-functions.c b/daemons/clvmd/lvm-functions.c
index 02d401b..60b6db4 100644
--- a/daemons/clvmd/lvm-functions.c
+++ b/daemons/clvmd/lvm-functions.c
@@ -542,8 +542,12 @@ int do_lock_lv(unsigned char command, unsigned char lock_flags, char *resource)
 	if (lock_flags & LCK_MIRROR_NOSYNC_MODE)
 		init_mirror_in_sync(0);
 
-	if (!(lock_flags & LCK_DMEVENTD_MONITOR_MODE))
-		init_dmeventd_monitor(DEFAULT_DMEVENTD_MONITOR);
+	if (!(lock_flags & LCK_DMEVENTD_MONITOR_MODE)) {
+		int dmeventd_mode =
+			find_config_tree_bool(cmd, "activation/monitoring",
+					      DEFAULT_DMEVENTD_MONITOR);
+		init_dmeventd_monitor(dmeventd_mode);
+	}
 
 	cmd->partial_activation = 0;
 
diff --git a/doc/example.conf b/doc/example.conf
index 511e20c..89e13ed 100644
--- a/doc/example.conf
+++ b/doc/example.conf
@@ -428,6 +428,10 @@ activation {
     # which used mlockall() to pin the whole process's memory while activating
     # devices.
     # use_mlockall = 0
+
+    # Monitoring is enabled by default when activating logical volumes.
+    # Set to 0 to disable monitoring or use the --ignoremonitoring option.
+    # monitoring = 1
 }
 
 
diff --git a/lib/metadata/lv_manip.c b/lib/metadata/lv_manip.c
index 93fd57f..bb3f5cd 100644
--- a/lib/metadata/lv_manip.c
+++ b/lib/metadata/lv_manip.c
@@ -3056,6 +3056,8 @@ int lv_create_single(struct volume_group *vg,
 
 	backup(vg);
 
+	init_dmeventd_monitor(lp->activation_monitoring);
+
 	if (lp->snapshot) {
 		if (!activate_lv_excl(cmd, lv)) {
 			log_error("Aborting. Failed to activate snapshot "
diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h
index 2f1405f..b02a22c 100644
--- a/lib/metadata/metadata-exported.h
+++ b/lib/metadata/metadata-exported.h
@@ -535,6 +535,7 @@ struct lvcreate_params {
 	int minor; /* all */
 	int log_count; /* mirror */
 	int nosync; /* mirror */
+	int activation_monitoring; /* all */
 
 	char *origin; /* snap */
 	const char *vg_name; /* all */
diff --git a/liblvm/lvm_lv.c b/liblvm/lvm_lv.c
index 86175ae..82e6d4b 100644
--- a/liblvm/lvm_lv.c
+++ b/liblvm/lvm_lv.c
@@ -111,6 +111,7 @@ static void _lv_set_default_params(struct lvcreate_params *lp,
 	lp->zero = 1;
 	lp->major = -1;
 	lp->minor = -1;
+	lp->activation_monitoring = DEFAULT_DMEVENTD_MONITOR;
 	lp->vg_name = vg->name;
 	lp->lv_name = lvname; /* FIXME: check this for safety */
 	lp->pvh = &vg->pvs;
diff --git a/man/lvchange.8.in b/man/lvchange.8.in
index 08a83c4..99f9bf3 100644
--- a/man/lvchange.8.in
+++ b/man/lvchange.8.in
@@ -56,8 +56,7 @@ of your data.
 Set the minor number.
 .TP
 .I \-\-monitor y|n
-Controls whether or not a mirrored logical volume is monitored by
-dmeventd, if it is installed.
+Start or stop monitoring a logical volume, if dmeventd is installed.
 If a device used by a monitored mirror reports an I/O error,
 the failure is handled according to 
 \fBmirror_image_fault_policy\fP and \fBmirror_log_fault_policy\fP
diff --git a/man/lvcreate.8.in b/man/lvcreate.8.in
index 314c354..826610f 100644
--- a/man/lvcreate.8.in
+++ b/man/lvcreate.8.in
@@ -7,6 +7,8 @@ lvcreate \- create a logical volume in an existing volume group
 [\-\-alloc AllocationPolicy]
 [\-A|\-\-autobackup y|n] [\-C|\-\-contiguous y|n] [\-d|\-\-debug]
 [\-h|\-?|\-\-help] [\-\-noudevsync]
+[\-\-ignoremonitoring]
+[\-\-monitor {y|n}]
 [\-i|\-\-stripes Stripes [\-I|\-\-stripesize StripeSize]]
 {\-l|\-\-extents LogicalExtentsNumber[%{VG|PVS|FREE}] |
  \-L|\-\-size LogicalVolumeSize[bBsSkKmMgGtTpPeE]}
@@ -26,6 +28,8 @@ VolumeGroupName [PhysicalVolumePath[:PE[-PE]]...]
  \-L|\-\-size LogicalVolumeSize[bBsSkKmMgGtTpPeE]}
 [\-c|\-\-chunksize ChunkSize]
 [\-\-noudevsync]
+[\-\-ignoremonitoring]
+[\-\-monitor {y|n}]
 \-n|\-\-name SnapshotLogicalVolumeName
 {{\-s|\-\-snapshot}
 OriginalLogicalVolumePath | 
@@ -128,6 +132,16 @@ process will not wait for notification from udev.
 It will continue irrespective of any possible udev processing
 in the background.  You should only use this if udev is not running
 or has rules that ignore the devices LVM2 creates.
+.I \-\-monitor y|n
+Start or avoid monitoring a logical volume, if dmeventd is installed.
+If a device used by a monitored mirror reports an I/O error,
+the failure is handled according to 
+\fBmirror_image_fault_policy\fP and \fBmirror_log_fault_policy\fP
+set in \fBlvm.conf\fP.
+.TP
+.I \-\-ignoremonitoring
+Make no attempt to interact with dmeventd unless \-\-monitor
+is specified.
 .TP
 .I \-p, \-\-permission r|rw
 Set access permissions to read only or read and write.
diff --git a/man/vgchange.8.in b/man/vgchange.8.in
index 5013c1e..e01158a 100644
--- a/man/vgchange.8.in
+++ b/man/vgchange.8.in
@@ -78,8 +78,7 @@ are not marked as clustered.
 Generate new random UUID for specified Volume Groups.
 .TP
 .BR \-\-monitor " " { y | n }
-Controls whether or not a mirrored logical volume is monitored by
-dmeventd, if it is installed.
+Start or stop monitoring a logical volume, if dmeventd is installed.
 If a device used by a monitored mirror reports an I/O error,
 the failure is handled according to 
 .BR mirror_image_fault_policy
diff --git a/tools/commands.h b/tools/commands.h
index ad0b385..58d6d7b 100644
--- a/tools/commands.h
+++ b/tools/commands.h
@@ -150,6 +150,8 @@ xx(lvcreate,
    "\t[-C|--contiguous {y|n}]\n"
    "\t[-d|--debug]\n"
    "\t[-h|-?|--help]\n"
+   "\t[--ignoremonitoring]\n"
+   "\t[--monitor {y|n}]\n"
    "\t[-i|--stripes Stripes [-I|--stripesize StripeSize]]\n"
    "\t{-l|--extents LogicalExtentsNumber[%{VG|PVS|FREE}] |\n"
    "\t -L|--size LogicalVolumeSize[bBsSkKmMgGtTpPeE]}\n"
@@ -177,6 +179,8 @@ xx(lvcreate,
    "\t[-C|--contiguous {y|n}]\n"
    "\t[-d|--debug]\n"
    "\t[-h|-?|--help]\n"
+   "\t[--ignoremonitoring]\n"
+   "\t[--monitor {y|n}]\n"
    "\t[-i|--stripes Stripes [-I|--stripesize StripeSize]]\n"
    "\t{-l|--extents LogicalExtentsNumber[%{VG|FREE|ORIGIN}] |\n"
    "\t -L|--size LogicalVolumeSize[bBsSkKmMgGtTpPeE]}\n"
@@ -192,11 +196,11 @@ xx(lvcreate,
    "\t[PhysicalVolumePath...]\n\n",
 
    addtag_ARG, alloc_ARG, autobackup_ARG, chunksize_ARG, contiguous_ARG,
-   corelog_ARG, extents_ARG, major_ARG, minor_ARG, mirrorlog_ARG, mirrors_ARG,
-   name_ARG, nosync_ARG, noudevsync_ARG, permission_ARG, persistent_ARG,
-   readahead_ARG, regionsize_ARG, size_ARG, snapshot_ARG, stripes_ARG,
-   stripesize_ARG, test_ARG, type_ARG, virtualoriginsize_ARG, virtualsize_ARG,
-   zero_ARG)
+   corelog_ARG, extents_ARG, ignoremonitoring_ARG, major_ARG, minor_ARG,
+   mirrorlog_ARG, mirrors_ARG, monitor_ARG, name_ARG, nosync_ARG, noudevsync_ARG,
+   permission_ARG, persistent_ARG, readahead_ARG, regionsize_ARG, size_ARG,
+   snapshot_ARG, stripes_ARG, stripesize_ARG, test_ARG, type_ARG,
+   virtualoriginsize_ARG, virtualsize_ARG, zero_ARG)
 
 xx(lvdisplay,
    "Display information about a logical volume",
diff --git a/tools/lvchange.c b/tools/lvchange.c
index 2d7955e..534c993 100644
--- a/tools/lvchange.c
+++ b/tools/lvchange.c
@@ -518,7 +518,7 @@ static int lvchange_single(struct cmd_context *cmd, struct logical_volume *lv,
 			   void *handle __attribute((unused)))
 {
 	int doit = 0, docmds = 0;
-	int archived = 0;
+	int dmeventd_mode, archived = 0;
 	struct logical_volume *origin;
 
 	if (!(lv->vg->status & LVM_WRITE) &&
@@ -575,9 +575,10 @@ static int lvchange_single(struct cmd_context *cmd, struct logical_volume *lv,
 		return ECMD_FAILED;
 	}
 
-	init_dmeventd_monitor(arg_int_value(cmd, monitor_ARG,
-					    (is_static() || arg_count(cmd, ignoremonitoring_ARG)) ?
-					    DMEVENTD_MONITOR_IGNORE : DEFAULT_DMEVENTD_MONITOR));
+	if (!get_activation_monitoring_mode(cmd, &dmeventd_mode))
+		return ECMD_FAILED;
+
+	init_dmeventd_monitor(dmeventd_mode);
 
 	/*
 	 * FIXME: DEFAULT_BACKGROUND_POLLING should be "unspecified".
diff --git a/tools/lvcreate.c b/tools/lvcreate.c
index 211cfeb..10b5aa5 100644
--- a/tools/lvcreate.c
+++ b/tools/lvcreate.c
@@ -482,6 +482,9 @@ static int _lvcreate_params(struct lvcreate_params *lp,
 		return 0;
 	}
 
+	if (!get_activation_monitoring_mode(cmd, &lp->activation_monitoring))
+		return_0;
+
 	if (!_lvcreate_name_params(lp, cmd, &argc, &argv) ||
 	    !_read_size_params(lp, lcp, cmd) ||
 	    !_read_stripe_params(lp, cmd) ||
diff --git a/tools/toollib.c b/tools/toollib.c
index 5470169..b1528e2 100644
--- a/tools/toollib.c
+++ b/tools/toollib.c
@@ -1423,3 +1423,24 @@ int pvcreate_params_validate(struct cmd_context *cmd,
 	return 1;
 }
 
+int get_activation_monitoring_mode(struct cmd_context *cmd,
+				   int *monitoring_mode)
+{
+	*monitoring_mode = DEFAULT_DMEVENTD_MONITOR;
+
+	if (arg_count(cmd, monitor_ARG) &&
+	    arg_count(cmd, ignoremonitoring_ARG)) {
+		log_error("Conflicting monitor and ignoremonitoring options");
+		return 0;
+	}
+
+	if (arg_count(cmd, monitor_ARG))
+		*monitoring_mode = arg_int_value(cmd, monitor_ARG,
+						 DEFAULT_DMEVENTD_MONITOR);
+	else if (is_static() || arg_count(cmd, ignoremonitoring_ARG) ||
+		 !find_config_tree_bool(cmd, "activation/monitoring",
+					DEFAULT_DMEVENTD_MONITOR))
+		*monitoring_mode = DMEVENTD_MONITOR_IGNORE;
+	
+	return 1;
+}
diff --git a/tools/toollib.h b/tools/toollib.h
index d284de4..987814e 100644
--- a/tools/toollib.h
+++ b/tools/toollib.h
@@ -112,4 +112,7 @@ int pvcreate_params_validate(struct cmd_context *cmd,
 			     int argc, char **argv,
 			     struct pvcreate_params *pp);
 
+int get_activation_monitoring_mode(struct cmd_context *cmd,
+				   int *monitoring_mode);
+
 #endif
diff --git a/tools/vgchange.c b/tools/vgchange.c
index 6d869f9..e013bde 100644
--- a/tools/vgchange.c
+++ b/tools/vgchange.c
@@ -522,16 +522,17 @@ static int vgchange_single(struct cmd_context *cmd, const char *vg_name,
 			   struct volume_group *vg,
 			   void *handle __attribute((unused)))
 {
-	int r = ECMD_FAILED;
+	int dmeventd_mode, r = ECMD_FAILED;
 
 	if (vg_is_exported(vg)) {
 		log_error("Volume group \"%s\" is exported", vg_name);
 		return ECMD_FAILED;
 	}
 
-	init_dmeventd_monitor(arg_int_value(cmd, monitor_ARG,
-					    (is_static() || arg_count(cmd, ignoremonitoring_ARG)) ?
-					    DMEVENTD_MONITOR_IGNORE : DEFAULT_DMEVENTD_MONITOR));
+	if (!get_activation_monitoring_mode(cmd, &dmeventd_mode))
+		return ECMD_FAILED;
+
+	init_dmeventd_monitor(dmeventd_mode);
 
 	/*
 	 * FIXME: DEFAULT_BACKGROUND_POLLING should be "unspecified".



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2] improve activation monitoring option processing
  2010-03-23 19:23 [PATCH v2] improve activation monitoring option processing Mike Snitzer
@ 2010-03-24  8:49 ` Milan Broz
  2010-03-24  9:19 ` Milan Broz
  1 sibling, 0 replies; 7+ messages in thread
From: Milan Broz @ 2010-03-24  8:49 UTC (permalink / raw)
  To: lvm-devel

On 03/23/2010 08:23 PM, Mike Snitzer wrote:
> Add "monitoring" option to "activation" section of lvm.conf
> 
> Have clvmd consult the lvm.conf "activation/monitoring" too.

I am not sure if this is correct, there is flag sent from
calling process. I think the process should not send
this flag in the first place and not read lvm.conf later.

and specifically NOT in do_lock_lv for every LV.

> Introduce toollib.c:get_activation_monitoring_mode().
> 
> Error out when both --monitor and --ignoremonitoring are provided.
> 
> Add --monitor and --ignoremonitoring support to lvcreate.  Update
> lvcreate man page accordingly.
> 
> Clarify that '--monitor' controls the start and stop of monitoring in
> the {vg,lv}change man pages.


Sorry if it is dumb question, but if user disables monitoring,
do we still emit "block_on_error" for mirror table?

If so, is it correct? When a fail happens mirror all IOs waits
for userspace to do something - but there is no monitoring active.

Probably ok if command run manually (like in itnitrd, because we later
start dmeventd in initscripts) but if it is disabled permanently in lvm.conf?

Milan



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2] improve activation monitoring option processing
  2010-03-23 19:23 [PATCH v2] improve activation monitoring option processing Mike Snitzer
  2010-03-24  8:49 ` Milan Broz
@ 2010-03-24  9:19 ` Milan Broz
  2010-03-24 13:07   ` Mike Snitzer
  1 sibling, 1 reply; 7+ messages in thread
From: Milan Broz @ 2010-03-24  9:19 UTC (permalink / raw)
  To: lvm-devel



On 03/23/2010 08:23 PM, Mike Snitzer wrote:
> Add "monitoring" option to "activation" section of lvm.conf
>
> Have clvmd consult the lvm.conf "activation/monitoring" too.
please do it something like this... (untested)

Milan
---

 daemons/clvmd/lvm-functions.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/daemons/clvmd/lvm-functions.c b/daemons/clvmd/lvm-functions.c
index 60b6db4..b62969c 100644
--- a/daemons/clvmd/lvm-functions.c
+++ b/daemons/clvmd/lvm-functions.c
@@ -483,6 +483,7 @@ const char *do_lock_query(char *resource)
 int do_lock_lv(unsigned char command, unsigned char lock_flags, char *resource)
 {
 	int status = 0;
+	int dmeventd_mode = dmeventd_monitor_mode();
 
 	DEBUGLOG("do_lock_lv: resource '%s', cmd = %s, flags = %s, memlock = %d\n",
 		 resource, decode_locking_cmd(command), decode_flags(lock_flags), memlock());
@@ -542,12 +543,8 @@ int do_lock_lv(unsigned char command, unsigned char lock_flags, char *resource)
 	if (lock_flags & LCK_MIRROR_NOSYNC_MODE)
 		init_mirror_in_sync(0);
 
-	if (!(lock_flags & LCK_DMEVENTD_MONITOR_MODE)) {
-		int dmeventd_mode =
-			find_config_tree_bool(cmd, "activation/monitoring",
-					      DEFAULT_DMEVENTD_MONITOR);
+	if (!(lock_flags & LCK_DMEVENTD_MONITOR_MODE))
 		init_dmeventd_monitor(dmeventd_mode);
-	}
 
 	cmd->partial_activation = 0;
 
@@ -638,6 +635,9 @@ int do_refresh_cache()
 		return -1;
 	}
 
+	init_dmeventd_monitor(find_config_tree_bool(cmd,
+			      "activation/monitoring",
+			      DEFAULT_DMEVENTD_MONITOR));
 	init_full_scan_done(0);
 	init_ignore_suspended_devices(1);
 	lvmcache_label_scan(cmd, 2);




^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2] improve activation monitoring option processing
  2010-03-24  9:19 ` Milan Broz
@ 2010-03-24 13:07   ` Mike Snitzer
  2010-03-24 15:44     ` Mike Snitzer
  0 siblings, 1 reply; 7+ messages in thread
From: Mike Snitzer @ 2010-03-24 13:07 UTC (permalink / raw)
  To: lvm-devel

On Wed, Mar 24 2010 at  5:19am -0400,
Milan Broz <mbroz@redhat.com> wrote:

> 
> 
> On 03/23/2010 08:23 PM, Mike Snitzer wrote:
> > Add "monitoring" option to "activation" section of lvm.conf
> >
> > Have clvmd consult the lvm.conf "activation/monitoring" too.
> please do it something like this... (untested)

OK, I already committed that other patch (got ack to do so).

Your patch looks good.  I had some doubts about checking lvm.conf like I
did in do_lock_query (but didn't explicitly share as much).

Mike



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2] improve activation monitoring option processing
  2010-03-24 13:07   ` Mike Snitzer
@ 2010-03-24 15:44     ` Mike Snitzer
  2010-03-25 21:51       ` Mike Snitzer
  0 siblings, 1 reply; 7+ messages in thread
From: Mike Snitzer @ 2010-03-24 15:44 UTC (permalink / raw)
  To: lvm-devel

On Wed, Mar 24 2010 at  9:07am -0400,
Mike Snitzer <snitzer@redhat.com> wrote:

> On Wed, Mar 24 2010 at  5:19am -0400,
> Milan Broz <mbroz@redhat.com> wrote:
> 
> > 
> > 
> > On 03/23/2010 08:23 PM, Mike Snitzer wrote:
> > > Add "monitoring" option to "activation" section of lvm.conf
> > >
> > > Have clvmd consult the lvm.conf "activation/monitoring" too.
> > please do it something like this... (untested)
> 
> OK, I already committed that other patch (got ack to do so).
> 
> Your patch looks good.  I had some doubts about checking lvm.conf like I
> did in do_lock_query (but didn't explicitly share as much).

Update _process_config() to establish '_dmeventd_monitor' global based
on lvm.conf.  This allows clvmd's calls to create_toolcontext() and
refresh_toolcontext() to (re)establish dmeventd monitoring based on
lvm.conf (rather than always using DEFAULT_DMEVENTD_MONITOR).

clvmd's do_lock_lv() already properly controls dmeventd monitoring based
on LCK_DMEVENTD_MONITOR_MODE in lock_flags -- which currently gets set
based on the '_dmeventd_monitor' global.

Follow-on work will tie command line overrides into the command context
to allow clvmd's dmeventd monitoring to be controlled from the
commandline.  So clvmd will consult the command context for dmeventd
monitoring mode rather than looking at the global (which is process
specific).

Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---

diff --git a/daemons/clvmd/lvm-functions.c b/daemons/clvmd/lvm-functions.c
index 60b6db4..3060954 100644
--- a/daemons/clvmd/lvm-functions.c
+++ b/daemons/clvmd/lvm-functions.c
@@ -542,12 +542,8 @@ int do_lock_lv(unsigned char command, unsigned char lock_flags, char *resource)
 	if (lock_flags & LCK_MIRROR_NOSYNC_MODE)
 		init_mirror_in_sync(0);
 
-	if (!(lock_flags & LCK_DMEVENTD_MONITOR_MODE)) {
-		int dmeventd_mode =
-			find_config_tree_bool(cmd, "activation/monitoring",
-					      DEFAULT_DMEVENTD_MONITOR);
-		init_dmeventd_monitor(dmeventd_mode);
-	}
+	if (!(lock_flags & LCK_DMEVENTD_MONITOR_MODE))
+		init_dmeventd_monitor(1);
 
 	cmd->partial_activation = 0;
 
diff --git a/lib/commands/toolcontext.c b/lib/commands/toolcontext.c
index 7aac361..94375de 100644
--- a/lib/commands/toolcontext.c
+++ b/lib/commands/toolcontext.c
@@ -281,6 +281,10 @@ static int _process_config(struct cmd_context *cmd)
 						  "activation/missing_stripe_filler",
 						  DEFAULT_STRIPE_FILLER);
 
+	init_dmeventd_monitor(find_config_tree_bool(cmd,
+						    "activation/monitoring",
+						    DEFAULT_DMEVENTD_MONITOR));
+
 	/* FIXME Missing error code checks from the stats, not log_warn?, notify if setting overridden, delay message/check till it is actually used (eg consider if lvm shell - file could appear later after this check)? */
 	if (!strcmp(cmd->stripe_filler, "/dev/ioerror") &&
 	    stat(cmd->stripe_filler, &st))



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2] improve activation monitoring option processing
  2010-03-24 15:44     ` Mike Snitzer
@ 2010-03-25 21:51       ` Mike Snitzer
  2010-03-26  2:59         ` [PATCH] allow disabled dmeventd monitoring to propagate via clvmd Mike Snitzer
  0 siblings, 1 reply; 7+ messages in thread
From: Mike Snitzer @ 2010-03-25 21:51 UTC (permalink / raw)
  To: lvm-devel

On Wed, Mar 24 2010 at 11:44am -0400,
Mike Snitzer <snitzer@redhat.com> wrote:
>
> Update _process_config() to establish '_dmeventd_monitor' global based
> on lvm.conf.  This allows clvmd's calls to create_toolcontext() and
> refresh_toolcontext() to (re)establish dmeventd monitoring based on
> lvm.conf (rather than always using DEFAULT_DMEVENTD_MONITOR).
> 
> clvmd's do_lock_lv() already properly controls dmeventd monitoring based
> on LCK_DMEVENTD_MONITOR_MODE in lock_flags -- which currently gets set
> based on the '_dmeventd_monitor' global.

Actually, it looks like we never offered/tested the ability to disable
monitoring across the cluster.

clvmd's do_lock_lv() will _not_ call init_dmeventd_monitor(0) unless
dmeventd monitoring is disabled (which will currently never happen
because '_dmeventd_monitor' defaults to enabled; the _process_config
patch, described in 1st paragraph above, will address that).

Also, dmeventd_monitor_mode() does not return a binary value.  That is
the low hanging fruit (I'm working on a patch to appropriately replace
dmeventd_monitor_mode() calls expecting a binary return with 
is_dmeventd_monitor_enabled()).

I think those 2 patches fix the ability to disable monitoring with
clvmd.  Tracing seems to show it works.  Does this sound reasonable?

I'll post the patches shortly.
Mike


p.s. overview of current cluster dmeventd_monitor propagation:

static int _dmeventd_monitor = DEFAULT_DMEVENTD_MONITOR;

int dmeventd_monitor_mode(void)
{
        return _dmeventd_monitor;
}

lib/locking/cluster_locking.c:_lock_for_cluster() {
...
        if (dmeventd_monitor_mode())
                args[1] |= LCK_DMEVENTD_MONITOR_MODE;
}

daemons/clvmd/lvm-functions.c:do_lock_lv() {
...
        if (!(lock_flags & LCK_DMEVENTD_MONITOR_MODE))
                init_dmeventd_monitor(0);

        ...

        if (!(lock_flags & LCK_DMEVENTD_MONITOR_MODE))
                init_dmeventd_monitor(DEFAULT_DMEVENTD_MONITOR);
}



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] allow disabled dmeventd monitoring to propagate via clvmd
  2010-03-25 21:51       ` Mike Snitzer
@ 2010-03-26  2:59         ` Mike Snitzer
  0 siblings, 0 replies; 7+ messages in thread
From: Mike Snitzer @ 2010-03-26  2:59 UTC (permalink / raw)
  To: lvm-devel

The lvm commands that control dmeventd monitoring (lvchange, vgchange,
lvcreate) now properly establish the '_dmeventd_monitor' via commandline
of lvm.conf -- they in turn pass their setting on to clvmd through
_lock_for_cluster().

So we really don't have a need to init_dmeventd_monitor() based on
lvm.conf when creating or refreshing the toolcontext -- especially not
for clvmd's benefit.

And all existing dmeventd_monitor_mode() callers properly account for it
returning tri-state (except for _lock_for_cluster() -- it treating it as
a binary return didn't dawn on me until I wasted too much time tracing
this issue).

I've tested this minimalist patch quite a bit:


clvmd's do_lock_lv() already properly controls dmeventd monitoring based
on LCK_DMEVENTD_MONITOR_MODE in lock_flags -- though one small fix was
needed for this to work: _lock_for_cluster() must treat
dmeventd_monitor_mode()'s return as a tri-state value.

Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
diff --git a/lib/locking/cluster_locking.c b/lib/locking/cluster_locking.c
index b82b077..f98f445 100644
--- a/lib/locking/cluster_locking.c
+++ b/lib/locking/cluster_locking.c
@@ -307,6 +307,7 @@ static int _lock_for_cluster(struct cmd_context *cmd, unsigned char clvmd_cmd,
 	char *args;
 	const char *node = "";
 	int len;
+	int dmeventd_mode;
 	int saved_errno = errno;
 	lvm_response_t *response = NULL;
 	int num_responses;
@@ -324,7 +325,8 @@ static int _lock_for_cluster(struct cmd_context *cmd, unsigned char clvmd_cmd,
 	if (mirror_in_sync())
 		args[1] |= LCK_MIRROR_NOSYNC_MODE;
 
-	if (dmeventd_monitor_mode())
+	dmeventd_mode = dmeventd_monitor_mode();
+	if (dmeventd_mode != DMEVENTD_MONITOR_IGNORE && dmeventd_mode)
 		args[1] |= LCK_DMEVENTD_MONITOR_MODE;
 
 	if (cmd->partial_activation)



^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2010-03-26  2:59 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-23 19:23 [PATCH v2] improve activation monitoring option processing Mike Snitzer
2010-03-24  8:49 ` Milan Broz
2010-03-24  9:19 ` Milan Broz
2010-03-24 13:07   ` Mike Snitzer
2010-03-24 15:44     ` Mike Snitzer
2010-03-25 21:51       ` Mike Snitzer
2010-03-26  2:59         ` [PATCH] allow disabled dmeventd monitoring to propagate via clvmd Mike Snitzer

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.