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: [email protected]
> >>http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> >>Home Page: http://linux-ha.org/
> >
> >_______________________________________________________
> >Linux-HA-Dev: [email protected]
> >http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> >Home Page: http://linux-ha.org/
>
> _______________________________________________________
> Linux-HA-Dev: [email protected]
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/
_______________________________________________________
Linux-HA-Dev: [email protected]
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/
