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

[Xinwei Hu]

2007/12/4, Andrew Beekhof <beekhof at gmail.com>:
>
> 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.
>
> but i think only allowing the long option name is a good idea
Only allowing long option is not easy as it's not supported by the
standard 'getopt' function.

My patch changes the behavior of cibadmin though, it doesn't prevent
itself from using in the scripts, you can work around by  'echo y |
cibadmin -r blahblah'.
> >
> >
> >>   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/
>