* Markus Armbruster (arm...@redhat.com) wrote: > "Dr. David Alan Gilbert" <dgilb...@redhat.com> writes: > > > * Markus Armbruster (arm...@redhat.com) wrote: > >> "Dr. David Alan Gilbert (git)" <dgilb...@redhat.com> writes: > >> > >> > From: "Dr. David Alan Gilbert" <dgilb...@redhat.com> > >> > > >> > Allow the 'help' command in preconfig state but > >> > make it only list the preconfig commands. > >> > > >> > Signed-off-by: Dr. David Alan Gilbert <dgilb...@redhat.com> > >> > Reviewed-by: Peter Xu <pet...@redhat.com> > >> > Reviewed-by: Igor Mammedov <imamm...@redhat.com> > >> > --- > >> > hmp-commands.hx | 1 + > >> > monitor.c | 8 +++++++- > >> > 2 files changed, 8 insertions(+), 1 deletion(-) > >> > > >> > diff --git a/hmp-commands.hx b/hmp-commands.hx > >> > index 0734fea931..8bf590ae4b 100644 > >> > --- a/hmp-commands.hx > >> > +++ b/hmp-commands.hx > >> > @@ -15,6 +15,7 @@ ETEXI > >> > .params = "[cmd]", > >> > .help = "show the help", > >> > .cmd = do_help_cmd, > >> > + .flags = "p", > >> > }, > >> > > >> > STEXI > >> > diff --git a/monitor.c b/monitor.c > >> > index f4a16e6a03..31c8f5dc88 100644 > >> > --- a/monitor.c > >> > +++ b/monitor.c > >> > @@ -957,6 +957,10 @@ static void help_cmd_dump_one(Monitor *mon, > >> > { > >> > int i; > >> > > >> > + if (runstate_check(RUN_STATE_PRECONFIG) && !cmd_can_preconfig(cmd)) > >> > { > >> > + return; > >> > + } > >> > + > >> > for (i = 0; i < prefix_args_nb; i++) { > >> > monitor_printf(mon, "%s ", prefix_args[i]); > >> > } > >> > @@ -979,7 +983,9 @@ static void help_cmd_dump(Monitor *mon, const > >> > mon_cmd_t *cmds, > >> > > >> > /* Find one entry to dump */ > >> > for (cmd = cmds; cmd->name != NULL; cmd++) { > >> > - if (compare_cmd(args[arg_index], cmd->name)) { > >> > + if (compare_cmd(args[arg_index], cmd->name) && > >> > + ((!runstate_check(RUN_STATE_PRECONFIG) || > >> > + cmd_can_preconfig(cmd)))) { > >> > >> Why do we need this check in addition to the one in help_cmd_dump_one()? > > > > To gate entire subtables (we've only currently got 'info' and that's > > enabled, > > anyway, so it never actually triggers). > > Something's bothering me around here, but I can't put a finger on it... > let me think aloud. > > There's an asymmetry between command execution and help. The former has > just one check, in monitor_parse_command(). If the command has a > sub-table, and there are arguments, monitor_parse_command() recurses > into the sub-table. Thus, if the command is unavailable, the > sub-commands are unavailable as well, regardless of their "p" flags. > > Help's recursion is different. It's limited to two levels, unlike > execution. help_cmd_dump_one()'s check applies to the last level. > help_cmd_dump()'s check applies to the first level. If there's just one > level, we check twice. Perhaps less than elegant, but harmless and > simple. > > You can't make a sub-command available without also making the command > available. Makes sense, I guess. If you try, your "p" flags on the > sub-commands are silently ignored. That's a bit ugly, but if it doesn't > bother you, I can pretend I didn't see it ;)
Yes I'm OK with that; you've got to enable all the way down to the command that youw ant to enable. I think the difference between execution and help comes from the way you can do 'help' on it's own to list everything, and help on a subtable (help info) and help on a command (help info uuid); I think that's why the recursion gets a bit hairier. Dave > > Dave > > > >> > if (cmd->sub_table) { > >> > /* continue with next arg */ > >> > help_cmd_dump(mon, cmd->sub_table, > > -- > > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK