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

[Dejan Muhamedagic]

Hi,

On Tue, Dec 04, 2007 at 09:56:50AM +0800, Xinwei Hu wrote:
> 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.

It is, in a way. See

http://www.gnu.org/software/libc/manual/html_node/Getopt-Long-Option-Example.html

> 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'.

Should be:

echo y | cibadmin -r

Anyway, there are even more dangerous options, such as -E. Not
sure where the line should be drawn: UNIX lets you shoot your
foot without too many whys. Guess that one has to get used to it.

Thanks,

Dejan


> > >
> > >
> > >>   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/
> >
> _______________________________________________________
> 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/