Skip to contentSkip to conversation

Do not print "-1" as locking type if locking fails.

PATCH New Milan Broz 2 participants 3 messages Jan 21, 2010

lore
Share & tools
lore

Milan Broz

author
#0 lore raw

PATCH 1/1 Do not print "-1" as locking type if locking fails.

Move error message to locking constructor and print more descriptive message if locking fails instead of "Locking type -1 initialisation failed."

---
 lib/locking/locking.c |   16 ++++++++++++----
 liblvm/lvm_base.c     |    1 -
 tools/lvmcmdline.c    |    2 --
 3 files changed, 12 insertions(+), 7 deletions(-)
lib/locking/locking.c +12 -4
index a2c6a3e..54e5e25 100644
@@ -224,7 +224,7 @@ int init_locking(int type, struct cmd_context *cmd)
 
 	_blocking_supported = find_config_tree_int(cmd,
 	    "global/wait_for_locks", DEFAULT_WAIT_FOR_LOCKS);
-	
+
 	switch (type) {
 	case 0:
 		init_no_locking(&_locking, cmd);
@@ -236,8 +236,10 @@ int init_locking(int type, struct cmd_context *cmd)
 		log_very_verbose("%sFile-based locking selected.",
 				 _blocking_supported ? "" : "Non-blocking ");
 
-		if (!init_file_locking(&_locking, cmd))
+		if (!init_file_locking(&_locking, cmd)) {
+			log_error("File-based locking initialisation failed.");
 			break;
+		}
 		return 1;
 
 #ifdef HAVE_LIBDL
@@ -249,8 +251,10 @@ int init_locking(int type, struct cmd_context *cmd)
 		}
 		if (!find_config_tree_int(cmd, "locking/fallback_to_clustered_locking",
 			    find_config_tree_int(cmd, "global/fallback_to_clustered_locking",
-						 DEFAULT_FALLBACK_TO_CLUSTERED_LOCKING)))
+						 DEFAULT_FALLBACK_TO_CLUSTERED_LOCKING))) {
+			log_error("External locking initialisation failed.");
 			break;
+		}
 #endif
 
 #ifdef CLUSTER_LOCKING_INTERNAL
@@ -259,8 +263,10 @@ int init_locking(int type, struct cmd_context *cmd)
 
 	case 3:
 		log_very_verbose("Cluster locking selected.");
-		if (!init_cluster_locking(&_locking, cmd))
+		if (!init_cluster_locking(&_locking, cmd)) {
+			log_error("Internal cluster locking initialisation failed.");
 			break;
+		}
 		return 1;
 #endif
 
@@ -285,6 +291,8 @@ int init_locking(int type, struct cmd_context *cmd)
 			  "be inaccessible.");
 		if (init_file_locking(&_locking, cmd))
 			return 1;
+		else
+			log_error("File-based locking initialisation failed.");
 	}
 
 	if (!ignorelockingfailure())
liblvm/lvm_base.c +0 -1
index b4be906..ac94692 100644
@@ -51,7 +51,6 @@ lvm_t lvm_init(const char *system_dir)
 	/* initialize locking */
 	if (!init_locking(-1, cmd)) {
 		/* FIXME: use EAGAIN as error code here */
-		log_error("Locking initialisation failed.");
 		lvm_quit((lvm_t) cmd);
 		return NULL;
 	}
tools/lvmcmdline.c +0 -2
index e4b56da..ad1513d 100644
@@ -1027,8 +1027,6 @@ int lvm_run_command(struct cmd_context *cmd, int argc, char **argv)
 		locking_type = -1;
 
 	if (!init_locking(locking_type, cmd)) {
-		log_error("Locking type %d initialisation failed.",
-			  locking_type);
 		ret = ECMD_FAILED;
 		goto out;
 	}
Signature · 1 line
1.6.6
#1 lore raw

PATCH 1/1 Do not print "-1" as locking type if locking fails.

Quoted 35 lines · Milan Broz

Move error message to locking constructor and print more descriptive message if locking fails instead of "Locking type -1 initialisation failed."

---
 lib/locking/locking.c |   16 ++++++++++++----
 liblvm/lvm_base.c     |    1 -
 tools/lvmcmdline.c    |    2 --
 3 files changed, 12 insertions(+), 7 deletions(-)
lib/locking/locking.c +4 -2
index a2c6a3e..54e5e25 100644
@@ -224,7 +224,7 @@ int init_locking(int type, struct cmd_context *cmd)
 
 	_blocking_supported = find_config_tree_int(cmd,
 	    "global/wait_for_locks", DEFAULT_WAIT_FOR_LOCKS);
-	
+
 	switch (type) {
 	case 0:
 		init_no_locking(&_locking, cmd);
@@ -236,8 +236,10 @@ int init_locking(int type, struct cmd_context *cmd)
 		log_very_verbose("%sFile-based locking selected.",
 				 _blocking_supported ? "" : "Non-blocking ");
 
-		if (!init_file_locking(&_locking, cmd))
+		if (!init_file_locking(&_locking, cmd)) {
+			log_error("File-based locking initialisation failed.");
 			break;
+		}
 		return 1;
 
 #ifdef HAVE_LIBDL

lib/locking/locking.c int init_locking(int type, struct cmd_context *cmd) · line 258

Quoted 9 lines
lib/locking/locking.c +3 -1
@@ -249,8 +251,10 @@ int init_locking(int type, struct cmd_context *cmd)
 		}
 		if (!find_config_tree_int(cmd, "locking/fallback_to_clustered_locking",
 			    find_config_tree_int(cmd, "global/fallback_to_clustered_locking",
-						 DEFAULT_FALLBACK_TO_CLUSTERED_LOCKING)))
+						 DEFAULT_FALLBACK_TO_CLUSTERED_LOCKING))) {
+			log_error("External locking initialisation failed.");
 			break;
+		}
 #endif

This looks strange and inconsistent with the rest of the code. IMO the message be directly under the above code when init_external_locking() fails, as the others are.

Quoted 40 lines
lib/locking/locking.c +5 -1
 #ifdef CLUSTER_LOCKING_INTERNAL
@@ -259,8 +263,10 @@ int init_locking(int type, struct cmd_context *cmd)
 
 	case 3:
 		log_very_verbose("Cluster locking selected.");
-		if (!init_cluster_locking(&_locking, cmd))
+		if (!init_cluster_locking(&_locking, cmd)) {
+			log_error("Internal cluster locking initialisation failed.");
 			break;
+		}
 		return 1;
 #endif
 
@@ -285,6 +291,8 @@ int init_locking(int type, struct cmd_context *cmd)
 			  "be inaccessible.");
 		if (init_file_locking(&_locking, cmd))
 			return 1;
+		else
+			log_error("File-based locking initialisation failed.");
 	}
 
 	if (!ignorelockingfailure())
liblvm/lvm_base.c +0 -1
index b4be906..ac94692 100644
@@ -51,7 +51,6 @@ lvm_t lvm_init(const char *system_dir)
 	/* initialize locking */
 	if (!init_locking(-1, cmd)) {
 		/* FIXME: use EAGAIN as error code here */
-		log_error("Locking initialisation failed.");
 		lvm_quit((lvm_t) cmd);
 		return NULL;
 	}
tools/lvmcmdline.c +0 -0
index e4b56da..ad1513d 100644

tools/lvmcmdline.c int lvm_run_command(struct cmd_context *cmd, int argc, char **argv) · line 1032

Quoted 8 lines
tools/lvmcmdline.c +0 -2
@@ -1027,8 +1027,6 @@ int lvm_run_command(struct cmd_context *cmd, int argc, char **argv)
 		locking_type = -1;
 
 	if (!init_locking(locking_type, cmd)) {
-		log_error("Locking type %d initialisation failed.",
-			  locking_type);
 		ret = ECMD_FAILED;
 		goto out;
 	}

The rest of the patch looks fine. Ack.

author
#2 lore raw

PATCH 1/1 Do not print "-1" as locking type if locking fails.

Quoted 6 lines · Dave Wysochanski
Quoted 6 lines
lib/locking/locking.c +1 -1
@@ -249,8 +251,10 @@ int init_locking(int type, struct cmd_context *cmd)
 		}
 		if (!find_config_tree_int(cmd, "locking/fallback_to_clustered_locking",
 			    find_config_tree_int(cmd, "global/fallback_to_clustered_locking",
-						 DEFAULT_FALLBACK_TO_CLUSTERED_LOCKING)))
+						 DEFAULT_FALLBACK_TO_CLUSTERED_LOCKING))) {
Quoted 8 lines

lib/locking/locking.c int init_locking(int type, struct cmd_context *cmd) · line 258

Quoted 4 lines
lib/locking/locking.c +2 -0
+			log_error("External locking initialisation failed.");
 			break;
+		}
 #endif

This looks strange and inconsistent with the rest of the code. IMO the message be directly under the above code when init_external_locking() fails, as the others are.

yes. because there is quiet fallback to internal cluster locking, I think it should not print error when fallback to internal locking succeeded.
(I think there are many RHEL4 configuration set to locking type 2 still)

Milan