[Linux-ha-dev] [RFC] Change the behavior of cibadmin on dangerous options

[Dejan Muhamedagic]

On Mon, Dec 03, 2007 at 07:40:11PM +0100, Andrew Beekhof wrote:
> 
> On Dec 3, 2007, at 7:23 PM, Dejan Muhamedagic wrote:
> 
> >Hi,
> >
> >On Tue, Dec 04, 2007 at 12:20:15AM +0800, Xinwei Hu wrote:
> >>Hi all,
> >>
> >>  We have a instance about cibadmin recently. A typo of 'cibadmin -r
> >>blahblah' forces the HA into RO mode without any warning, and the
> >>field engineer almost panic. ;)
> >
> >Agree. Why is the RO mode needed? Doesn't look useful for regular
> >users.
> 
> its not.
> 
> >Also, cibadmin should exit without doing any changes if
> >the parameters supplied are not good.
> 
> -r is a perfectly valid command.  no reason to bail.

They used "cibadmin -r whatever" and that had the same effect,
though there was a warning printed:

# cibadmin -r skfjskdjf
non-option ARGV-elements: skfjskdjf 

> but i think only allowing the long option name is a good idea
> 
> >
> >
> >>  This leads me to think that cibadmin should give detailed warning
> >>on these dangerous options ahead. But the change will inevitably
> >>change the current behavior of cibadmin.
> >>
> >>  A draft patch is attached for this purpose. Your insightful
> >>comments are welcome.
> >
> >Thanks for the patch.
> >
> >Dejan
> >
> >>  Thanks.
> >
> >>--- ../../dev-dsk/crm/admin/cibadmin.c	2007-09-24  
> >>13:57:31.000000000 +0800
> >>+++ admin/cibadmin.c	2007-12-03 15:37:52.000000000 +0800
> >>@@ -95,6 +95,8 @@
> >>{
> >>	int argerr = 0;
> >>	int flag;
> >>+	int cib_dangerous = 0;
> >>+	const char* cib_dangerous_reason = NULL;
> >>	char *admin_input_xml = NULL;
> >>	char *admin_input_file = NULL;
> >>	gboolean admin_input_stdin = FALSE;
> >>@@ -222,10 +224,14 @@
> >>				break;
> >>			case 'r':
> >>				cib_action = CIB_OP_SLAVE;
> >>+				cib_dangerous = 1;
> >>+				cib_dangerous_reason = "forces the local CIB 
> >>instance into R/O  mode";
> >>				break;
> >>			case 'w':
> >>				cib_action = CIB_OP_MASTER;
> >>				command_options |= cib_scope_local;
> >>+				cib_dangerous = 1;
> >>+				cib_dangerous_reason = "forces the local CIB 
> >>instance into R/W  mode";
> >>				break;
> >>			case 'V':
> >>				command_options = command_options | 
> >>				cib_verbose;
> >>@@ -259,12 +265,16 @@
> >>			case 'b':
> >>				command_options |= cib_inhibit_bcast;
> >>				command_options |= cib_scope_local;
> >>+				cib_dangerous = 1;
> >>+				cib_dangerous_reason = "not be broadcast to 
> >>other nodes in  anyway";
> >>				break;
> >>			case 's':
> >>				command_options |= cib_sync_call;
> >>				break;
> >>			case 'f':
> >>				command_options |= cib_quorum_override;
> >>+				cib_dangerous = 1;
> >>+				cib_dangerous_reason = "force a write to the 
> >>CIB regardless of  quorum";
> >>				break;
> >>			default:
> >>				printf("Argument code 0%o (%c)"
> >>@@ -294,6 +304,19 @@
> >>		usage(crm_system_name, LSB_EXIT_GENERIC);
> >>	}
> >>	
> >>+	if (cib_dangerous) {
> >>+		char ui;
> >>+		fprintf(stdout, "Your command will %s. \nAre you sure this 
> >>is  really what you want?[y/N]", cib_dangerous_reason);
> >>+		fflush(stdout);
> >>+		scanf("%c", &ui);
> >>+		if (ui=='Y' || ui=='y') {
> >>+			fprintf(stdout, "Go as you wish\n");
> >>+			fflush(stdout);
> >>+		} else {
> >>+			exit(LSB_EXIT_GENERIC);
> >>+		}
> >>+	}
> >>+
> >>	if(admin_input_file != NULL) {
> >>		FILE *xml_strm = fopen(admin_input_file, "r");
> >>		input = file2xml(xml_strm, FALSE);
> >
> >>_______________________________________________________
> >>Linux-HA-Dev: Linux-HA-Dev at lists.linux-ha.org
> >>http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> >>Home Page: http://linux-ha.org/
> >
> >_______________________________________________________
> >Linux-HA-Dev: Linux-HA-Dev at lists.linux-ha.org
> >http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> >Home Page: http://linux-ha.org/
> 
> _______________________________________________________
> Linux-HA-Dev: Linux-HA-Dev at lists.linux-ha.org
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/